refactor: user godfathers views

core/static/user/user_godfathers.scss

@@ -195,8 +195,9 @@
           }
         }
       }
+    }
 
-      &.delete {
+    form .link-like {
       margin-top: 10px;
       display: block;
       text-align: center;
@@ -209,7 +210,6 @@
       }
     }
   }
-  }
 
   >a.mini_profile_link {
     display: none;

core/templates/core/macros.jinja

@@ -78,12 +78,6 @@
   {% endif %}
 {% endmacro %}
 
-{% macro delete_godfather(user, profile, godfather, is_father) %}
-  {% if user == profile or user.is_root or user.is_board_member %}
-    <a class="delete" href="{{ url("core:user_godfathers_delete", user_id=profile.id, godfather_id=godfather.id, is_father=is_father) }}">{% trans %}Delete{% endtrans %}</a>
-  {% endif %}
-{% endmacro %}
-
 {% macro paginate_alpine(page, nb_pages) %}
     {# Add pagination buttons for ajax based content with alpine
 

core/templates/core/user_godfathers.jinja

@@ -29,7 +29,16 @@
             <a href="{{ url('core:user_godfathers', user_id=u.id) }}" class="mini_profile_link">
               {{ u.get_mini_item() | safe }}
             </a>
-            {{ delete_godfather(user, profile, u, True) }}
+            {% if user == profile or user.is_root or user.is_board_member %}
+              <form
+                method="post"
+                class="no-margin"
+                action="{{ url("core:user_godfathers_delete", user_id=profile.id, godfather_id=u.id, is_father=True) }}"
+              >
+                {% csrf_token %}
+                <input type="submit" class="link-like" value="{% trans %}Delete{% endtrans %}">
+              </form>
+            {% endif %}
           </li>
         {% endfor %}
       </ul>
@@ -46,7 +55,16 @@
             <a href="{{ url('core:user_godfathers', user_id=u.id) }}" class="mini_profile_link">
               {{ u.get_mini_item()|safe }}
             </a>
-            {{ delete_godfather(user, profile, u, False) }}
+            {% if user == profile or user.is_root or user.is_board_member %}
+              <form
+                method="post"
+                class="no-margin"
+                action="{{ url("core:user_godfathers_delete", user_id=profile.id, godfather_id=u.id, is_father=False) }}"
+              >
+                {% csrf_token %}
+                <input type="submit" class="link-like link-red" value="{% trans %}Delete{% endtrans %}">
+              </form>
+            {% endif %}
           </li>
         {% endfor %}
       </ul>

core/tests/test_user.py

@@ -476,7 +476,55 @@ class TestChangeUserPassword:
         response = client.post(
             url, {"new_password1": "poutou", "new_password2": "poutou"}
         )
-        print(response.text)
         assertRedirects(response, reverse("core:password_change_done"))
         user.refresh_from_db()
         assert user.check_password("poutou") is True
+
+
+@pytest.mark.django_db
+class TestUserGodfather:
+    @pytest.mark.parametrize("godfather", [True, False])
+    def test_add_family(self, client: Client, godfather):
+        user = subscriber_user.make()
+        other_user = subscriber_user.make()
+        client.force_login(user)
+        url = reverse("core:user_godfathers", kwargs={"user_id": user.id})
+        response = client.get(url)
+        assert response.status_code == 200
+        response = client.post(
+            url,
+            {"type": "godfather" if godfather else "godchild", "user": other_user.id},
+        )
+        assertRedirects(response, url)
+        if godfather:
+            assert user.godfathers.contains(other_user)
+        else:
+            assert user.godchildren.contains(other_user)
+
+    def test_tree(self, client: Client):
+        user = subscriber_user.make()
+        client.force_login(user)
+        response = client.get(
+            reverse("core:user_godfathers_tree", kwargs={"user_id": user.id})
+        )
+        assert response.status_code == 200
+
+    def test_remove_family(self, client: Client):
+        user = subscriber_user.make()
+        other_user = subscriber_user.make()
+        user.godfathers.add(other_user)
+        client.force_login(user)
+        response = client.post(
+            reverse(
+                "core:user_godfathers_delete",
+                kwargs={
+                    "user_id": user.id,
+                    "godfather_id": other_user.id,
+                    "is_father": True,
+                },
+            )
+        )
+        assertRedirects(
+            response, reverse("core:user_godfathers", kwargs={"user_id": user.id})
+        )
+        assert not user.godfathers.contains(other_user)

core/views/forms.py

@@ -303,7 +303,6 @@ class UserGodfathersForm(forms.Form):
     )
     user = forms.ModelChoiceField(
         label=_("Select user"),
-        help_text=None,
         required=True,
         widget=AutoCompleteSelectUser,
         queryset=User.objects.all(),
@@ -315,8 +314,6 @@ class UserGodfathersForm(forms.Form):
 
     def clean_user(self):
         other_user = self.cleaned_data.get("user")
-        if not other_user:
-            raise ValidationError(_("This user does not exist"))
         if other_user == self.target_user:
             raise ValidationError(_("You cannot be related to yourself"))
         return other_user

core/views/user.py

@@ -29,6 +29,7 @@ from operator import itemgetter
 from smtplib import SMTPException
 
 from django.contrib.auth import login, views
+from django.contrib.auth.decorators import login_required
 from django.contrib.auth.forms import PasswordChangeForm, SetPasswordForm
 from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin
 from django.core.exceptions import PermissionDenied
@@ -42,6 +43,7 @@ from django.urls import reverse, reverse_lazy
 from django.utils.decorators import method_decorator
 from django.utils.safestring import SafeString
 from django.utils.translation import gettext as _
+from django.views.decorators.http import require_POST
 from django.views.generic import (
     CreateView,
     DeleteView,
@@ -288,10 +290,12 @@ class UserView(UserTabsMixin, CanViewMixin, DetailView):
         return kwargs
 
 
+@require_POST
+@login_required
 def delete_user_godfather(request, user_id, godfather_id, is_father):
     user_is_admin = request.user.is_root or request.user.is_board_member
     if user_id != request.user.id and not user_is_admin:
-        raise PermissionDenied()
+        raise PermissionDenied
     user = get_object_or_404(User, id=user_id)
     to_remove = get_object_or_404(User, id=godfather_id)
     if is_father: