UI/UX tweaks

club/forms.py

@@ -348,7 +348,7 @@ class ClubRoleForm(forms.ModelForm):
     def clean(self):
         cleaned_data = super().clean()
         if "ORDER" in cleaned_data:
-            self.instance.order = cleaned_data["ORDER"]
+            self.instance.order = cleaned_data["ORDER"] - 1
         return cleaned_data
 
 

club/templates/club/club_members.jinja

@@ -17,7 +17,7 @@
       href="{{ url("club:club_roles", club_id=object.id) }}"
       class="btn btn-blue margin-bottom"
     >
-      {% trans %}Manage roles{% endtrans %}
+      <i class="fa fa-users-gear"></i> {% trans %}Manage roles{% endtrans %}
     </a>
   {% endif %}
 

club/templates/club/club_roles.jinja

@@ -1,7 +1,7 @@
 {% extends "core/base.jinja" %}
 
 {% block additional_js %}
-  <script type="module" src="{{ static("bundled/club/role-list-index.ts") }}"></script>
+  <script type="module" src="{{ static("bundled/club/role-list-index.ts") }}" xmlns="http://www.w3.org/1999/html"></script>
 {% endblock %}
 
 {% block additional_css %}
@@ -78,7 +78,7 @@
     {{ form.management_form }}
     {{ form.non_form_errors() }}
     <h3>{% trans %}Presidency{% endtrans %}</h3>
-    <a class="btn btn-blue margin-bottom" href="{{ url("club:new_role_president", club_id=club.id) }}">
+    <a class="btn btn-grey margin-bottom" href="{{ url("club:new_role_president", club_id=club.id) }}">
       <i class="fa fa-plus"></i> {% trans %}add role{% endtrans %}
     </a>
     <details class="clickable margin-bottom">
@@ -105,8 +105,9 @@
         {% endif %}
       {% endfor %}
     </div>
+    <br>
     <h3>{% trans %}Board{% endtrans %}</h3>
-    <a class="btn btn-blue margin-bottom" href="{{ url("club:new_role_board", club_id=club.id) }}">
+    <a class="btn btn-grey margin-bottom" href="{{ url("club:new_role_board", club_id=club.id) }}">
       <i class="fa fa-plus"></i> {% trans %}add role{% endtrans %}
     </a>
     <details class="clickable margin-bottom">
@@ -140,8 +141,9 @@
         {% endif %}
       {% endfor %}
     </div>
+    <br>
     <h3>{% trans %}Members{% endtrans %}</h3>
-    <a class="btn btn-blue margin-bottom" href="{{ url("club:new_role_member", club_id=club.id) }}">
+    <a class="btn btn-grey margin-bottom" href="{{ url("club:new_role_member", club_id=club.id) }}">
       <i class="fa fa-plus"></i> {% trans %}add role{% endtrans %}
     </a>
     <details class="clickable margin-bottom">
@@ -160,6 +162,11 @@
         {% endif %}
       {% endfor %}
     </div>
-    <p><input type="submit" value="{% trans %}Save{% endtrans %}" /></p>
+    <br>
+    <p>
+      <button type="submit" class="btn btn-blue">
+        <i class="fa fa-check"></i>{% trans %}Save{% endtrans %}
+      </button>
+    </p>
   </form>
 {% endblock content %}

club/tests/test_clubrole.py

@@ -23,7 +23,7 @@ def make_club():
     recipe.make(
         is_board=iter([True, True, False]),
         is_presidency=iter([True, False, False]),
-        order=iter([1, 2, 3]),
+        order=iter([0, 1, 2]),
         _quantity=3,
         _bulk_create=True,
     )
@@ -143,6 +143,7 @@ class TestClubRoleUpdate(TestCase):
             Membership, club=cls.club, role=cls.roles[0], user=cls.user, end_date=None
         )
         cls.url = reverse("club:club_roles", kwargs={"club_id": cls.club.id})
+        cls.redirect_url = reverse("club:club_members", kwargs={"club_id": cls.club.id})
 
     def setUp(self):
         self.payload = {
@@ -150,7 +151,7 @@ class TestClubRoleUpdate(TestCase):
             "roles-INITIAL_FORMS": 3,
             "roles-MIN_NUM_FORMS": 0,
             "roles-MAX_NUM_FORMS": 1000,
-            "roles-0-ORDER": self.roles[0].order,
+            "roles-0-ORDER": 1,
             "roles-0-id": self.roles[0].id,
             "roles-0-club": self.club.id,
             "roles-0-is_presidency": True,
@@ -158,7 +159,7 @@ class TestClubRoleUpdate(TestCase):
             "roles-0-name": self.roles[0].name,
             "roles-0-description": self.roles[0].description,
             "roles-0-is_active": True,
-            "roles-1-ORDER": self.roles[1].order,
+            "roles-1-ORDER": 2,
             "roles-1-id": self.roles[1].id,
             "roles-1-club": self.club.id,
             "roles-1-is_presidency": False,
@@ -166,7 +167,7 @@ class TestClubRoleUpdate(TestCase):
             "roles-1-name": self.roles[1].name,
             "roles-1-description": self.roles[1].description,
             "roles-1-is_active": True,
-            "roles-2-ORDER": self.roles[2].order,
+            "roles-2-ORDER": 3,
             "roles-2-id": self.roles[2].id,
             "roles-2-club": self.club.id,
             "roles-2-is_presidency": False,
@@ -183,7 +184,7 @@ class TestClubRoleUpdate(TestCase):
         assert res.status_code == 200
         self.payload["roles-2-name"] = "foo"
         res = self.client.post(self.url, data=self.payload)
-        assertRedirects(res, self.url)
+        assertRedirects(res, self.redirect_url)
         self.roles[2].refresh_from_db()
         assert self.roles[2].name == "foo"
 
@@ -242,9 +243,7 @@ class TestClubRoleUpdate(TestCase):
         self.payload["roles-0-is_presidency"] = False
         self.client.force_login(self.user)
         res = self.client.post(self.url, data=self.payload)
-        assertRedirects(
+        assertRedirects(res, self.redirect_url)
-            res, reverse("club:club_members", kwargs={"club_id": self.club.id})
-        )
         # When the user clicked that button, it still had the right to update roles,
         # so the modification should be applied
         self.roles[0].refresh_from_db()

club/views.py

@@ -441,17 +441,12 @@ class ClubRoleUpdateView(
         return super().get_form_kwargs() | {"form_kwargs": {"label_suffix": ""}}
 
     def get_success_url(self):
-        # if the user lost the right to view the role update page
-        # (because it moved its own role out of the presidency),
-        # redirect to the club member page, else stay on the same page.
-        if self.club.can_roles_be_edited_by(self.request.user):
-            return self.request.path
         return reverse("club:club_members", kwargs={"club_id": self.club.id})
 
     def get_context_data(self, **kwargs):
         return super().get_context_data(**kwargs) | {
             "user_role": ClubRole.objects.filter(
-                club=self.object,
+                club=self.club,
                 members__user=self.request.user,
                 members__end_date=None,
             )