Notifications: fix markers string ID comparison

app/soapbox/actions/__tests__/notifications.test.ts

@@ -0,0 +1,38 @@
+import { OrderedMap as ImmutableOrderedMap } from 'immutable';
+
+import { __stub } from 'soapbox/api';
+import { mockStore, rootState } from 'soapbox/jest/test-helpers';
+import { normalizeNotification } from 'soapbox/normalizers';
+
+import { markReadNotifications } from '../notifications';
+
+describe('markReadNotifications()', () => {
+  it('fires off marker when top notification is newer than lastRead', async() => {
+    __stub((mock) => mock.onPost('/api/v1/markers').reply(200, {}));
+
+    const items = ImmutableOrderedMap({
+      '10': normalizeNotification({ id: '10' }),
+    });
+
+    const state = rootState
+      .set('me', '123')
+      .setIn(['notifications', 'lastRead'], '9')
+      .setIn(['notifications', 'items'], items);
+
+    const store = mockStore(state);
+
+    const expectedActions =  [{
+      type: 'MARKER_SAVE_REQUEST',
+      marker: {
+        notifications: {
+          last_read_id: '10',
+        },
+      },
+    }];
+
+    store.dispatch(markReadNotifications());
+    const actions = store.getActions();
+
+    expect(actions).toEqual(expectedActions);
+  });
+});

app/soapbox/actions/notifications.ts

@@ -7,6 +7,7 @@ import 'intl-pluralrules';
 import { defineMessages } from 'react-intl';
 
 import api, { getLinks } from 'soapbox/api';
+import compareId from 'soapbox/compare_id';
 import { getFilters, regexFromFilters } from 'soapbox/selectors';
 import { isLoggedIn } from 'soapbox/utils/auth';
 import { getFeatures, parseVersion, PLEROMA } from 'soapbox/utils/features';
@@ -304,13 +305,11 @@ const markReadNotifications = () =>
     if (!isLoggedIn(getState)) return;
 
     const state = getState();
-    const instance = state.instance;
+    const topNotificationId: string | undefined = state.notifications.get('items').first(ImmutableMap()).get('id');
-    const topNotificationId = state.notifications.get('items').first(ImmutableMap()).get('id');
+    const lastReadId: string | -1 = state.notifications.get('lastRead');
-    const lastReadId = state.notifications.get('lastRead');
+    const v = parseVersion(state.instance.version);
-    const v = parseVersion(instance.version);
-
-    if (!(topNotificationId && topNotificationId > lastReadId)) return;
 
+    if (topNotificationId && (lastReadId === -1 || compareId(topNotificationId, lastReadId) > 0)) {
       const marker = {
         notifications: {
           last_read_id: topNotificationId,
@@ -322,6 +321,7 @@ const markReadNotifications = () =>
       if (v.software === PLEROMA) {
         dispatch(markReadPleroma(topNotificationId));
       }
+    }
   };
 
 export {

app/soapbox/compare_id.ts

@@ -1,5 +1,14 @@
 'use strict';
 
+/**
+ * Compare numerical primary keys represented as strings.
+ * For example, '10' (as a string) is considered less than '9'
+ * when sorted alphabetically. So compare string length first.
+ *
+ * - `0`: id1 == id2
+ * - `1`: id1 > id2
+ * - `-1`: id1 < id2
+ */
 export default function compareId(id1: string, id2: string) {
   if (id1 === id2) {
     return 0;