Add create views (#11246)

* Add create view for project owners

* Fix test

* Add create view for project email notifications

* Add create view for project translations

* Fix tests

* Use formview

* Lint

* Use formview

* Add tests for email notification views

readthedocs/projects/tests/test_views.py

@@ -157,7 +157,7 @@ def setUp(self):
         )
 
     def test_invite_by_username(self):
-        url = reverse("projects_users", args=[self.project.slug])
+        url = reverse("projects_users_create", args=[self.project.slug])
         self.client.force_login(self.user)
         resp = self.client.post(
             url,
@@ -176,7 +176,7 @@ def test_invite_by_username(self):
         self.assertEqual(invitation.to_email, None)
 
     def test_invite_by_email(self):
-        url = reverse("projects_users", args=[self.project.slug])
+        url = reverse("projects_users_create", args=[self.project.slug])
         self.client.force_login(self.user)
         resp = self.client.post(
             url,
@@ -196,7 +196,7 @@ def test_invite_by_email(self):
 
     def test_invite_existing_maintainer_by_username(self):
         self.project.users.add(self.another_user)
-        url = reverse("projects_users", args=[self.project.slug])
+        url = reverse("projects_users_create", args=[self.project.slug])
         self.client.force_login(self.user)
         resp = self.client.post(
             url,
@@ -212,7 +212,7 @@ def test_invite_existing_maintainer_by_username(self):
 
     def test_invite_existing_maintainer_by_email(self):
         self.project.users.add(self.another_user)
-        url = reverse("projects_users", args=[self.project.slug])
+        url = reverse("projects_users_create", args=[self.project.slug])
         self.client.force_login(self.user)
         resp = self.client.post(
             url,
@@ -227,7 +227,7 @@ def test_invite_existing_maintainer_by_email(self):
         self.assertFalse(Invitation.objects.for_object(self.project).exists())
 
     def test_invite_unknown_user(self):
-        url = reverse("projects_users", args=[self.project.slug])
+        url = reverse("projects_users_create", args=[self.project.slug])
         self.client.force_login(self.user)
         resp = self.client.post(
             url,

readthedocs/projects/urls/private.py

@@ -30,18 +30,21 @@
     ProjectAdvertisingUpdate,
     ProjectDashboard,
     ProjectDelete,
+    ProjectEmailNotificationsCreate,
     ProjectNotifications,
     ProjectNotificationsDelete,
     ProjectRedirectsCreate,
     ProjectRedirectsDelete,
     ProjectRedirectsInsert,
     ProjectRedirectsList,
     ProjectRedirectsUpdate,
+    ProjectTranslationsCreate,
     ProjectTranslationsDelete,
-    ProjectTranslationsListAndCreate,
+    ProjectTranslationsList,
     ProjectUpdate,
-    ProjectUsersCreateList,
+    ProjectUsersCreate,
     ProjectUsersDelete,
+    ProjectUsersList,
     ProjectVersionCreate,
     ProjectVersionDeleteHTML,
     ProjectVersionDetail,
@@ -105,9 +108,14 @@
     ),
     re_path(
         r"^(?P<project_slug>[-\w]+)/users/$",
-        ProjectUsersCreateList.as_view(),
+        ProjectUsersList.as_view(),
         name="projects_users",
     ),
+    re_path(
+        r"^(?P<project_slug>[-\w]+)/users/create/$",
+        ProjectUsersCreate.as_view(),
+        name="projects_users_create",
+    ),
     re_path(
         r"^(?P<project_slug>[-\w]+)/users/delete/$",
         ProjectUsersDelete.as_view(),
@@ -118,16 +126,26 @@
         ProjectNotifications.as_view(),
         name="projects_notifications",
     ),
+    re_path(
+        r"^(?P<project_slug>[-\w]+)/notifications/create/$",
+        ProjectEmailNotificationsCreate.as_view(),
+        name="projects_notifications_create",
+    ),
     re_path(
         r"^(?P<project_slug>[-\w]+)/notifications/delete/$",
         ProjectNotificationsDelete.as_view(),
         name="projects_notification_delete",
     ),
     re_path(
         r"^(?P<project_slug>[-\w]+)/translations/$",
-        ProjectTranslationsListAndCreate.as_view(),
+        ProjectTranslationsList.as_view(),
         name="projects_translations",
     ),
+    re_path(
+        r"^(?P<project_slug>[-\w]+)/translations/create/$",
+        ProjectTranslationsCreate.as_view(),
+        name="projects_translations_create",
+    ),
     re_path(
         r"^(?P<project_slug>[-\w]+)/translations/delete/(?P<child_slug>[-\w]+)/$",  # noqa
         ProjectTranslationsDelete.as_view(),

readthedocs/projects/views/private.py

@@ -435,24 +435,19 @@ def get_success_url(self):
     def _is_last_user(self):
         return self.get_queryset().count() <= 1
 
+    def get_form(self, data=None, files=None, **kwargs):
+        kwargs["request"] = self.request
+        return super().get_form(data, files, **kwargs)
 
-class ProjectUsersCreateList(SuccessMessageMixin, ProjectUsersMixin, FormView):
-    template_name = "projects/project_users.html"
-    success_message = _("Invitation sent")
 
-    def form_valid(self, form):
-        # Manually calling to save, since this isn't a ModelFormView.
-        form.save()
-        return super().form_valid(form)
+class ProjectUsersList(SuccessMessageMixin, ProjectUsersMixin, FormView):
+    # We only use this to display the form in the list view.
+    http_method_names = ["get"]
+    template_name = "projects/project_users.html"
 
     def _get_invitations(self):
         return Invitation.objects.for_object(self.get_project())
 
-    def get_form_kwargs(self):
-        kwargs = super().get_form_kwargs()
-        kwargs["request"] = self.request
-        return kwargs
-
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
         context["users"] = self.get_queryset()
@@ -461,6 +456,11 @@ def get_context_data(self, **kwargs):
         return context
 
 
+class ProjectUsersCreate(SuccessMessageMixin, ProjectUsersMixin, CreateView):
+    success_message = _("Invitation sent")
+    template_name = "projects/project_users_form.html"
+
+
 class ProjectUsersDelete(ProjectUsersMixin, GenericView):
     http_method_names = ["post"]
 
@@ -485,33 +485,26 @@ def post(self, request, *args, **kwargs):
 
 
 class ProjectNotificationsMixin(ProjectAdminMixin, PrivateViewMixin):
+    form_class = EmailHookForm
+
     def get_success_url(self):
         return reverse(
             "projects_notifications",
             args=[self.get_project().slug],
         )
 
+    def get_form(self, data=None, files=None, **kwargs):
+        kwargs["project"] = self.get_project()
+        return super().get_form(data, files, **kwargs)
+
 
-class ProjectNotifications(ProjectNotificationsMixin, TemplateView):
+class ProjectNotifications(ProjectNotificationsMixin, FormView):
 
     """Project notification view and form view."""
 
+    # We only use this to display the form in the list view.
+    http_method_names = ["get"]
     template_name = "projects/project_notifications.html"
-    email_form = EmailHookForm
-
-    def get_email_form(self):
-        project = self.get_project()
-        return self.email_form(
-            self.request.POST or None,
-            project=project,
-        )
-
-    def post(self, request, *args, **kwargs):
-        if "email" in request.POST:
-            email_form = self.get_email_form()
-            if email_form.is_valid():
-                email_form.save()
-        return HttpResponseRedirect(self.get_success_url())
 
     def _has_old_webhooks(self):
         """
@@ -525,20 +518,25 @@ def _has_old_webhooks(self):
         ).exists()
 
     def get_context_data(self, **kwargs):
-        context = super().get_context_data()
+        context = super().get_context_data(**kwargs)
 
         project = self.get_project()
         emails = project.emailhook_notifications.all()
         context.update(
             {
-                "email_form": self.get_email_form(),
+                # TODO: delete once we no longer need the form in the list view.
+                "email_form": context["form"],
                 "emails": emails,
                 "has_old_webhooks": self._has_old_webhooks(),
             },
         )
         return context
 
 
+class ProjectEmailNotificationsCreate(ProjectNotificationsMixin, CreateView):
+    template_name = "projects/project_notifications_form.html"
+
+
 class ProjectNotificationsDelete(ProjectNotificationsMixin, GenericView):
     http_method_names = ["post"]
 
@@ -615,36 +613,39 @@ def get_webhook(self):
 
 
 class ProjectTranslationsMixin(ProjectAdminMixin, PrivateViewMixin):
+    form_class = TranslationForm
+
     def get_success_url(self):
         return reverse(
             "projects_translations",
             args=[self.get_project().slug],
         )
 
+    def get_form(self, data=None, files=None, **kwargs):
+        kwargs["parent"] = self.get_project()
+        kwargs["user"] = self.request.user
+        return self.form_class(data, files, **kwargs)
+
 
-class ProjectTranslationsListAndCreate(ProjectTranslationsMixin, FormView):
+class ProjectTranslationsList(ProjectTranslationsMixin, FormView):
 
     """Project translations view and form view."""
 
-    form_class = TranslationForm
+    # We only use this to display the form in the list view.
+    http_method_names = ["get"]
     template_name = "projects/project_translations.html"
 
-    def form_valid(self, form):
-        form.save()
-        return HttpResponseRedirect(self.get_success_url())
-
-    def get_form(self, data=None, files=None, **kwargs):
-        kwargs["parent"] = self.get_project()
-        kwargs["user"] = self.request.user
-        return self.form_class(data, files, **kwargs)
-
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
         project = self.get_project()
         context["lang_projects"] = project.translations.all()
         return context
 
 
+class ProjectTranslationsCreate(ProjectTranslationsMixin, CreateView):
+    template_name = "projects/project_translations_form.html"
+
+
 class ProjectTranslationsDelete(ProjectTranslationsMixin, GenericView):
     http_method_names = ["post"]
 

readthedocs/rtd_tests/tests/test_project.py

@@ -315,11 +315,11 @@ def test_user_can_add_own_project_as_translation(self):
         )
 
         self.client.login(username=user_a.username, password="test")
-        self.client.post(
-            reverse("projects_translations", args=[project_a.slug]),
+        r = self.client.post(
+            reverse("projects_translations_create", args=[project_a.slug]),
             data={"project": project_b.slug},
         )
-
+        self.assertEqual(r.status_code, 302)
         self.assertEqual(project_a.translations.first(), project_b)
         project_b.refresh_from_db()
         self.assertEqual(project_b.main_language_project, project_a)
@@ -344,11 +344,11 @@ def test_user_can_add_project_as_translation_if_is_owner(self):
         )
 
         self.client.login(username=user_a.username, password="test")
-        self.client.post(
-            reverse("projects_translations", args=[project_a.slug]),
+        r = self.client.post(
+            reverse("projects_translations_create", args=[project_a.slug]),
             data={"project": project_b.slug},
         )
-
+        self.assertEqual(r.status_code, 302)
         self.assertEqual(project_a.translations.first(), project_b)
 
     def test_user_can_not_add_other_user_project_as_translation(self):
@@ -372,9 +372,10 @@ def test_user_can_not_add_other_user_project_as_translation(self):
         # User A try to add project B as translation of project A
         self.client.login(username=user_a.username, password="test")
         resp = self.client.post(
-            reverse("projects_translations", args=[project_a.slug]),
+            reverse("projects_translations_create", args=[project_a.slug]),
             data={"project": project_b.slug},
         )
+        self.assertEqual(resp.status_code, 200)
 
         self.assertContains(resp, "Select a valid choice")
         self.assertEqual(project_a.translations.count(), 0)

readthedocs/rtd_tests/tests/test_project_views.py

@@ -14,7 +14,13 @@
 from readthedocs.oauth.models import RemoteRepository, RemoteRepositoryRelation
 from readthedocs.organizations.models import Organization
 from readthedocs.projects.constants import PUBLIC
-from readthedocs.projects.models import Domain, Project, WebHook, WebHookEvent
+from readthedocs.projects.models import (
+    Domain,
+    EmailHook,
+    Project,
+    WebHook,
+    WebHookEvent,
+)
 from readthedocs.projects.views.mixins import ProjectRelationMixin
 from readthedocs.projects.views.private import ImportWizardView
 from readthedocs.projects.views.public import ProjectBadgeView
@@ -593,6 +599,44 @@ def test_project_filtering_work_with_tags_with_space_in_name(self):
         self.assertContains(response, '"/projects/pip/"')
 
 
+@override_settings(RTD_ALLOW_ORGANIZATIONS=False)
+class TestProjectEmailNotifications(TestCase):
+    def setUp(self):
+        self.user = get(User)
+        self.project = get(Project, slug="test", users=[self.user])
+        self.version = get(Version, slug="1.0", project=self.project)
+        self.email_notification = get(EmailHook, project=self.project)
+        self.client.force_login(self.user)
+
+    def test_list(self):
+        resp = self.client.get(
+            reverse("projects_notifications", args=[self.project.slug]),
+        )
+        self.assertEqual(resp.status_code, 200)
+        queryset = resp.context["emails"]
+        self.assertEqual(queryset.count(), 1)
+        self.assertEqual(queryset.first(), self.email_notification)
+
+    def test_create(self):
+        self.assertEqual(self.project.emailhook_notifications.all().count(), 1)
+        resp = self.client.post(
+            reverse("projects_notifications_create", args=[self.project.slug]),
+            data={
+                "email": "test@example.com",
+            },
+        )
+        self.assertEqual(resp.status_code, 302)
+        self.assertEqual(self.project.emailhook_notifications.all().count(), 2)
+
+    def test_delete(self):
+        self.assertEqual(self.project.emailhook_notifications.all().count(), 1)
+        self.client.post(
+            reverse("projects_notification_delete", args=[self.project.slug]),
+            data={"email": self.email_notification.email},
+        )
+        self.assertEqual(self.project.emailhook_notifications.all().count(), 0)
+
+
 @override_settings(RTD_ALLOW_ORGANIZATIONS=False)
 class TestWebhooksViews(TestCase):
     def setUp(self):

readthedocs/templates/projects/project_notifications.html

@@ -73,7 +73,8 @@ <h3>{% trans "New Email Notifications" %}</h3>
   <p>
     {% trans "Add an email address to notify" %}
   </p>
-  <form method="post" action=".">{% csrf_token %}
+  <form method="post" action="{% url "projects_notifications_create" project.slug %}">
+    {% csrf_token %}
     {{ email_form }}
     <p>
       <input style="display: inline;" type="submit" value="{% trans "Add" %}">