Skip to content

Commit 1cf32db

Browse files
committed
Initial example of using notification system for view-specific errors
I found myself rebuilding these messages in template code and figured it would be a good excuse to further consolidate the patterns we're using. In short, these messages are emitted at view time and are not saved to the database. The view time logic is similar as notifications however. A few questions stand for me: - I like centering all of our notifications/errors around exceptions, as it retains an opportunity to emit exceptions from wherever we might want to move some of this logic. Arguably, this logic should not exist on a single view and could be moved on to modeling/queryset classes. - I don't have a strong opinion on whether it's important for these messages to exist in the registry. I see arguments for both, but I chose to use the registry so that we can consolidate logic there -- format values and message lookup, etc. I'm looking for input on these patterns. On the template side, the display is now just a simple `if` and template logic similar to the current notification display, using the header/body render methods.
1 parent bee983d commit 1cf32db

File tree

3 files changed

+117
-3
lines changed

3 files changed

+117
-3
lines changed

readthedocs/projects/exceptions.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33

44
from readthedocs.doc_builder.exceptions import BuildAppError, BuildUserError
5+
from readthedocs.notifications.exceptions import NotificationBaseException
56

67

78
class ProjectConfigurationError(BuildUserError):
@@ -32,3 +33,20 @@ class SyncRepositoryLocked(BuildAppError):
3233
"""Error risen when there is another sync_repository_task already running."""
3334

3435
REPOSITORY_LOCKED = "project:repository:locked"
36+
37+
38+
class ProjectAutomaticCreationDisallowed(NotificationBaseException):
39+
40+
"""Used for raising messages in the project automatic create form view"""
41+
42+
NO_CONNECTED_ACCOUNT = "project:create:automatic:no-connected-account"
43+
SSO_ENABLED = "project:create:automatic:sso-enabled"
44+
INADEQUATE_PERMISSIONS = "project:create:automatic:inadequate-permissions"
45+
46+
47+
class ProjectManualCreationDisallowed(NotificationBaseException):
48+
49+
"""Used for raising messages in the project manual create form view"""
50+
51+
SSO_ENABLED = "project:create:manual:sso-enabled"
52+
INADEQUATE_PERMISSIONS = "project:create:manual:inadequate-permissions"

readthedocs/projects/notifications.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
from readthedocs.notifications.constants import ERROR, INFO, WARNING
77
from readthedocs.notifications.messages import Message, registry
88
from readthedocs.projects.exceptions import (
9+
ProjectAutomaticCreationDisallowed,
910
ProjectConfigurationError,
11+
ProjectManualCreationDisallowed,
1012
RepositoryError,
1113
SyncRepositoryLocked,
1214
UserFileNotFound,
@@ -145,5 +147,48 @@
145147
),
146148
type=WARNING,
147149
),
150+
# The following messages are added to the registry but are only used
151+
# directly by the project creation view template. These could be split out
152+
# directly for use by import, instead of reusing message id lookup.
153+
Message(
154+
id=ProjectAutomaticCreationDisallowed.NO_CONNECTED_ACCOUNT,
155+
header=_("No connected services found"),
156+
body=_(
157+
textwrap.dedent(
158+
# Translators: "connected service" refers to the user setting page for "Connected Services"
159+
"""
160+
You must first <a href="{{ url }}">add a connected service to your account</a>
161+
to enable automatic configuration of repositories.
162+
"""
163+
).strip(),
164+
),
165+
type=WARNING,
166+
),
167+
Message(
168+
id=ProjectAutomaticCreationDisallowed.SSO_ENABLED,
169+
header=_("Organization single sign-on enabled"),
170+
body=_(
171+
textwrap.dedent(
172+
"""
173+
Only organization owners may create new projects when single
174+
sign-on is enabled.
175+
"""
176+
).strip(),
177+
),
178+
type=WARNING,
179+
),
180+
Message(
181+
id=ProjectManualCreationDisallowed.SSO_ENABLED,
182+
header=_("Organization single sign-on enabled"),
183+
body=_(
184+
textwrap.dedent(
185+
"""
186+
Projects cannot be manually configured when single sign-on is
187+
enabled.
188+
"""
189+
).strip(),
190+
),
191+
type=WARNING,
192+
),
148193
]
149194
registry.add(messages)

readthedocs/projects/views/private.py

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,15 @@
4242
from readthedocs.core.notifications import MESSAGE_EMAIL_VALIDATION_PENDING
4343
from readthedocs.integrations.models import HttpExchange, Integration
4444
from readthedocs.invitations.models import Invitation
45+
from readthedocs.notifications.messages import registry as messages_registry
4546
from readthedocs.notifications.models import Notification
4647
from readthedocs.oauth.services import registry
4748
from readthedocs.oauth.tasks import attach_webhook
4849
from readthedocs.oauth.utils import update_webhook
50+
from readthedocs.projects.exceptions import (
51+
ProjectAutomaticCreationDisallowed,
52+
ProjectManualCreationDisallowed,
53+
)
4954
from readthedocs.projects.filters import ProjectListFilterSet
5055
from readthedocs.projects.forms import (
5156
AddonsConfigForm,
@@ -397,9 +402,55 @@ def post(self, request, *args, **kwargs):
397402
def get_context_data(self, **kwargs):
398403
context = super().get_context_data(**kwargs)
399404
context['view_csrf_token'] = get_token(self.request)
400-
context['has_connected_accounts'] = SocialAccount.objects.filter(
401-
user=self.request.user,
402-
).exists()
405+
406+
if settings.RTD_EXT_THEME_ENABLED:
407+
error_import_manually = None
408+
error_import_automatically = None
409+
410+
has_connected_account = SocialAccount.objects.filter(
411+
user=self.request.user,
412+
).exists()
413+
414+
# First check ability for automatic project creation
415+
# NOTE that try catch is used here, but isn't super useful yet. But
416+
# if it makes more sense to house this logic outside this view,
417+
# somewhere central to organization/user modeling, then we don't
418+
# have to change any code here.
419+
try:
420+
if has_connected_account:
421+
raise ProjectAutomaticCreationDisallowed(
422+
message_id=ProjectAutomaticCreationDisallowed.NO_CONNECTED_ACCOUNT,
423+
format_values={
424+
"url": reverse("socialaccount_connections"),
425+
},
426+
)
427+
# TODO if organization sso enabled and user is not an owner
428+
if True:
429+
raise ProjectAutomaticCreationDisallowed(
430+
message_id=ProjectAutomaticCreationDisallowed.SSO_ENABLED,
431+
)
432+
except ProjectAutomaticCreationDisallowed as exc:
433+
error_import_automatically = messages_registry.get(
434+
exc.message_id,
435+
format_values=exc.format_values,
436+
)
437+
438+
# Next check ability for manual project creation
439+
try:
440+
# TODO if organization sso enabled
441+
if True:
442+
raise ProjectManualCreationDisallowed(
443+
ProjectManualCreationDisallowed.SSO_ENABLED
444+
)
445+
except ProjectManualCreationDisallowed as exc:
446+
error_import_manually = messages_registry.get(
447+
exc.message_id,
448+
format_values=exc.format_values,
449+
)
450+
451+
context["error_import_automatically"] = error_import_automatically
452+
context["error_import_manually"] = error_import_manually
453+
403454
return context
404455

405456

0 commit comments

Comments
 (0)