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

Use custom data model for storing notifications #3464

Open
mathjazz opened this issue Dec 2, 2024 · 0 comments
Open

Use custom data model for storing notifications #3464

mathjazz opened this issue Dec 2, 2024 · 0 comments
Labels
notifications P3 Default, possibly shipping in the following two quarters task

Comments

@mathjazz
Copy link
Collaborator

mathjazz commented Dec 2, 2024

We need to store additional data for handling notifications (e.g categories).

There's a couple ways to achieve that:

  1. Use existing django-notifications library, which supports creating custom data models. We've hit some issues with this approach, see below.
  2. Create our own data model and drop django-notifications. That would also allow for other optimizations. For example, the current model creates a separate instance of the Notification model for each user, including all the content and metadata.

This is a past attempt to override the existing model from django-notifications:

diff --git a/pontoon/base/models/__init__.py b/pontoon/base/models/__init__.py
index f095bdf61..4f6012455 100644
--- a/pontoon/base/models/__init__.py
+++ b/pontoon/base/models/__init__.py
@@ -4,6 +4,7 @@ from pontoon.base.models.comment import Comment
 from pontoon.base.models.entity import Entity, get_word_count
 from pontoon.base.models.external_resource import ExternalResource
 from pontoon.base.models.locale import Locale, LocaleCodeHistory, validate_cldr
+from pontoon.base.models.notification import Notification
 from pontoon.base.models.permission_changelog import PermissionChangelog
 from pontoon.base.models.project import Priority, Project, ProjectSlugHistory
 from pontoon.base.models.project_locale import ProjectLocale
