Skip to content

Some fixes for notifications #11093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions readthedocs/doc_builder/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,18 @@
Then the header/body texts should be defined in ``readthedocs/notifications/messages.py``.
"""

from django.utils.translation import gettext_lazy as _

from readthedocs.notifications.exceptions import NotificationBaseException

class BuildBaseException(Exception):

default_message = "Build user exception"
class BuildBaseException(NotificationBaseException):

def __init__(self, message_id, format_values=None, **kwargs):
self.message_id = message_id
self.format_values = format_values
super().__init__(self.default_message, **kwargs)
default_message = _("Build user exception")


class BuildAppError(BuildBaseException):
default_message = "Build app exception"
default_message = _("Build application exception")

GENERIC_WITH_BUILD_ID = "build:app:generic-with-build-id"
BUILDS_DISABLED = "build:app:project-builds-disabled"
Expand Down
18 changes: 18 additions & 0 deletions readthedocs/notifications/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
from django.utils.translation import gettext_lazy as _


class NotificationBaseException(Exception):

"""
The base exception class for notification and messages.

This provides the additional ``message_id`` and ``format_values`` attributes
that are used for message display and registry lookup.
"""

default_message = _("Undefined error")

def __init__(self, message_id, format_values=None, **kwargs):
self.message_id = message_id
self.format_values = format_values
super().__init__(self.default_message, **kwargs)
16 changes: 14 additions & 2 deletions readthedocs/notifications/messages.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import copy
import textwrap

import structlog
from django.template import Context, Template
from django.utils.translation import gettext_noop as _

from readthedocs.core.context_processors import readthedocs_processor
from readthedocs.doc_builder.exceptions import (
BuildAppError,
BuildCancelled,
Expand Down Expand Up @@ -514,8 +516,18 @@ def add(self, messages):
raise ValueError("A message with the same 'id' is already registered.")
self.messages[message.id] = message

def get(self, message_id):
return self.messages.get(message_id)
def get(self, message_id, format_values=None):
# Copy to avoid setting format values on the static instance of the
# message inside the registry, set on a per-request instance instead.
message = copy.copy(self.messages.get(message_id))

if message is not None:
# Always include global variables, override with provided values
all_format_values = readthedocs_processor(None)
all_format_values.update(format_values or {})
message.set_format_values(all_format_values)

return message


registry = MessagesRegistry()
Expand Down
19 changes: 6 additions & 13 deletions readthedocs/notifications/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
from django.utils.translation import gettext_noop as _
from django_extensions.db.models import TimeStampedModel

from readthedocs.core.context_processors import readthedocs_processor

from .constants import CANCELLED, DISMISSED, READ, UNREAD, WARNING
from .messages import Message, registry
Expand Down Expand Up @@ -67,7 +66,12 @@ def __str__(self):
return self.message_id

def get_message(self):
message = registry.get(self.message_id)
# Pass the instance attached to this notification
format_values = {
"instance": self.attached_to,
}

message = registry.get(self.message_id, format_values=format_values)
if message is None:
# Log the error and return an unknown error to avoid breaking the response.
log.error(
Expand All @@ -88,15 +92,4 @@ def get_message(self):
type=WARNING,
)

# Pass the instance attached to this notification
all_format_values = {
"instance": self.attached_to,
}

# Always include global variables
all_format_values.update(readthedocs_processor(None))

# Pass the values stored in the database
all_format_values.update(self.format_values or {})
message.set_format_values(all_format_values)
return message
18 changes: 18 additions & 0 deletions readthedocs/projects/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@


from readthedocs.doc_builder.exceptions import BuildAppError, BuildUserError
from readthedocs.notifications.exceptions import NotificationBaseException


class ProjectConfigurationError(BuildUserError):
Expand Down Expand Up @@ -32,3 +33,20 @@ 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"
45 changes: 45 additions & 0 deletions readthedocs/projects/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
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,
Expand Down Expand Up @@ -145,5 +147,48 @@
),
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=WARNING,
),
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=WARNING,
),
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=WARNING,
),
]
registry.add(messages)
57 changes: 54 additions & 3 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,15 @@
from readthedocs.core.notifications import MESSAGE_EMAIL_VALIDATION_PENDING
from readthedocs.integrations.models import HttpExchange, Integration
from readthedocs.invitations.models import Invitation
from readthedocs.notifications.messages import registry as messages_registry
from readthedocs.notifications.models import Notification
from readthedocs.oauth.services import registry
from readthedocs.oauth.tasks import attach_webhook
from readthedocs.oauth.utils import update_webhook
from readthedocs.projects.exceptions import (
ProjectAutomaticCreationDisallowed,
ProjectManualCreationDisallowed,
)
from readthedocs.projects.filters import ProjectListFilterSet
from readthedocs.projects.forms import (
AddonsConfigForm,
Expand Down Expand Up @@ -398,9 +403,55 @@ def post(self, request, *args, **kwargs):
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context['view_csrf_token'] = get_token(self.request)
context['has_connected_accounts'] = SocialAccount.objects.filter(
user=self.request.user,
).exists()

if settings.RTD_EXT_THEME_ENABLED:
error_import_manually = None
error_import_automatically = None

has_connected_account = SocialAccount.objects.filter(
user=self.request.user,
).exists()

# First check ability for automatic project creation
# NOTE that try catch is used here, but isn't super useful yet. But
# if it makes more sense to house this logic outside this view,
# somewhere central to organization/user modeling, then we don't
# have to change any code here.
try:
if has_connected_account:
raise ProjectAutomaticCreationDisallowed(
message_id=ProjectAutomaticCreationDisallowed.NO_CONNECTED_ACCOUNT,
format_values={
"url": reverse("socialaccount_connections"),
},
)
# TODO if organization sso enabled and user is not an owner
if True:
raise ProjectAutomaticCreationDisallowed(
message_id=ProjectAutomaticCreationDisallowed.SSO_ENABLED,
)
except ProjectAutomaticCreationDisallowed as exc:
error_import_automatically = messages_registry.get(
exc.message_id,
format_values=exc.format_values,
)

# Next check ability for manual project creation
try:
# TODO if organization sso enabled
if True:
raise ProjectManualCreationDisallowed(
ProjectManualCreationDisallowed.SSO_ENABLED
)
except ProjectManualCreationDisallowed as exc:
error_import_manually = messages_registry.get(
exc.message_id,
format_values=exc.format_values,
)

context["error_import_automatically"] = error_import_automatically
context["error_import_manually"] = error_import_manually

return context


Expand Down