about summary refs log tree commit diff
diff options
context:
space:
mode:
authorunarist <m.unarist@gmail.com>2018-05-30 00:42:29 +0900
committerEugen Rochko <eugen@zeonfederated.com>2018-05-29 17:42:29 +0200
commit7706ed038f1a16d232635d564a28e6f90e53e0fd (patch)
tree7784b8430924432305ec1b4ce6ecda13714809cf
parent0345cd5a0f434a43ea988a2b50ab6f8597bcf58e (diff)
Fix context building in the reducer (#7671)
This fixes below bugs:

* addReply() had used native compare operator for string ids
  => descendants may appears at wrong position
* CONTEXT_FETCH_SUCCESS had added the focused status as the reply of the *first* status in ancestors, not last status.
  => descendants may also appears wrong position as well as correct position
* TIMELINE_UPDATE had added the status to replies of *itself* instead of replied status
  => browser will hangs if you open the status due to a circular reference
-rw-r--r--app/javascript/mastodon/reducers/contexts.js27
1 files changed, 14 insertions, 13 deletions
diff --git a/app/javascript/mastodon/reducers/contexts.js b/app/javascript/mastodon/reducers/contexts.js
index 53e70b58e..4c2d6cc8a 100644
--- a/app/javascript/mastodon/reducers/contexts.js
+++ b/app/javascript/mastodon/reducers/contexts.js
@@ -5,6 +5,7 @@ import {
 import { CONTEXT_FETCH_SUCCESS } from '../actions/statuses';
 import { TIMELINE_DELETE, TIMELINE_UPDATE } from '../actions/timelines';
 import { Map as ImmutableMap, List as ImmutableList } from 'immutable';
+import compareId from '../compare_id';
 
 const initialState = ImmutableMap({
   inReplyTos: ImmutableMap(),
@@ -15,27 +16,27 @@ const normalizeContext = (immutableState, id, ancestors, descendants) => immutab
   state.update('inReplyTos', immutableAncestors => immutableAncestors.withMutations(inReplyTos => {
     state.update('replies', immutableDescendants => immutableDescendants.withMutations(replies => {
       function addReply({ id, in_reply_to_id }) {
-        if (in_reply_to_id) {
-          const siblings = replies.get(in_reply_to_id, ImmutableList());
+        if (in_reply_to_id && !inReplyTos.has(id)) {
 
-          if (!siblings.includes(id)) {
-            const index = siblings.findLastIndex(sibling => sibling.id < id);
-            replies.set(in_reply_to_id, siblings.insert(index + 1, id));
-          }
+          replies.update(in_reply_to_id, ImmutableList(), siblings => {
+            const index = siblings.findLastIndex(sibling => compareId(sibling, id) < 0);
+            return siblings.insert(index + 1, id);
+          });
 
           inReplyTos.set(id, in_reply_to_id);
         }
       }
 
-      if (ancestors[0]) {
-        addReply({ id, in_reply_to_id: ancestors[0].id });
-      }
+      // We know in_reply_to_id of statuses but `id` itself.
+      // So we assume that the status of the id replies to last ancestors.
 
-      if (descendants[0]) {
-        addReply({ id: descendants[0].id, in_reply_to_id: id });
+      ancestors.forEach(addReply);
+
+      if (ancestors[0]) {
+        addReply({ id, in_reply_to_id: ancestors[ancestors.length - 1].id });
       }
 
-      [ancestors, descendants].forEach(statuses => statuses.forEach(addReply));
+      descendants.forEach(addReply);
     }));
   }));
 });
@@ -80,7 +81,7 @@ const updateContext = (state, status) => {
       mutable.setIn(['inReplyTos', status.id], status.in_reply_to_id);
 
       if (!replies.includes(status.id)) {
-        mutable.setIn(['replies', status.id], replies.push(status.id));
+        mutable.setIn(['replies', status.in_reply_to_id], replies.push(status.id));
       }
     });
   }