@@ -25,6 +26,7 @@ __all__ = [
     "ExternalResource",
     "Locale",
     "LocaleCodeHistory",
+    "Notification",
     "PermissionChangelog",
     "Priority",
     "Project",
diff --git a/pontoon/base/models/notification.py b/pontoon/base/models/notification.py
new file mode 100644
index 000000000..cff0a7bac
--- /dev/null
+++ b/pontoon/base/models/notification.py
@@ -0,0 +1,21 @@
+from django.db import models
+from notifications.base.models import AbstractNotification
+
+
+class Notification(AbstractNotification):
+    class Categories(models.TextChoices):
+        NEW_STRINGS = "new_strings", "new_strings"
+        TARGET_DATES = "target_dates", "target_dates"
+        NEW_COMMENTS = "new_comments", "new_comments"
+        NEW_SUGGESTIONS = "new_suggestions", "new_suggestions"
+        NEW_REVIEWS = "new_reviews", "new_reviews"
+        NEW_CONTRIBUTORS = "new_contributors", "new_contributors"
+        MESSAGING_CENTER = "messaging_center", "messaging_center"
+
+    category = models.CharField(
+        max_length=20,
+        choices=Categories.choices,
+    )
+
+    class Meta(AbstractNotification.Meta):
+        abstract = False
diff --git a/pontoon/settings/base.py b/pontoon/settings/base.py
index f09bcf729..d553cf74a 100644
--- a/pontoon/settings/base.py
+++ b/pontoon/settings/base.py
@@ -1102,6 +1102,7 @@ SOCIALACCOUNT_PROVIDERS = {
 }

 # Configuration of `django-notifications-hq` app
+NOTIFICATIONS_NOTIFICATION_MODEL = "pontoon.base.models.Notification"
 DJANGO_NOTIFICATIONS_CONFIG = {
     # Attach extra arguments passed to notify.send(...) to the .data attribute
     # of the Notification object.

Which failed with the following error:

pontoon@d0168d836c37:/app$ ./manage.py showmigrations
SystemCheckError: System check identified some issues:

ERRORS:
base.Notification.action_object_content_type: (fields.E304) Reverse accessor 'ContentType.notify_action_object' for 'base.Notification.action_object_content_type' clashes with reverse accessor for 'notifications.Notification.action_object_content_type'.
	HINT: Add or change a related_name argument to the definition for 'base.Notification.action_object_content_type' or 'notifications.Notification.action_object_content_type'.
base.Notification.action_object_content_type: (fields.E305) Reverse query name for 'base.Notification.action_object_content_type' clashes with reverse query name for 'notifications.Notification.action_object_content_type'.
	HINT: Add or change a related_name argument to the definition for 'base.Notification.action_object_content_type' or 'notifications.Notification.action_object_content_type'.
base.Notification.actor_content_type: (fields.E304) Reverse accessor 'ContentType.notify_actor' for 'base.Notification.actor_content_type' clashes with reverse accessor for 'notifications.Notification.actor_content_type'.
	HINT: Add or change a related_name argument to the definition for 'base.Notification.actor_content_type' or 'notifications.Notification.actor_content_type'.
base.Notification.actor_content_type: (fields.E305) Reverse query name for 'base.Notification.actor_content_type' clashes with reverse query name for 'notifications.Notification.actor_content_type'.
	HINT: Add or change a related_name argument to the definition for 'base.Notification.actor_content_type' or 'notifications.Notification.actor_content_type'.
base.Notification.recipient: (fields.E304) Reverse accessor 'User.notifications' for 'base.Notification.recipient' clashes with reverse accessor for 'notifications.Notification.recipient'.
	HINT: Add or change a related_name argument to the definition for 'base.Notification.recipient' or 'notifications.Notification.recipient'.
base.Notification.recipient: (fields.E305) Reverse query name for 'base.Notification.recipient' clashes with reverse query name for 'notifications.Notification.recipient'.
	HINT: Add or change a related_name argument to the definition for 'base.Notification.recipient' or 'notifications.Notification.recipient'.
base.Notification.target_content_type: (fields.E304) Reverse accessor 'ContentType.notify_target' for 'base.Notification.target_content_type' clashes with reverse accessor for 'notifications.Notification.target_content_type'.
	HINT: Add or change a related_name argument to the definition for 'base.Notification.target_content_type' or 'notifications.Notification.target_content_type'.
base.Notification.target_content_type: (fields.E305) Reverse query name for 'base.Notification.target_content_type' clashes with reverse query name for 'notifications.Notification.target_content_type'.
	HINT: Add or change a related_name argument to the definition for 'base.Notification.target_content_type' or 'notifications.Notification.target_content_type'.
notifications.Notification.action_object_content_type: (fields.E304) Reverse accessor 'ContentType.notify_action_object' for 'notifications.Notification.action_object_content_type' clashes with reverse accessor for 'base.Notification.action_object_content_type'.
	HINT: Add or change a related_name argument to the definition for 'notifications.Notification.action_object_content_type' or 'base.Notification.action_object_content_type'.
notifications.Notification.action_object_content_type: (fields.E305) Reverse query name for 'notifications.Notification.action_object_content_type' clashes with reverse query name for 'base.Notification.action_object_content_type'.
	HINT: Add or change a related_name argument to the definition for 'notifications.Notification.action_object_content_type' or 'base.Notification.action_object_content_type'.
notifications.Notification.actor_content_type: (fields.E304) Reverse accessor 'ContentType.notify_actor' for 'notifications.Notification.actor_content_type' clashes with reverse accessor for 'base.Notification.actor_content_type'.
	HINT: Add or change a related_name argument to the definition for 'notifications.Notification.actor_content_type' or 'base.Notification.actor_content_type'.
notifications.Notification.actor_content_type: (fields.E305) Reverse query name for 'notifications.Notification.actor_content_type' clashes with reverse query name for 'base.Notification.actor_content_type'.
	HINT: Add or change a related_name argument to the definition for 'notifications.Notification.actor_content_type' or 'base.Notification.actor_content_type'.
notifications.Notification.recipient: (fields.E304) Reverse accessor 'User.notifications' for 'notifications.Notification.recipient' clashes with reverse accessor for 'base.Notification.recipient'.
	HINT: Add or change a related_name argument to the definition for 'notifications.Notification.recipient' or 'base.Notification.recipient'.
notifications.Notification.recipient: (fields.E305) Reverse query name for 'notifications.Notification.recipient' clashes with reverse query name for 'base.Notification.recipient'.
	HINT: Add or change a related_name argument to the definition for 'notifications.Notification.recipient' or 'base.Notification.recipient'.
notifications.Notification.target_content_type: (fields.E304) Reverse accessor 'ContentType.notify_target' for 'notifications.Notification.target_content_type' clashes with reverse accessor for 'base.Notification.target_content_type'.
	HINT: Add or change a related_name argument to the definition for 'notifications.Notification.target_content_type' or 'base.Notification.target_content_type'.
notifications.Notification.target_content_type: (fields.E305) Reverse query name for 'notifications.Notification.target_content_type' clashes with reverse query name for 'base.Notification.target_content_type'.
	HINT: Add or change a related_name argument to the definition for 'notifications.Notification.target_content_type' or 'base.Notification.target_content_type'.
pontoon@d0168d836c37:/app$
@mathjazz mathjazz added task P3 Default, possibly shipping in the following two quarters notifications labels Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notifications P3 Default, possibly shipping in the following two quarters task
Projects
None yet
Development

No branches or pull requests

1 participant