Merge branch 'switch-to-string-ids' into favorites

with some changes/merge conflicts resolution

* switch-to-string-ids:
  fixx?????
  fix notifications?
  fix lint
  fix tests, removed one unused function, fix real problem that tests helped to surface
  added some more explicit to string conversion since BE seem to be sending numbers and it could cause an issue.
  Remove all explicit and implicit conversions of statusId to number, changed explicit ones so that they convert them to string

src/components/conversation-page/conversation-page.js

@@ -1,5 +1,5 @@
 import Conversation from '../conversation/conversation.vue'
-import { find, toInteger } from 'lodash'
+import { find } from 'lodash'
 
 const conversationPage = {
   components: {
@@ -7,7 +7,7 @@ const conversationPage = {
   },
   computed: {
     statusoid () {
-      const id = toInteger(this.$route.params.id)
+      const id = String(this.$route.params.id)
       const statuses = this.$store.state.statuses.allStatuses
       const status = find(statuses, {id})
 

src/components/conversation/conversation.js

@@ -33,7 +33,7 @@ const conversation = {
     replies () {
       let i = 1
       return reduce(this.conversation, (result, {id, in_reply_to_status_id}) => {
-        const irid = Number(in_reply_to_status_id)
+        const irid = String(in_reply_to_status_id)
         if (irid) {
           result[irid] = result[irid] || []
           result[irid].push({
@@ -70,7 +70,7 @@ const conversation = {
       }
     },
     getReplies (id) {
-      id = Number(id)
+      id = String(id)
       return this.replies[id] || []
     },
     focused (id) {
@@ -81,7 +81,7 @@ const conversation = {
       }
     },
     setHighlight (id) {
-      this.highlight = Number(id)
+      this.highlight = String(id)
     }
   }
 }

src/components/status/status.js

@@ -270,7 +270,7 @@ const Status = {
     },
     replyEnter (id, event) {
       this.showPreview = true
-      const targetId = Number(id)
+      const targetId = id
       const statuses = this.$store.state.statuses.allStatuses
 
       if (!this.preview) {
@@ -295,7 +295,6 @@ const Status = {
   },
   watch: {
     'highlight': function (id) {
-      id = Number(id)
       if (this.status.id === id) {
         let rect = this.$el.getBoundingClientRect()
         if (rect.top < 100) {

src/modules/statuses.js

@@ -1,4 +1,4 @@
-import { remove, slice, sortBy, toInteger, each, find, flatten, maxBy, minBy, merge, last, isArray } from 'lodash'
+import { remove, slice, each, find, maxBy, minBy, merge, last, isArray } from 'lodash'
 import apiService from '../services/api/api.service.js'
 // import parse from '../services/status_parser/status_parser.js'
 
@@ -62,11 +62,10 @@ const visibleNotificationTypes = (rootState) => {
   ].filter(_ => _)
 }
 
-export const findMaxId = (...args) => {
-  return (maxBy(flatten(args), 'id') || {}).id
-}
-
 const mergeOrAdd = (arr, obj, item) => {
+  // For sequential IDs BE passes numbers as numbers, we want them as strings.
+  item.id = String(item.id)
+
   const oldItem = obj[item.id]
 
   if (oldItem) {
@@ -84,9 +83,11 @@ const mergeOrAdd = (arr, obj, item) => {
   }
 }
 
+const sortById = (a, b) => a.id > b.id ? -1 : 1
+
 const sortTimeline = (timeline) => {
-  timeline.visibleStatuses = sortBy(timeline.visibleStatuses, ({id}) => -id)
+  timeline.visibleStatuses = timeline.visibleStatuses.sort(sortById)
-  timeline.statuses = sortBy(timeline.statuses, ({id}) => -id)
+  timeline.statuses = timeline.statuses.sort(sortById)
   timeline.minVisibleId = (last(timeline.visibleStatuses) || {}).id
   return timeline
 }
@@ -162,7 +163,7 @@ const addNewStatuses = (state, { statuses, showImmediately = false, timeline, us
   }
 
   const favoriteStatus = (favorite, counter) => {
-    const status = find(allStatuses, { id: toInteger(favorite.in_reply_to_status_id) })
+    const status = find(allStatuses, { id: String(favorite.in_reply_to_status_id) })
     if (status) {
       // This is our favorite, so the relevant bit.
       if (favorite.user.id === user.id) {
@@ -258,8 +259,12 @@ const addNewNotifications = (state, { dispatch, notifications, older, visibleNot
 
     // Only add a new notification if we don't have one for the same action
     if (!state.notifications.idStore.hasOwnProperty(notification.id)) {
-      state.notifications.maxId = Math.max(notification.id, state.notifications.maxId)
+      state.notifications.maxId = notification.id > state.notifications.maxId
-      state.notifications.minId = Math.min(notification.id, state.notifications.minId)
+        ? notification.id
+        : state.notifications.maxId
+      state.notifications.minId = notification.id < state.notifications.minId
+        ? notification.id
+        : state.notifications.minId
 
       state.notifications.data.push(notification)
       state.notifications.idStore[notification.id] = notification

src/services/notification_utils/notification_utils.js

@@ -10,8 +10,8 @@ export const visibleTypes = store => ([
 ].filter(_ => _))
 
 export const visibleNotificationsFromStore = store => {
-  // Don't know why, but sortBy([seen, -action.id]) doesn't work.
+  // map is just to clone the array since sort mutates it and it causes some issues
-  let sortedNotifications = sortBy(notificationsFromStore(store), ({action}) => -action.id)
+  let sortedNotifications = notificationsFromStore(store).map(_ => _).sort((a, b) => a.action.id > b.action.id ? -1 : 1)
   sortedNotifications = sortBy(sortedNotifications, 'seen')
   return sortedNotifications.filter((notification) => visibleTypes(store).includes(notification.type))
 }

test/unit/specs/modules/statuses.spec.js

@@ -1,11 +1,11 @@
 import { cloneDeep } from 'lodash'
-import { defaultState, mutations, findMaxId, prepareStatus } from '../../../../src/modules/statuses.js'
+import { defaultState, mutations, prepareStatus } from '../../../../src/modules/statuses.js'
 
 // eslint-disable-next-line camelcase
 const makeMockStatus = ({id, text, type = 'status'}) => {
   return {
     id,
-    user: {id: 0},
+    user: {id: '0'},
     name: 'status',
     text: text || `Text number ${id}`,
     fave_num: 0,
@@ -17,30 +17,15 @@ const makeMockStatus = ({id, text, type = 'status'}) => {
 
 describe('Statuses.prepareStatus', () => {
   it('sets deleted flag to false', () => {
-    const aStatus = makeMockStatus({id: 1, text: 'Hello oniichan'})
+    const aStatus = makeMockStatus({id: '1', text: 'Hello oniichan'})
     expect(prepareStatus(aStatus).deleted).to.eq(false)
   })
 })
 
-describe('Statuses.findMaxId', () => {
-  it('returns the largest id in any of the given arrays', () => {
-    const statusesOne = [{ id: 100 }, { id: 2 }]
-    const statusesTwo = [{ id: 3 }]
-
-    const maxId = findMaxId(statusesOne, statusesTwo)
-    expect(maxId).to.eq(100)
-  })
-
-  it('returns undefined for empty arrays', () => {
-    const maxId = findMaxId([], [])
-    expect(maxId).to.eq(undefined)
-  })
-})
-
 describe('The Statuses module', () => {
   it('adds the status to allStatuses and to the given timeline', () => {
     const state = cloneDeep(defaultState)
-    const status = makeMockStatus({id: 1})
+    const status = makeMockStatus({id: '1'})
 
     mutations.addNewStatuses(state, { statuses: [status], timeline: 'public' })
 
@@ -52,7 +37,7 @@ describe('The Statuses module', () => {
 
   it('counts the status as new if it has not been seen on this timeline', () => {
     const state = cloneDeep(defaultState)
-    const status = makeMockStatus({id: 1})
+    const status = makeMockStatus({id: '1'})
 
     mutations.addNewStatuses(state, { statuses: [status], timeline: 'public' })
     mutations.addNewStatuses(state, { statuses: [status], timeline: 'friends' })
@@ -70,7 +55,7 @@ describe('The Statuses module', () => {
 
   it('add the statuses to allStatuses if no timeline is given', () => {
     const state = cloneDeep(defaultState)
-    const status = makeMockStatus({id: 1})
+    const status = makeMockStatus({id: '1'})
 
     mutations.addNewStatuses(state, { statuses: [status] })
 
@@ -82,7 +67,7 @@ describe('The Statuses module', () => {
 
   it('adds the status to allStatuses and to the given timeline, directly visible', () => {
     const state = cloneDeep(defaultState)
-    const status = makeMockStatus({id: 1})
+    const status = makeMockStatus({id: '1'})
 
     mutations.addNewStatuses(state, { statuses: [status], showImmediately: true, timeline: 'public' })
 
@@ -94,8 +79,8 @@ describe('The Statuses module', () => {
 
   it('removes statuses by tag on deletion', () => {
     const state = cloneDeep(defaultState)
-    const status = makeMockStatus({id: 1})
+    const status = makeMockStatus({id: '1'})
-    const otherStatus = makeMockStatus({id: 3})
+    const otherStatus = makeMockStatus({id: '3'})
     status.uri = 'xxx'
     const deletion = makeMockStatus({id: 2, type: 'deletion'})
     deletion.text = 'Dolus deleted notice {{tag:gs.smuglo.li,2016-11-18:noticeId=1038007:objectType=note}}.'
@@ -107,36 +92,36 @@ describe('The Statuses module', () => {
     expect(state.allStatuses).to.eql([otherStatus])
     expect(state.timelines.public.statuses).to.eql([otherStatus])
     expect(state.timelines.public.visibleStatuses).to.eql([otherStatus])
-    expect(state.timelines.public.maxId).to.eql(3)
+    expect(state.timelines.public.maxId).to.eql('3')
   })
 
   it('does not update the maxId when the noIdUpdate flag is set', () => {
     const state = cloneDeep(defaultState)
-    const status = makeMockStatus({id: 1})
+    const status = makeMockStatus({id: '1'})
-    const secondStatus = makeMockStatus({id: 2})
+    const secondStatus = makeMockStatus({id: '2'})
 
     mutations.addNewStatuses(state, { statuses: [status], showImmediately: true, timeline: 'public' })
-    expect(state.timelines.public.maxId).to.eql(1)
+    expect(state.timelines.public.maxId).to.eql('1')
 
     mutations.addNewStatuses(state, { statuses: [secondStatus], showImmediately: true, timeline: 'public', noIdUpdate: true })
     expect(state.timelines.public.statuses).to.eql([secondStatus, status])
     expect(state.timelines.public.visibleStatuses).to.eql([secondStatus, status])
-    expect(state.timelines.public.maxId).to.eql(1)
+    expect(state.timelines.public.maxId).to.eql('1')
   })
 
   it('keeps a descending by id order in timeline.visibleStatuses and timeline.statuses', () => {
     const state = cloneDeep(defaultState)
-    const nonVisibleStatus = makeMockStatus({id: 1})
+    const nonVisibleStatus = makeMockStatus({id: '1'})
-    const status = makeMockStatus({id: 3})
+    const status = makeMockStatus({id: '3'})
-    const statusTwo = makeMockStatus({id: 2})
+    const statusTwo = makeMockStatus({id: '2'})
-    const statusThree = makeMockStatus({id: 4})
+    const statusThree = makeMockStatus({id: '4'})
 
     mutations.addNewStatuses(state, { statuses: [nonVisibleStatus], showImmediately: false, timeline: 'public' })
 
     mutations.addNewStatuses(state, { statuses: [status], showImmediately: true, timeline: 'public' })
     mutations.addNewStatuses(state, { statuses: [statusTwo], showImmediately: true, timeline: 'public' })
 
-    expect(state.timelines.public.minVisibleId).to.equal(2)
+    expect(state.timelines.public.minVisibleId).to.equal('2')
 
     mutations.addNewStatuses(state, { statuses: [statusThree], showImmediately: true, timeline: 'public' })
 
@@ -157,22 +142,22 @@ describe('The Statuses module', () => {
     expect(state.timelines.public.visibleStatuses).to.have.length(1)
     expect(state.timelines.public.statuses).to.have.length(1)
     expect(state.allStatuses).to.have.length(2)
-    expect(state.allStatuses[0].id).to.equal(1)
+    expect(state.allStatuses[0].id).to.equal('1')
-    expect(state.allStatuses[1].id).to.equal(2)
+    expect(state.allStatuses[1].id).to.equal('2')
 
     // It refers to the modified status.
     mutations.addNewStatuses(state, { statuses: [modStatus], timeline: 'public' })
     expect(state.allStatuses).to.have.length(2)
-    expect(state.allStatuses[0].id).to.equal(1)
+    expect(state.allStatuses[0].id).to.equal('1')
     expect(state.allStatuses[0].text).to.equal(modStatus.text)
-    expect(state.allStatuses[1].id).to.equal(2)
+    expect(state.allStatuses[1].id).to.equal('2')
     expect(retweet.retweeted_status.text).to.eql(modStatus.text)
   })
 
   it('replaces existing statuses with the same id', () => {
     const state = cloneDeep(defaultState)
-    const status = makeMockStatus({id: 1})
+    const status = makeMockStatus({id: '1'})
-    const modStatus = makeMockStatus({id: 1, text: 'something else'})
+    const modStatus = makeMockStatus({id: '1', text: 'something else'})
 
     // Add original status
     mutations.addNewStatuses(state, { statuses: [status], showImmediately: true, timeline: 'public' })
@@ -209,7 +194,7 @@ describe('The Statuses module', () => {
 
   it('handles favorite actions', () => {
     const state = cloneDeep(defaultState)
-    const status = makeMockStatus({id: 1})
+    const status = makeMockStatus({id: '1'})
 
     const favorite = {
       id: 2,
@@ -217,7 +202,7 @@ describe('The Statuses module', () => {
       in_reply_to_status_id: '1', // The API uses strings here...
       uri: 'tag:shitposter.club,2016-08-21:fave:3895:note:773501:2016-08-21T16:52:15+00:00',
       text: 'a favorited something by b',
-      user: { id: 99 }
+      user: { id: '99' }
     }
 
     mutations.addNewStatuses(state, { statuses: [status], showImmediately: true, timeline: 'public' })
@@ -236,7 +221,7 @@ describe('The Statuses module', () => {
 
     // If something is favorited by the current user, it also sets the 'favorited' property but does not increment counter to avoid over-counting. Counter is incremented (updated, really) via response to the favorite request.
     const user = {
-      id: 1
+      id: '1'
     }
 
     const ownFavorite = {
@@ -257,11 +242,11 @@ describe('The Statuses module', () => {
 
   describe('notifications', () => {
     it('removes a notification when the notice gets removed', () => {
-      const user = { id: 1 }
+      const user = { id: '1' }
       const state = cloneDeep(defaultState)
-      const status = makeMockStatus({id: 1})
+      const status = makeMockStatus({id: '1'})
-      const otherStatus = makeMockStatus({id: 3})
+      const otherStatus = makeMockStatus({id: '3'})
-      const mentionedStatus = makeMockStatus({id: 2})
+      const mentionedStatus = makeMockStatus({id: '2'})
       mentionedStatus.attentions = [user]
       mentionedStatus.uri = 'xxx'
       otherStatus.attentions = [user]

test/unit/specs/modules/users.spec.js

@@ -6,8 +6,8 @@ describe('The users module', () => {
   describe('mutations', () => {
     it('adds new users to the set, merging in new information for old users', () => {
       const state = cloneDeep(defaultState)
-      const user = { id: 1, name: 'Guy' }
+      const user = { id: '1', name: 'Guy' }
-      const modUser = { id: 1, name: 'Dude' }
+      const modUser = { id: '1', name: 'Dude' }
 
       mutations.addNewUsers(state, [user])
       expect(state.users).to.have.length(1)
@@ -21,7 +21,7 @@ describe('The users module', () => {
 
     it('sets a mute bit on users', () => {
       const state = cloneDeep(defaultState)
-      const user = { id: 1, name: 'Guy' }
+      const user = { id: '1', name: 'Guy' }
 
       mutations.addNewUsers(state, [user])
       mutations.setMuted(state, {user, muted: true})
@@ -38,11 +38,11 @@ describe('The users module', () => {
     it('returns user with matching screen_name', () => {
       const state = {
         users: [
-          { screen_name: 'Guy', id: 1 }
+          { screen_name: 'Guy', id: '1' }
         ]
       }
       const name = 'Guy'
-      const expected = { screen_name: 'Guy', id: 1 }
+      const expected = { screen_name: 'Guy', id: '1' }
       expect(getters.userByName(state)(name)).to.eql(expected)
     })
   })
@@ -51,11 +51,11 @@ describe('The users module', () => {
     it('returns user with matching id', () => {
       const state = {
         users: [
-          { screen_name: 'Guy', id: 1 }
+          { screen_name: 'Guy', id: '1' }
         ]
       }
-      const id = 1
+      const id = '1'
-      const expected = { screen_name: 'Guy', id: 1 }
+      const expected = { screen_name: 'Guy', id: '1' }
       expect(getters.userById(state)(id)).to.eql(expected)
     })
   })

test/unit/specs/services/notification_utils/notification_utils.spec.js

@@ -9,15 +9,15 @@ describe('NotificationUtils', () => {
             notifications: {
               data: [
                 {
-                  action: { id: 1 },
+                  action: { id: '1' },
                   type: 'like'
                 },
                 {
-                  action: { id: 2 },
+                  action: { id: '2' },
                   type: 'mention'
                 },
                 {
-                  action: { id: 3 },
+                  action: { id: '3' },
                   type: 'repeat'
                 }
               ]
@@ -34,11 +34,11 @@ describe('NotificationUtils', () => {
       }
       const expected = [
         {
-          action: { id: 3 },
+          action: { id: '3' },
           type: 'repeat'
         },
         {
-          action: { id: 1 },
+          action: { id: '1' },
           type: 'like'
         }
       ]
@@ -54,12 +54,12 @@ describe('NotificationUtils', () => {
             notifications: {
               data: [
                 {
-                  action: { id: 1 },
+                  action: { id: '1' },
                   type: 'like',
                   seen: false
                 },
                 {
-                  action: { id: 2 },
+                  action: { id: '2' },
                   type: 'mention',
                   seen: true
                 }
@@ -77,7 +77,7 @@ describe('NotificationUtils', () => {
       }
       const expected = [
         {
-          action: { id: 1 },
+          action: { id: '1' },
           type: 'like',
           seen: false
         }