Merge branch 'marker-fix' into 'develop'

Fix notification markers on Mastodon

Closes #1061

See merge request soapbox-pub/soapbox-fe!1706

app/soapbox/__tests__/compare_id.test.ts

@@ -0,0 +1,7 @@
+import compareId from '../compare_id';
+
+test('compareId', () => {
+  expect(compareId('3', '3')).toBe(0);
+  expect(compareId('10', '1')).toBe(1);
+  expect(compareId('99', '100')).toBe(-1);
+});

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;

dangerfile.ts

@@ -11,7 +11,7 @@ if (docs.edited) {
 const uiCode = danger.git.fileMatch('app/soapbox/components/ui/**');
 const uiTests = danger.git.fileMatch('app/soapbox/components/ui/**/__tests__/**');
 
-if (uiCode.modified && !uiTests.modified) {
+if (uiCode.edited && !uiTests.edited) {
   warn('You have UI changes (`soapbox/components/ui`) without tests.');
 }
 
@@ -19,7 +19,7 @@ if (uiCode.modified && !uiTests.modified) {
 const actionsCode = danger.git.fileMatch('app/soapbox/actions/**');
 const actionsTests = danger.git.fileMatch('app/soapbox/actions/**__tests__/**');
 
-if (actionsCode.modified && !actionsTests.modified) {
+if (actionsCode.edited && !actionsTests.edited) {
   warn('You have actions changes (`soapbox/actions`) without tests.');
 }
 
@@ -27,6 +27,6 @@ if (actionsCode.modified && !actionsTests.modified) {
 const reducersCode = danger.git.fileMatch('app/soapbox/reducers/**');
 const reducersTests = danger.git.fileMatch('app/soapbox/reducers/**__tests__/**');
 
-if (reducersCode.modified && !reducersTests.modified) {
+if (reducersCode.edited && !reducersTests.edited) {
   warn('You have reducer changes (`soapbox/reducers`) without tests.');
 }