Skip to content
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

New notification system: implementation #10922

Merged
merged 103 commits into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from 85 commits
Commits
Show all changes
103 commits
Select commit Hold shift + click to select a range
92447e3
Add some comments to understand the scope of the changes
humitos Nov 23, 2023
a7cb5ad
Add more comments about the plan
humitos Nov 23, 2023
fb79264
Delete old code
humitos Nov 23, 2023
780e28b
Add more comments to organize the work of new notification system
humitos Nov 23, 2023
749622c
Notification: initial modelling
humitos Nov 23, 2023
7777960
Define `max_length=128` that's required for SQLite (tests)
humitos Nov 27, 2023
105e7c8
Initial implementation for build notifications API endpoint
humitos Nov 27, 2023
cbf0e54
API endpoint to create build notifications
humitos Nov 27, 2023
584c055
Use APIv2 for internal endpoint to call from builders
humitos Nov 27, 2023
3118eea
Use APIv2 to create notifications from the builder
humitos Nov 28, 2023
03ca9a4
Check project slug when attaching to a Build/Project instance
humitos Nov 29, 2023
dd8fb31
Add extra TODO comments
humitos Nov 29, 2023
514c8f9
Initial migration from Exceptions to Messages
humitos Nov 29, 2023
e9d0c02
Disable `Notification.format_values` for now
humitos Nov 29, 2023
0c01d76
Define all the notifications
humitos Nov 29, 2023
36de5f8
Messages for symlink exceptions
humitos Nov 29, 2023
e05592a
Simplify how `NOTIFICATION_MESSAGES` is built
humitos Nov 29, 2023
9f9fcfb
Add `Notification.format_values` field
humitos Nov 29, 2023
b7600ce
Notification with `format_values` passed when exception is raised
humitos Dec 13, 2023
fa27fb1
Use `get_display_icon` to show the proper icon in the serializer
humitos Dec 13, 2023
1dc0f6f
Proper relation name
humitos Dec 13, 2023
27425fd
Initial work to replace site notifications
humitos Dec 13, 2023
05e7e31
We are not going to create Notifications via APIv3 for now
humitos Dec 13, 2023
fa11b1a
Send `format_values` to exception
humitos Dec 13, 2023
10d5c9c
Small changes and comments added
humitos Dec 13, 2023
4e00365
Initial work to migrate an old SiteNotification to new system
humitos Dec 13, 2023
4e8d63d
Create a notification `registry` to register messages by application
humitos Dec 13, 2023
ff43348
Use a custom `models.Manager` to implement de-duplication
humitos Dec 13, 2023
15c8517
Small refactor to email notifications
humitos Dec 13, 2023
82637ec
Render build/project notifications from Django templates
humitos Dec 14, 2023
18c9067
Attach "build.commands" in beta notification
humitos Dec 14, 2023
7af4e87
Initial migration pattern for YAML config errors
humitos Dec 14, 2023
98447fc
Pass default settings for notifications
humitos Dec 14, 2023
130cff9
Organization disabled notification
humitos Dec 14, 2023
da3c503
Email not verified notification
humitos Dec 14, 2023
e214eb9
Project skipped notification
humitos Dec 14, 2023
fef0e01
Missing file for NotificationQuerySet
humitos Dec 14, 2023
b27c038
Migrate `oauth` app notifications to the new system
humitos Dec 14, 2023
8bc8eca
Add small comment about notification migration
humitos Dec 14, 2023
5137152
Rename import
humitos Dec 14, 2023
e619b96
Fix queryset when creating a notification via `.add()`
humitos Dec 14, 2023
ab09850
Remove `SiteNotification` completely
humitos Dec 14, 2023
482a415
Rename file and remove `__init__` magic
humitos Dec 14, 2023
2be5731
Remove `django-message-extends` completely
humitos Dec 14, 2023
10199b9
Note about user notifications in base template
humitos Dec 14, 2023
75bcc86
Remove commented code for debugging
humitos Dec 14, 2023
6d0f90e
Typo (from review)
humitos Dec 19, 2023
87c1e03
Migrate `ConfigError` to the new notification system
humitos Dec 19, 2023
51435e1
Implement refact `icon` and `icon_type` into `icon_classes`
humitos Dec 19, 2023
65212a1
Remove unused setting
humitos Dec 19, 2023
23105c8
Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…
humitos Dec 19, 2023
41b9993
Update config file test cases to match changes
humitos Dec 20, 2023
3c1dcb5
Mark test that depend on search to be skipped
humitos Dec 20, 2023
17953c6
Cancel build test fixed
humitos Dec 20, 2023
33806ee
Update APIv3 test cases
humitos Dec 20, 2023
ff844c9
Down to a few of tests failing
humitos Dec 20, 2023
201584b
Raise exception properly by passing `message_id`
humitos Dec 20, 2023
ead8ab1
More tests updated/fixed
humitos Dec 20, 2023
3e9c293
BuildUser/BuildAdd error
humitos Dec 20, 2023
8d50fc5
Instantiate `BuildDirector` on `before_start`
humitos Dec 20, 2023
f46ea00
Update test case to pass
humitos Dec 20, 2023
2617d02
Update more tests related to notifications
humitos Dec 20, 2023
6f3c13d
Make usage of `.add` for deduplication
humitos Dec 20, 2023
ecd270a
Move the link to the message itself.
humitos Dec 20, 2023
6549a3f
Create message IDs property
humitos Dec 20, 2023
78c3bd7
Use IDs to avoid notifications
humitos Dec 20, 2023
9a22ba6
Update comments
humitos Dec 20, 2023
4562969
Attach organization on retry
humitos Dec 20, 2023
3bcde18
Test we are hitting APIv2 for notifications
humitos Dec 20, 2023
8712ffd
Remove old comment
humitos Dec 20, 2023
f55a4f7
Remove temporary debugging link
humitos Dec 20, 2023
d869a71
Remove old comment
humitos Dec 20, 2023
ee657b3
Update send build status celery tests
humitos Dec 20, 2023
47762ed
Explain how to show notifications attached to users
humitos Dec 20, 2023
1472ab0
Better comment in template
humitos Dec 20, 2023
aa96583
Lint
humitos Dec 20, 2023
1cb8128
Minor changes
humitos Dec 20, 2023
db8dd38
Refactor `ValidationError` to raise a proper exception/notification
humitos Dec 21, 2023
738b3bf
Update tests for config after refactor
humitos Dec 21, 2023
3a6e8ea
Pass `user_notifications` to templates
humitos Dec 21, 2023
212bbdb
More updates on config tests
humitos Dec 21, 2023
2c4b99d
Remove debugging code
humitos Dec 21, 2023
05285e4
Add context processor for user notifications
humitos Dec 21, 2023
8b925b4
Don't retrieve notifications if anonymous user
humitos Dec 21, 2023
0a765de
Use proper exception
humitos Dec 21, 2023
57c26e1
Apply suggestions from code review
humitos Jan 3, 2024
623190d
Remove old TODO messages
humitos Jan 3, 2024
30b0830
Remove `deprecated_` utils and notifications
humitos Jan 3, 2024
7d1daee
Use the proper HTML/CSS for user notifications
humitos Jan 3, 2024
a930ce5
Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…
humitos Jan 3, 2024
6f16258
Message for deploy key (required in commercial)
humitos Jan 3, 2024
bf85137
Migrate Docker errors to new system
humitos Jan 3, 2024
ac624b0
Migrate more messages to the new notifications system
humitos Jan 3, 2024
d532c53
Typo
humitos Jan 3, 2024
35b64cc
Fix error type
humitos Jan 3, 2024
d4951a8
Remove showing errors from `build.error`
humitos Jan 3, 2024
e711d8f
Typo in copy
humitos Jan 3, 2024
5f15af7
Revert "Remove showing errors from `build.error`"
humitos Jan 4, 2024
2047a57
Comment explaining the decision about `Build.error` HTML+KO code
humitos Jan 4, 2024
4c71772
Use `textwrap.dedent` + `.strip()` for all messages
humitos Jan 4, 2024
36db237
Avoid breaking at rendering time
humitos Jan 4, 2024
8828a4b
Apply feedback from review
humitos Jan 4, 2024
e7b83e0
Lint
humitos Jan 4, 2024
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
2 changes: 1 addition & 1 deletion common
54 changes: 54 additions & 0 deletions readthedocs/api/v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@


