#2571 Read only repo - Clearly display when access to a repo may not be functional
Merged by pingou. Opened by vivekanand1101.
vivekanand1101/pagure banner  into  master

Download 2571.patch

You should rollback here then :)

rebased

rebased

2 new commits added

  • read only repo: front end banner
  • read only repo: mark read only while adding/removing user/group

1 new commit added

  • read only repo: Add documentation

rebased onto 9ecc3cf91d477886fd39683f4ee1c9cb1fe4eec1

rebased onto 697b44aa929bfcaa969b414cf547593c92dedb5b

1 new commit added

  • give project: generate gitolite acls after giving a project

Let's make clear that it's the fork that's in R/O not the main project that is being forked

I prefer the or or and to be at the start of the line rather than the end of it

Missing an empty line at the end here ;-)_

at least?

is a time taking -> may be a time consuming task (depending on the number of projects hosted)

starts to look good :)

rebased onto 74f62f88db890b44c1af43e2edfef822bd3b9583

i am not able to understand how to write more tests

I propose adding this:

--- a/ pagure/templates/repo_master.html        
+++ b/ pagure/templates/repo_master.html        
@@ -30,7 +30,7 @@
           <span class="oi red-icon" data-glyph="lock-locked"
               title="Private project"></span>
         {% endif %}
-        {% if repo.read_only == true %}
+        {% if repo.read_only %}
             <span class="oi red-icon" data-glyph="file"
                   title="Read Only"></span>
         {% endif %}
@@ -168,10 +168,16 @@
             namespace=repo.parent.namespace)}}">
             {{ repo.parent.fullname }}
           </a>
+          {%- if repo.read_only %} (Read-Only){% endif %}
         </div>
         {% elif repo.is_fork and not repo.parent %}
         <div class="m-t-0">
             Fork from a deleted repository
+            {%- if repo.read_only %} (Read-Only){% endif %}
+        </div>
+        {% elif repo.read_only %}
+        <div class="m-t-0">
+          (Read-Only)
         </div>
         {% endif %}
         <div class="projectinfo m-t-1 m-b-1">

flake8 is pointing out a couple of errors as well

6 new commits added

  • give project: generate gitolite acls after giving a project
  • read only repo: Add unit test for update_read_only_mode method
  • read only repo: Add documentation
  • read only repo: front end banner
  • read only repo: mark read only while adding/removing user/group
  • Read only repo: Add migration script, lib methods

I think this is good but I'd like to change the way we're display the "read-only" message.

This is my proposition:

diff --git a/ pagure/static/pagure.css b/ pagure/static/pagure.css
index 3033695..6542064 100644
--- a/ pagure/static/pagure.css 
+++ b/ pagure/static/pagure.css 
@@ -23,12 +23,6 @@ html
     border-bottom:1px solid #DDD;
 }
-.repo-header-read-only
-{
-    background:#ffc0cb;
-    border-bottom:1px solid #DDD;
-}
-
 .issue_comment table,
 .readme table,
 .card .m-a-2 table
diff --git a/ pagure/templates/repo_master.html b/ pagure/templates/repo_master.html
index c50a6cf..2829d67 100644
--- a/ pagure/templates/repo_master.html        
+++ b/ pagure/templates/repo_master.html        
@@ -18,11 +18,7 @@
 {% endif %}
 {% block content %}
-{% if repo.read_only == true %}
-<div class="repo-header-read-only p-t-1">
-{% else %}
 <div class="repo-header p-t-1">
-{% endif %}
   <div class="container">
     <header>
       <h2 class="repo-name m-b-0">
@@ -30,10 +26,6 @@
           <span class="oi red-icon" data-glyph="lock-locked"
               title="Private project"></span>
         {% endif %}
-        {% if repo.read_only %}
-            <span class="oi red-icon" data-glyph="file"
-                  title="Read Only"></span>
-        {% endif %}
         {% if repo.is_fork -%}
           <a href="{{ url_for(
             'view_user', username=repo.user.user)
@@ -168,16 +160,10 @@
             namespace=repo.parent.namespace)}}">
             {{ repo.parent.fullname }}
           </a>
-          {%- if repo.read_only %} (Read-Only){% endif %}
         </div>
         {% elif repo.is_fork and not repo.parent %}
         <div class="m-t-0">
             Fork from a deleted repository
-            {%- if repo.read_only %} (Read-Only){% endif %}
-        </div>
-        {% elif repo.read_only %}
-        <div class="m-t-0">
-          (Read-Only)
         </div>
         {% endif %}
         <div class="projectinfo m-t-1 m-b-1">
@@ -355,6 +341,21 @@
 </div>
 <div class="container p-t-3">
+    {% if repo.read_only %}
+    <div class="container p-t-2">
+        <div class="alert alert-danger alert-dismissible" role="alert">
+          <button type="button" class="close" data-dismiss="alert" aria-label="Close">
+            <span aria-hidden="true">&times;</span>
+            <span class="sr-only">Close</span>
+          </button>
+          <span class="oi red-icon" data-glyph="file"
+              title="Read Only"></span>
+          The access of this repository are being updated, this may take a
+          little while and during this time you or some of your contributors
+          may not be able to push to the repository.
+        </div>
+    </div>
+    {% endif %}
 {% block repo %}
 {% endblock %}
 </div>

It may require changes to the test as well.

What do you think?

rebased onto ed7e6e6d19e087a01e486dbfeae1766e876296d9

1 new commit added

  • Fix the unit-tests

:thumbsup: :ship: it

Pull-Request has been merged by pingou

Metadata