Use form validation errors for important UI feedback

readthedocs/core/forms.py

@@ -5,6 +5,7 @@
 from crispy_forms.layout import Fieldset, Layout, Submit
 from django import forms
 from django.contrib.auth.models import User
+from django.core.exceptions import NON_FIELD_ERRORS
 from django.forms.fields import CharField
 from django.utils.translation import gettext_lazy as _
 
@@ -98,6 +99,91 @@ class Meta:
         fields = ["allow_ads"]
 
 
+class PrevalidatedForm(forms.Form):
+
+    """
+    Form class that allows raising form errors before form submission.
+
+    The base ``Form`` does not support validation errors while the form is
+    unbound (does not have ``data`` defined). There are cases in our UI where we
+    want to show errors and/or disabled the form before the user has a chance to
+    interact with the form -- for example, when a feature is unavailable or
+    disabled for the user or organization.
+
+    This provides the ``clean_prevalidation`` method, which acts much like the
+    ``clean`` method. Any validation errors raised in this method surface as non
+    field errors in the UI.
+    """
+
+    @property
+    def is_valid(self):
+        # This differs from ``Form`` in that we don't care if the form is bound
+        return not self.errors
+
+    @property
+    def is_disabled(self):
+        return self._prevalidation_errors is not None
+
+    def full_clean(self):
+        """
+        Extend full clean method with prevalidation cleaning.
+
+        Where :py:method:`forms.Form.full_clean` bails out if there is no bound
+        data on the form, this method always checks prevalidation no matter
+        what. This gives errors before submission and after submission.
+        """
+        # Always call prevalidation, ``full_clean`` bails if the form is unbound
+        self._prevalidation_errors = None
+        self._clean_prevalidation()
+
+        super().full_clean()
+
+        # ``full_clean`` sets ``self._errors``, so we prepend prevalidation
+        # errors after calling the parent ``full_clean``
+        if self._prevalidation_errors is not None:
+            non_field_errors = []
+            non_field_errors.extend(self._prevalidation_errors)
+            non_field_errors.extend(self._errors.get(NON_FIELD_ERRORS, []))
+            self._errors[NON_FIELD_ERRORS] = non_field_errors
+
+    def _clean_prevalidation(self):
+        """
+        Catch validation errors raised by the subclassed ``clean_validation()``.
+
+        This wraps ``clean_prevalidation()`` using the same pattern that
+        :py:method:`form.Form._clean_form` wraps :py:method:`clean`. An validation
+        errors raised in the subclass method will be eventually added to the
+        form error list but :py:method:`full_clean`.
+        """
+        try:
+            self.clean_prevalidation()
+        except forms.ValidationError as validation_error:
+            self._prevalidation_errors = [validation_error]
+
+
+class RichValidationError(forms.ValidationError):
+
+    """
+    Show non-field form errors as titled messages.
+
+    This uses more of the FUI message specification to give a really clear,
+    concise error message to the user. Without this class, non-field validation
+    errors show at the top of the form with a title "Error", which isn't as
+    helpful to users as something like "Connected service required".
+
+    :param header str: Message header/title text
+    :param message_class str: FUI CSS class to use on the message -- "info",
+        etc. Default: "error".
+    """
+
+    def __init__(
+        self, message, code=None, params=None, header=None, message_class=None
+    ):
+        super().__init__(message, code, params)
+        self.header = header
+        self.message_class = message_class
+
+
 class FacetField(forms.MultipleChoiceField):
 
     """

readthedocs/projects/exceptions.py

@@ -2,7 +2,6 @@
 
 
 from readthedocs.doc_builder.exceptions import BuildAppError, BuildUserError
-from readthedocs.notifications.exceptions import NotificationBaseException
 
 
 class ProjectConfigurationError(BuildUserError):
@@ -33,20 +32,3 @@ class SyncRepositoryLocked(BuildAppError):
     """Error risen when there is another sync_repository_task already running."""
 
     REPOSITORY_LOCKED = "project:repository:locked"
-
-
-class ProjectAutomaticCreationDisallowed(NotificationBaseException):
-
-    """Used for raising messages in the project automatic create form view"""
-
-    NO_CONNECTED_ACCOUNT = "project:create:automatic:no-connected-account"
-    SSO_ENABLED = "project:create:automatic:sso-enabled"
-    INADEQUATE_PERMISSIONS = "project:create:automatic:inadequate-permissions"
-
-
-class ProjectManualCreationDisallowed(NotificationBaseException):
-
-    """Used for raising messages in the project manual create form view"""
-
-    SSO_ENABLED = "project:create:manual:sso-enabled"
-    INADEQUATE_PERMISSIONS = "project:create:manual:inadequate-permissions"

readthedocs/projects/forms.py

@@ -4,6 +4,7 @@
 from re import fullmatch
 from urllib.parse import urlparse
 
+from allauth.socialaccount.models import SocialAccount
 from django import forms
 from django.conf import settings
 from django.contrib.auth.models import User
@@ -12,12 +13,15 @@
 from django.utils.translation import gettext_lazy as _
 
 from readthedocs.builds.constants import INTERNAL
+from readthedocs.core.forms import PrevalidatedForm, RichValidationError
 from readthedocs.core.history import SimpleHistoryModelForm
+from readthedocs.core.permissions import AdminPermission
 from readthedocs.core.utils import slugify, trigger_build
 from readthedocs.core.utils.extend import SettingsOverrideObject
 from readthedocs.integrations.models import Integration
 from readthedocs.invitations.models import Invitation
 from readthedocs.oauth.models import RemoteRepository
+from readthedocs.organizations.models import Team
 from readthedocs.projects.models import (
     AddonsConfig,
     Domain,
@@ -78,6 +82,109 @@ class ProjectBackendForm(forms.Form):
     backend = forms.CharField()
 
 
+class ProjectFormParamMixin:
+    def __init__(self, *args, **kwargs):
+        self.user = kwargs.pop("user", None)
+        super().__init__(*args, **kwargs)
+
+    def clean_prevalidation(self):
+        # Shared conditionals between automatic and manual forms
+        self.user_has_connected_account = SocialAccount.objects.filter(
+            user=self.user,
+        ).exists()
+        self.user_is_nonowner_with_sso = None
+        self.user_missing_admin_permission = None
+        if settings.RTD_ALLOW_ORGANIZATIONS:
+            # TODO there should be some way to initially select the organization
+            # and maybe the team too. It's mostly safe to automatically select
+            # the first organization, but explicit would be better. Reusing the
+            # organization selection UI works, we only really need a query param
+            # here.
+            self.user_is_nonowner_with_sso = all(
+                [
+                    AdminPermission.has_sso_enabled(self.user),
+                    AdminPermission.organizations(
+                        user=self.user,
+                        owner=False,
+                    ).exists(),
+                ]
+            )
+
+            # TODO this logic should be possible from AdminPermission
+            # AdminPermssion.is_admin only inspects organization owners, so the
+            # additional team check is necessary
+            self.user_missing_admin_permission = not any(
+                [
+                    AdminPermission.organizations(
+                        user=self.user,
+                        owner=True,
+                    ).exists(),
+                    Team.objects.admin(self.user).exists(),
+                ]
+            )
+
+
+class ProjectAutomaticForm(ProjectFormParamMixin, PrevalidatedForm):
+    def clean_prevalidation(self):
+        """
+        Block user from using this form for important blocking states.
+
+        We know before the user gets a chance to use this form that the user
+        might not have the ability to add a project into their organization.
+        These errors are raised before the user submits the form.
+        """
+        super().clean_prevalidation()
+        if not self.user_has_connected_account:
+            url = reverse("socialaccount_connections")
+            raise RichValidationError(
+                _(
+                    f"You must first <a href='{url}'>add a connected service "
+                    f"to your account</a> to enable automatic configuration of "
+                    f"repositories."
+                ),
+                header=_("No connected services found"),
+            )
+        if settings.RTD_ALLOW_ORGANIZATIONS:
+            if self.user_is_nonowner_with_sso:
+                raise RichValidationError(
+                    _(
+                        "Only organization owners may create new projects "
+                        "when single sign-on is enabled."
+                    ),
+                    header=_("Organization single sign-on enabled"),
+                )
+            if self.user_missing_admin_permission:
+                raise RichValidationError(
+                    _(
+                        "You must be on a team with admin permissions "
+                        "in order to add a new project to your organization."
+                    ),
+                    header=_("Admin permission required"),
+                )
+
+
+class ProjectManualForm(ProjectFormParamMixin, PrevalidatedForm):
+    def clean_prevalidation(self):
+        super().clean_prevalidation()
+        if settings.RTD_ALLOW_ORGANIZATIONS:
+            if self.user_is_nonowner_with_sso:
+                raise RichValidationError(
+                    _(
+                        "Projects cannot be manually configured when "
+                        "single sign-on is enabled for your organization."
+                    ),
+                    header=_("Organization single sign-on enabled"),
+                )
+            if self.user_missing_admin_permission:
+                raise RichValidationError(
+                    _(
+                        "You must be on a team with admin permissions "
+                        "in order to add a new project to your organization."
+                    ),
+                    header=_("Admin permission required"),
+                )
+
+
 class ProjectBasicsForm(ProjectForm):
 
     """Form for basic project fields."""

readthedocs/projects/notifications.py

@@ -6,9 +6,7 @@
 from readthedocs.notifications.constants import ERROR, INFO, WARNING
 from readthedocs.notifications.messages import Message, registry
 from readthedocs.projects.exceptions import (
-    ProjectAutomaticCreationDisallowed,
     ProjectConfigurationError,
-    ProjectManualCreationDisallowed,
     RepositoryError,
     SyncRepositoryLocked,
     UserFileNotFound,
@@ -147,86 +145,5 @@
         ),
         type=WARNING,
     ),
-    # The following messages are added to the registry but are only used
-    # directly by the project creation view template. These could be split out
-    # directly for use by import, instead of reusing message id lookup.
-    Message(
-        id=ProjectAutomaticCreationDisallowed.NO_CONNECTED_ACCOUNT,
-        header=_("No connected services found"),
-        body=_(
-            textwrap.dedent(
-                # Translators: "connected service" refers to the user setting page for "Connected Services"
-                """
-                You must first <a href="{{ url }}">add a connected service to your account</a>
-                to enable automatic configuration of repositories.
-                """
-            ).strip(),
-        ),
-        type=ERROR,
-    ),
-    Message(
-        id=ProjectAutomaticCreationDisallowed.SSO_ENABLED,
-        header=_("Organization single sign-on enabled"),
-        body=_(
-            textwrap.dedent(
-                """
-                Only organization owners may create new projects when single
-                sign-on is enabled.
-                """
-            ).strip(),
-        ),
-        type=ERROR,
-    ),
-    Message(
-        id=ProjectAutomaticCreationDisallowed.NO_CONNECTED_ACCOUNT,
-        header=_("No connected services found"),
-        body=_(
-            textwrap.dedent(
-                # Translators: "connected service" refers to the user setting page for "Connected Services"
-                """
-                You must first <a href="{{ url }}">add a connected service to your account</a>
-                to enable automatic configuration of repositories.
-                """
-            ).strip(),
-        ),
-        type=ERROR,
-    ),
-    Message(
-        id=ProjectAutomaticCreationDisallowed.INADEQUATE_PERMISSIONS,
-        header=_("Admin permission required"),
-        body=_(
-            textwrap.dedent(
-                """
-                You must be on a team with admin permissions to add a new project.
-                """
-            ).strip(),
-        ),
-        type=ERROR,
-    ),
-    # Same as above but for manual import
-    Message(
-        id=ProjectManualCreationDisallowed.SSO_ENABLED,
-        header=_("Organization single sign-on enabled"),
-        body=_(
-            textwrap.dedent(
-                """
-                Projects cannot be manually configured when single sign-on is enabled.
-                """
-            ).strip(),
-        ),
-        type=ERROR,
-    ),
-    Message(
-        id=ProjectManualCreationDisallowed.INADEQUATE_PERMISSIONS,
-        header=_("Admin permission required"),
-        body=_(
-            textwrap.dedent(
-                """
-                You must be on a team with admin permissions to add a new project.
-                """
-            ).strip(),
-        ),
-        type=ERROR,
-    ),
 ]
 registry.add(messages)