from allauth.socialaccount.models import SocialAccount
from django.core.exceptions import ObjectDoesNotExist
from django.utils.translation import gettext as _
from generic_relations.relations import GenericRelatedField
from rest_framework import serializers

from readthedocs.api.v2.utils import normalize_build_command
from readthedocs.builds.models import Build, BuildCommandResult, Version
from readthedocs.core.resolver import Resolver
from readthedocs.notifications.models import Notification
from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
from readthedocs.projects.models import Domain, Project

Expand Down Expand Up @@ -378,3 +382,53 @@ def get_username(self, obj):
or obj.extra_data.get("login")
# FIXME: which one is GitLab?
)


class NotificationAttachedToRelatedField(serializers.RelatedField):

"""
Attached to related field for Notifications.
Used together with ``rest-framework-generic-relations`` to accept multiple object types on ``attached_to``.
See https://github.com/LilyFoote/rest-framework-generic-relations
"""

default_error_messages = {
"required": _("This field is required."),
"does_not_exist": _("Object does not exist."),
"incorrect_type": _(
"Incorrect type. Expected URL string, received {data_type}."
),
}

def to_representation(self, value):
return f"{self.queryset.model._meta.model_name}/{value.pk}"

def to_internal_value(self, data):
# TODO: handle exceptions
model, pk = data.strip("/").split("/")
if self.queryset.model._meta.model_name != model:
self.fail("incorrect_type")

