Use Base58 eventids and move queries into model

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',

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():

funnel/templates/notification_feed.html.jinja2

@@ -45,7 +45,7 @@
   <script type="text/javascript">
     $(function() {
       var notificationConfig = {
-        markReadUrl: {{ url_for('notification_mark_read', eventid='eventid')|tojson }},
+        markReadUrl: {{ url_for('notification_mark_read', eventid_b58='eventid_b58')|tojson }},
         divElem: '#notifications',
       };
 

funnel/views/notification.py

@@ -110,7 +110,7 @@ def tracking_tags(self, transport=None, campaign=None):
             'utm_medium': transport or 'email',
         }
         if not self.notification.for_private_recipient:
-            tags['utm_source'] = self.notification.eventid
+            tags['utm_source'] = self.notification.eventid_b58
         return tags
 
     def unsubscribe_token(self, transport):
@@ -163,7 +163,7 @@ def unsubscribe_short_url(self, transport='sms'):
                 'notification_type': self.notification.type,
                 'transport': transport,
                 'hash': self.transport_for(transport).transport_hash,
-                'eventid': self.notification.eventid,
+                'eventid_b58': self.notification.eventid_b58,
                 'timestamp': datetime.utcnow(),  # Naive timestamp
             },
             timeout=14 * 24 * 60 * 60,  # Reserve generated token for 14 days

funnel/views/notification_feed.py

@@ -1,11 +1,11 @@
-from flask import redirect
+from flask import abort, redirect
 
 from coaster.auth import current_auth
 from coaster.views import ClassView, render_with, requestargs, route
 import baseframe.forms as forms
 
 from .. import app, funnelapp, lastuserapp
-from ..models import Notification, UserNotification, db, notification_web_types
+from ..models import UserNotification, db
 from .helpers import app_url_for
 from .login_session import requires_login
 
@@ -19,15 +19,8 @@ class AllNotificationsView(ClassView):
     @render_with('notification_feed.html.jinja2', json=True)
     @requestargs(('page', int), ('per_page', int))
     def view(self, page=1, per_page=10):
-        pagination = (
-            UserNotification.query.join(Notification)
-            .filter(
-                Notification.type.in_(notification_web_types),
-                UserNotification.user == current_auth.user,
-                UserNotification.revoked_at.is_(None),
-            )
-            .order_by(Notification.created_at.desc())
-            .paginate(page=page, per_page=per_page, max_per_page=100)
+        pagination = UserNotification.web_notifications_for(current_auth.user).paginate(
+            page=page, per_page=per_page, max_per_page=100
         )
         return {
             'show_transport_alert': not current_auth.user.has_transport_sms(),
@@ -61,16 +54,7 @@ def view(self, page=1, per_page=10):
         }
 
     def unread_count(self):
-        return (
-            UserNotification.query.join(Notification)
-            .filter(
-                Notification.type.in_(notification_web_types),
-                UserNotification.user == current_auth.user,
-                UserNotification.read_at.is_(None),
-                UserNotification.revoked_at.is_(None),
-            )
-            .count()
-        )
+        UserNotification.unread_count_for(current_auth.user)
 
     @route('count', endpoint='notifications_count')
     @render_with(json=True)
@@ -84,39 +68,37 @@ def unread(self):
             }
         return {'status': 'error', 'error': 'requires_login'}, 400
 
-    @route('mark_read/<eventid>', endpoint='notification_mark_read', methods=['POST'])
+    @route(
+        'mark_read/<eventid_b58>', endpoint='notification_mark_read', methods=['POST']
+    )
     @requires_login
     @render_with(json=True)
-    def mark_read(self, eventid):
-        # TODO: Use Base58 ids
+    def mark_read(self, eventid_b58):
         form = forms.Form()
         del form.form_nonce
         if form.validate_on_submit():
-            # TODO: Use query.get((user_id, eventid)) and do manual 404
-            un = UserNotification.query.filter(
-                UserNotification.user == current_auth.user,
-                UserNotification.eventid == eventid,
-            ).one_or_404()
+            un = UserNotification.get_for(current_auth.user, eventid_b58)
+            if not un:
+                abort(404)
             un.is_read = True
             db.session.commit()
             return {'status': 'ok', 'unread': self.unread_count()}
         return {'status': 'error', 'error': 'csrf'}, 400
 
     @route(
-        'mark_unread/<eventid>', endpoint='notification_mark_unread', methods=['POST']
+        'mark_unread/<eventid_b58>',
+        endpoint='notification_mark_unread',
+        methods=['POST'],
     )
     @requires_login
     @render_with(json=True)
-    def mark_unread(self, eventid):
-        # TODO: Use Base58 ids
+    def mark_unread(self, eventid_b58):
         form = forms.Form()
         del form.form_nonce
         if forms.validate_on_submit():
-            # TODO: Use query.get((user_id, eventid)) and do manual 404
-            un = UserNotification.query.filter(
-                UserNotification.user == current_auth.user,
-                UserNotification.eventid == eventid,
-            ).one_or_404()
+            un = UserNotification.get_for(current_auth.user, eventid_b58)
+            if not un:
+                abort(404)
             un.is_read = False
             db.session.commit()
             return {'status': 'ok', 'unread': self.unread_count()}