Skip to content

Some fixes for notifications #11094

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

Merged
merged 1 commit into from
Feb 6, 2024
Merged
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