From 64ade99fe19803f195c352696e95766cca20ae3b Mon Sep 17 00:00:00 2001 From: Kiran Jonnalagadda Date: Fri, 18 Sep 2020 04:14:10 +0530 Subject: [PATCH] Use Base58 eventids and move queries into model --- funnel/assets/js/notification_list.js | 10 ++- funnel/models/notification.py | 80 ++++++++++++++++++- .../templates/notification_feed.html.jinja2 | 2 +- funnel/views/notification.py | 4 +- funnel/views/notification_feed.py | 56 +++++-------- 5 files changed, 105 insertions(+), 47 deletions(-) diff --git a/funnel/assets/js/notification_list.js b/funnel/assets/js/notification_list.js index f1d61d950..73ad9a6d9 100644 --- a/funnel/assets/js/notification_list.js +++ b/funnel/assets/js/notification_list.js @@ -50,14 +50,16 @@ const Notification = { addNotifications(notifications, refresh) { notifications.forEach((notice) => { if ( - !notificationApp.eventids.includes(notice.notification.eventid) + !notificationApp.eventids.includes( + notice.notification.eventid_b58 + ) ) { if (refresh) { notificationApp.notifications.unshift(notice); } else { notificationApp.notifications.push(notice); } - notificationApp.eventids.push(notice.notification.eventid); + notificationApp.eventids.push(notice.notification.eventid_b58); } }); }, @@ -87,8 +89,8 @@ const Notification = { $(notification).attr('data-index') ]; let url = this.markReadUrl.replace( - 'eventid', - notificationItem.notification.eventid + 'eventid_b58', + notificationItem.notification.eventid_b58 ); $.ajax({ type: 'POST', diff --git a/funnel/models/notification.py b/funnel/models/notification.py index ddca09a2e..07094811a 100644 --- a/funnel/models/notification.py +++ b/funnel/models/notification.py @@ -83,8 +83,18 @@ from werkzeug.utils import cached_property from baseframe import __ -from coaster.sqlalchemy import auto_init_default, immutable, with_roles -from coaster.utils import LabeledEnum, classmethodproperty +from coaster.sqlalchemy import ( + SqlUuidB58Comparator, + auto_init_default, + immutable, + with_roles, +) +from coaster.utils import ( + LabeledEnum, + classmethodproperty, + uuid_from_base58, + uuid_to_base58, +) from . import BaseMixin, NoIdMixin, UUIDType, db from .helpers import reopen @@ -300,6 +310,7 @@ class Notification(NoIdMixin, db.Model): __datasets__ = { 'primary': { 'eventid', + 'eventid_b58', 'document_type', 'fragment_type', 'document', @@ -309,6 +320,7 @@ class Notification(NoIdMixin, db.Model): }, 'related': { 'eventid', + 'eventid_b58', 'document_type', 'fragment_type', 'document', @@ -374,6 +386,19 @@ def identity(self): """Primary key of this object.""" return (self.eventid, self.id) + @hybrid_property + def eventid_b58(self): + """URL-friendly UUID representation, using Base58 with the Bitcoin alphabet""" + return uuid_to_base58(self.eventid) + + @eventid_b58.setter + def eventid_b58(self, value): + self.eventid = uuid_from_base58(value) + + @eventid_b58.comparator + def eventid_b58(cls): # NOQA: N805 + return SqlUuidB58Comparator(cls.eventid) + @with_roles(read={'all'}) @classmethodproperty def document_type(cls): # NOQA: N805 @@ -495,7 +520,7 @@ class PreviewNotification: """ def __init__(self, cls, document, fragment=None): - self.eventid = self.id = 'preview' # May need to be a UUID + self.eventid = self.eventid_b58 = self.id = 'preview' # May need to be a UUID self.cls = cls self.document = document self.document_uuid = document.uuid @@ -628,6 +653,7 @@ class UserNotification(UserNotificationMixin, NoIdMixin, db.Model): 'primary': { 'created_at', 'eventid', + 'eventid_b58', 'role', 'read_at', 'is_read', @@ -638,6 +664,7 @@ class UserNotification(UserNotificationMixin, NoIdMixin, db.Model): 'related': { 'created_at', 'eventid', + 'eventid_b58', 'role', 'read_at', 'is_read', @@ -653,6 +680,21 @@ def identity(self): """Primary key of this object.""" return (self.user_id, self.eventid) + @hybrid_property + def eventid_b58(self): + """URL-friendly UUID representation, using Base58 with the Bitcoin alphabet""" + return uuid_to_base58(self.eventid) + + @eventid_b58.setter + def eventid_b58(self, value): + self.eventid = uuid_from_base58(value) + + @eventid_b58.comparator + def eventid_b58(cls): # NOQA: N805 + return SqlUuidB58Comparator(cls.eventid) + + with_roles(eventid_b58, read={'owner'}) + @hybrid_property def is_read(self): """Whether this notification has been marked as read.""" @@ -849,6 +891,38 @@ def rolledup_fragments(self): ) ) + @classmethod + def get_for(cls, user, eventid_b58): + """ + Helper method to retrieve a UserNotification using SQLAlchemy session cache. + """ + return cls.query.get((user.id, uuid_from_base58(eventid_b58))) + + @classmethod + def web_notifications_for(cls, user): + return ( + UserNotification.query.join(Notification) + .filter( + Notification.type.in_(notification_web_types), + UserNotification.user == user, + UserNotification.revoked_at.is_(None), + ) + .order_by(Notification.created_at.desc()) + ) + + @classmethod + def unread_count_for(cls, user): + return ( + UserNotification.query.join(Notification) + .filter( + Notification.type.in_(notification_web_types), + UserNotification.user == user, + UserNotification.read_at.is_(None), + UserNotification.revoked_at.is_(None), + ) + .count() + ) + @classmethod def migrate_user(cls, old_user, new_user): for user_notification in cls.query.filter_by(user_id=old_user.id).all(): diff --git a/funnel/templates/notification_feed.html.jinja2 b/funnel/templates/notification_feed.html.jinja2 index 3bbeb2281..c3c65e390 100644 --- a/funnel/templates/notification_feed.html.jinja2 +++ b/funnel/templates/notification_feed.html.jinja2 @@ -45,7 +45,7 @@