try:
return self.queryset.get(pk=pk)
except (ObjectDoesNotExist, ValueError, TypeError):
self.fail("does_not_exist")


class NotificationSerializer(serializers.ModelSerializer):
# Accept different object types (Project, Build, User, etc) depending on what the notification is attached to.
# The client has to send a value like "<model>/<pk>".
# Example: "build/3522" will attach the notification to the Build object with id 3522
agjohnson marked this conversation as resolved.
Show resolved Hide resolved
attached_to = GenericRelatedField(
{
Build: NotificationAttachedToRelatedField(queryset=Build.objects.all()),
Project: NotificationAttachedToRelatedField(queryset=Project.objects.all()),
},
required=True,
)

class Meta:
model = Notification
exclude = ["attached_to_id", "attached_to_content_type"]
2 changes: 2 additions & 0 deletions readthedocs/api/v2/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
BuildCommandViewSet,
BuildViewSet,
DomainViewSet,
NotificationViewSet,
ProjectViewSet,
RemoteOrganizationViewSet,
RemoteRepositoryViewSet,
Expand All @@ -25,6 +26,7 @@
router.register(r"version", VersionViewSet, basename="version")
router.register(r"project", ProjectViewSet, basename="project")
router.register(r"domain", DomainViewSet, basename="domain")
router.register(r"notifications", NotificationViewSet, basename="notifications")
router.register(
r"remote/org",
RemoteOrganizationViewSet,
Expand Down
37 changes: 37 additions & 0 deletions readthedocs/api/v2/views/model_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from readthedocs.api.v2.utils import normalize_build_command
from readthedocs.builds.constants import INTERNAL
from readthedocs.builds.models import Build, BuildCommandResult, Version
from readthedocs.notifications.models import Notification
from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
from readthedocs.oauth.services import registry
from readthedocs.projects.models import Domain, Project
Expand All @@ -29,6 +30,7 @@
BuildCommandSerializer,
BuildSerializer,
DomainSerializer,
NotificationSerializer,
ProjectAdminSerializer,
ProjectSerializer,
RemoteOrganizationSerializer,
Expand Down Expand Up @@ -361,6 +363,41 @@ def get_queryset_for_api_key(self, api_key):
return self.model.objects.filter(build__project=api_key.project)


class NotificationViewSet(DisableListEndpoint, CreateModelMixin, UserSelectViewSet):

"""
Create a notification attached to an object (User, Project, Build, Organization).

This endpoint is currently used only internally by the builder.
Notifications are attached to `Build` objects only when using this endpoint.
This limitation will change in the future when re-implementing this on APIv3 if neeed.
"""

parser_classes = [JSONParser, MultiPartParser]
permission_classes = [HasBuildAPIKey | ReadOnlyPermission]
humitos marked this conversation as resolved.
Show resolved Hide resolved
renderer_classes = (JSONRenderer,)
serializer_class = NotificationSerializer
model = Notification

def perform_create(self, serializer):
"""Restrict creation to notifications attached to the project's builds from the api key."""
attached_to = serializer.validated_data["attached_to"]

build_api_key = self.request.build_api_key

if isinstance(attached_to, Build):
project_slug = attached_to.project.slug
elif isinstance(attached_to, Project):
project_slug = attached_to.slug

# Limit the permissions to create a notification on this object only if the API key
# is attached to the related project
if not project_slug or not build_api_key.project.slug == project_slug:
humitos marked this conversation as resolved.
Show resolved Hide resolved
raise PermissionDenied()

return super().perform_create(serializer)


class DomainViewSet(DisableListEndpoint, UserSelectViewSet):
permission_classes = [ReadOnlyPermission]
renderer_classes = (JSONRenderer,)
Expand Down
85 changes: 85 additions & 0 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from readthedocs.core.resolver import Resolver
from readthedocs.core.utils import slugify
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.notifications.models import Notification
from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
from readthedocs.organizations.models import Organization, Team
from readthedocs.projects.constants import (
Expand Down Expand Up @@ -59,10 +60,33 @@ class Meta:
fields = []


# TODO: decide whether or not include a `_links` field on the object
humitos marked this conversation as resolved.
Show resolved Hide resolved
#
# This also includes adding `/api/v3/notifications/<pk>` endpoint,
# which I'm not sure it's useful at this point.
humitos marked this conversation as resolved.
Show resolved Hide resolved
#
# class NotificationLinksSerializer(BaseLinksSerializer):
# _self = serializers.SerializerMethodField()
# attached_to = serializers.SerializerMethodField()

# def get__self(self, obj):
# path = reverse(
# "notifications-detail",
# kwargs={
# "pk": obj.pk,
# },
# )
# return self._absolute_url(path)

# def get_attached_to(self, obj):
# return None


class BuildLinksSerializer(BaseLinksSerializer):
_self = serializers.SerializerMethodField()
version = serializers.SerializerMethodField()
project = serializers.SerializerMethodField()
notifications = serializers.SerializerMethodField()

def get__self(self, obj):
path = reverse(
Expand Down Expand Up @@ -95,6 +119,16 @@ def get_project(self, obj):
)
return self._absolute_url(path)

def get_notifications(self, obj):
path = reverse(
"project-builds-notifications-list",
kwargs={
"parent_lookup_project__slug": obj.project.slug,
"parent_lookup_build__id": obj.pk,
},
)
return self._absolute_url(path)


class BuildURLsSerializer(BaseLinksSerializer, serializers.Serializer):
build = serializers.URLField(source="get_full_url")
Expand Down Expand Up @@ -189,6 +223,57 @@ def get_success(self, obj):
return None


class NotificationMessageSerializer(serializers.Serializer):
id = serializers.SlugField()
header = serializers.CharField(source="get_rendered_header")
body = serializers.CharField(source="get_rendered_body")
type = serializers.CharField()
icon_classes = serializers.CharField(source="get_display_icon_classes")

class Meta:
fields = [
"id",
"header",
"body",
"type",
"icon_classes",
]


class NotificationCreateSerializer(serializers.ModelSerializer):
class Meta:
model = Notification
fields = [
"message_id",
"dismissable",
"news",
"state",
]


class NotificationSerializer(serializers.ModelSerializer):
message = NotificationMessageSerializer(source="get_message")
attached_to_content_type = serializers.SerializerMethodField()
# TODO: review these fields
# _links = BuildLinksSerializer(source="*")
# urls = BuildURLsSerializer(source="*")

class Meta:
model = Notification
fields = [
"id",
"state",
"dismissable",
"news",
"attached_to_content_type",
"attached_to_id",
"message",
]

def get_attached_to_content_type(self, obj):
return obj.attached_to_content_type.name


class VersionLinksSerializer(BaseLinksSerializer):
_self = serializers.SerializerMethodField()
builds = serializers.SerializerMethodField()
Expand Down
1 change: 1 addition & 0 deletions readthedocs/api/v3/tests/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
)
class APIEndpointMixin(TestCase):
fixtures = []
maxDiff = None # So we get an actual diff when it fails

def setUp(self):
self.created = make_aware(datetime.datetime(2019, 4, 29, 10, 0, 0))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"_links": {
"_self": "https://readthedocs.org/api/v3/projects/project/builds/1/",
"project": "https://readthedocs.org/api/v3/projects/project/",
"version": "https://readthedocs.org/api/v3/projects/project/versions/v1.0/"
"version": "https://readthedocs.org/api/v3/projects/project/versions/v1.0/",
"notifications": "https://readthedocs.org/api/v3/projects/project/builds/1/notifications/"
},
"urls": {
"build": "https://readthedocs.org/projects/project/builds/1/",
Expand Down
1 change: 1 addition & 0 deletions readthedocs/api/v3/tests/responses/projects-detail.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"id": 1,
"_links": {
"_self": "https://readthedocs.org/api/v3/projects/project/builds/1/",
"notifications": "https://readthedocs.org/api/v3/projects/project/builds/1/notifications/",
"project": "https://readthedocs.org/api/v3/projects/project/",
"version": "https://readthedocs.org/api/v3/projects/project/versions/v1.0/"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"id": 2,
"_links": {
"_self": "https://readthedocs.org/api/v3/projects/project/builds/2/",
"notifications": "https://readthedocs.org/api/v3/projects/project/builds/2/notifications/",
"project": "https://readthedocs.org/api/v3/projects/project/",
"version": "https://readthedocs.org/api/v3/projects/project/versions/v1.0/"
},
Expand Down
19 changes: 17 additions & 2 deletions readthedocs/api/v3/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
BuildsCreateViewSet,
BuildsViewSet,
EnvironmentVariablesViewSet,
NotificationsViewSet,
ProjectsViewSet,
RedirectsViewSet,
RemoteOrganizationViewSet,
Expand All @@ -12,7 +13,6 @@
VersionsViewSet,
)


router = DefaultRouterWithNesting()

# allows /api/v3/projects/
Expand Down Expand Up @@ -63,13 +63,28 @@

# allows /api/v3/projects/pip/builds/
# allows /api/v3/projects/pip/builds/1053/
projects.register(
builds = projects.register(
r"builds",
BuildsViewSet,
basename="projects-builds",
parents_query_lookups=["project__slug"],
)

# NOTE: we are only listing notifications on APIv3 for now.
# The front-end will use this endpoint.
# allows /api/v3/projects/pip/builds/1053/notifications/
builds.register(
r"notifications",
NotificationsViewSet,
basename="project-builds-notifications",
parents_query_lookups=["project__slug", "build__id"],
)

# TODO: create an APIv3 endpoint to PATCH Build/Project notifications.
humitos marked this conversation as resolved.
Show resolved Hide resolved
# This way the front-end can mark them as READ/DISMISSED.
#
# TODO: create an APIv3 endpoint to list notifications for Projects.
humitos marked this conversation as resolved.
Show resolved Hide resolved

# allows /api/v3/projects/pip/redirects/
# allows /api/v3/projects/pip/redirects/1053/
projects.register(
Expand Down
29 changes: 29 additions & 0 deletions readthedocs/api/v3/views.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import django_filters.rest_framework as filters
from django.contrib.contenttypes.models import ContentType
from django.db.models import Exists, OuterRef
from rest_flex_fields import is_expanded
from rest_flex_fields.views import FlexFieldsMixin
Expand All @@ -23,6 +24,7 @@
from readthedocs.builds.models import Build, Version
from readthedocs.core.utils import trigger_build
from readthedocs.core.utils.extend import SettingsOverrideObject
from readthedocs.notifications.models import Notification
from readthedocs.oauth.models import (
RemoteOrganization,
RemoteRepository,
Expand Down Expand Up @@ -62,6 +64,7 @@
BuildCreateSerializer,
BuildSerializer,
EnvironmentVariableSerializer,
NotificationSerializer,
OrganizationSerializer,
ProjectCreateSerializer,
ProjectSerializer,
Expand Down Expand Up @@ -381,6 +384,32 @@ def create(self, request, **kwargs): # pylint: disable=arguments-differ
return Response(data=data, status=code)


class NotificationsViewSet(
APIv3Settings,
NestedViewSetMixin,
ProjectQuerySetMixin,
FlexFieldsMixin,
ReadOnlyModelViewSet,
):
model = Notification
lookup_field = "pk"
lookup_url_kwarg = "notification_pk"
serializer_class = NotificationSerializer
queryset = Notification.objects.all()
# filterset_class = BuildFilter

# http://chibisov.github.io/drf-extensions/docs/#usage-with-generic-relations
def get_queryset(self):
queryset = self.queryset.filter(
attached_to_content_type=ContentType.objects.get_for_model(Build)
)

# TODO: make sure if this particular filter should be applied here or somewhere else.
return queryset.filter(
attached_to_id=self.kwargs.get("parent_lookup_build__id")
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I'm following things the same way you are, but what is this going to look like when we add notification look up by project, for example? Are you planning on a separate view set class, or can we use independent query params to distinguish a lookup for build/project/etc notifications?

If the plan is a separate class, perhaps this class should be renamed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can be flexible here. We can have 1) one API endpoint per resource or 2) only one global endpoint with a filter per resource.

I started doing 1) as a test because is what DRF extensions allows to do and it's a pattern we have been following for other endpoints too. Also, I think it's easier to understand what notifications you are getting back. An example of the request would be:

  • Notifications for a Build: /api/v3/project/docs/builds/123456/notifications/
  • Notifications for a Project: /api/v3/project/docs/notifications/
  • etc..

The particular view where you commented (NotificationsViewSet) would cover 2) --it's not finished, and can probably be commented out/removed for now-- and would require the user sending the ?attached_to=build and also ?attached_to_id=123456 so the endpoint knows what's the exact object to retrieve notifications for: /api/v3/notifications/?attached_to=build&attached_to_id=123456

I didn't finish this because I wanted to discuss a little more with you before making the final decision. I personally prefer 1) since it's a cleaner UX and it's already working 😄 . Do you have any preference here?

However, the view NotificationsViewSet may also be required for a different usage: mark a specific notification object as read via the URL: PATCH /api/v3/notifications/718273/. So, we may keep it for that purpose.

We don't have to decide this right now, since it's outside the scope of this PR, tho.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened an issue for this at #10984

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll follow along that issue, but to note here:

I personally prefer 1) since it's a cleaner UX and it's already working

I think I prefer this too. It doesn't make much difference to the front end as we need to resolve the API URL and pass that into the JS view either way.

However, the view NotificationsViewSet may also be required for a different usage

Good point. On the front end, the API should give a URL for dismissing the notification in the API response, so the JS doesn't have to reconstruct this. So either way works here too, and it wouldn't be any more/less work to PATCH a deeper URL like /api/v3/project/docs/builds/123456/notifications/789



class RedirectsViewSet(
APIv3Settings,
NestedViewSetMixin,
Expand Down
Loading