Skip to content

Commit

Permalink
Notifications: de-duplicate them when using APIv2 from builders (#11197)
Browse files Browse the repository at this point in the history
* Notifications: de-duplicate them when using APIv2 from builders

Related #11131

* Update readthedocs/notifications/querysets.py

Co-authored-by: Santos Gallegos <stsewd@proton.me>

---------

Co-authored-by: Santos Gallegos <stsewd@proton.me>
  • Loading branch information
humitos and stsewd committed Mar 11, 2024
1 parent 2ab8c0b commit 535703c
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 1 deletion.
5 changes: 5 additions & 0 deletions readthedocs/api/v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,3 +427,8 @@ class NotificationSerializer(serializers.ModelSerializer):
class Meta:
model = Notification
exclude = ["attached_to_id", "attached_to_content_type"]

def create(self, validated_data):
# Override this method to allow de-duplication of notifications,
# by calling our custom ``.add()`` method that does this.
return Notification.objects.add(**validated_data)
6 changes: 6 additions & 0 deletions readthedocs/notifications/querysets.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ def add(self, *args, **kwargs):
).first()

if notification:
# Remove the fields we are overriding.
# Avoids passing these fields twice to ``.update()`` which
# raises an exception in that case.
kwargs.pop("state", None)
kwargs.pop("modified", None)

self.filter(pk=notification.pk).update(
*args,
modified=timezone.now(),
Expand Down
40 changes: 39 additions & 1 deletion readthedocs/rtd_tests/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@
LATEST,
)
from readthedocs.builds.models import APIVersion, Build, BuildCommandResult, Version
from readthedocs.doc_builder.exceptions import BuildUserError
from readthedocs.doc_builder.exceptions import BuildMaxConcurrencyError, BuildUserError
from readthedocs.integrations.models import GenericAPIWebhook, Integration
from readthedocs.notifications.constants import READ, UNREAD
from readthedocs.notifications.models import Notification
from readthedocs.oauth.models import (
RemoteOrganization,
Expand Down Expand Up @@ -1567,6 +1568,43 @@ def test_concurrent_builds(self):
self.assertEqual(resp.status_code, 200)
self.assertDictEqual(expected, resp.data)

def test_add_notification_deduplicated(self):
project = get(Project)
build = get(Build, project=project)
data = {
"attached_to": f"build/{build.pk}",
"message_id": BuildMaxConcurrencyError.LIMIT_REACHED,
"state": READ,
"dismissable": False,
"news": False,
"format_values": {"limit": 10},
}

url = "/api/v2/notifications/"

self.client.logout()

self.assertEqual(Notification.objects.count(), 0)
client = APIClient()
_, build_api_key = BuildAPIKey.objects.create_key(project)
client.credentials(HTTP_AUTHORIZATION=f"Token {build_api_key}")

response = client.post(url, data=data)
self.assertEqual(response.status_code, 201)
self.assertEqual(Notification.objects.count(), 1)
n1 = Notification.objects.first()

# Adding the same notification, de-duplicates it
response = client.post(url, data=data)
self.assertEqual(response.status_code, 201)
self.assertEqual(Notification.objects.count(), 1)
n2 = Notification.objects.first()

self.assertEqual(n1.pk, n2.pk)
self.assertEqual(n1.state, READ)
self.assertEqual(n2.state, UNREAD)
self.assertNotEqual(n1.modified, n2.modified)


class APIImportTests(TestCase):

Expand Down

0 comments on commit 535703c

Please sign in to comment.