From 0844a80d6629c86186170a4a7a1c0c1edd4aaabe Mon Sep 17 00:00:00 2001 From: Lauryn Menard Date: Thu, 17 Mar 2022 16:58:10 +0100 Subject: [PATCH] edit_history: Check edit history for stream and topic match. Creates a helper function in `message_edit.js` that loops over a message's edit history to see if a stream and topic pair existed at some point in the message history. Exports `util.lower_same` function to use for comparing edit history topics as lowercase. Also adds test for new function in `mesage_edit` node tests. --- frontend_tests/node_tests/message_edit.js | 111 ++++++++++++++++++++++ static/js/message_edit.js | 48 ++++++++++ static/js/util.js | 4 +- 3 files changed, 161 insertions(+), 2 deletions(-) diff --git a/frontend_tests/node_tests/message_edit.js b/frontend_tests/node_tests/message_edit.js index 71eb0f93dd..7646072e34 100644 --- a/frontend_tests/node_tests/message_edit.js +++ b/frontend_tests/node_tests/message_edit.js @@ -158,3 +158,114 @@ run_test("get_deletability", ({override}) => { message.timestamp = current_timestamp - 60; assert.equal(message_edit.get_deletability(message), false); }); + +run_test("stream_and_topic_exist_in_edit_history", () => { + // A message with no edit history should always return false; + // the message's current stream_id and topic are not compared + // to the stream_id and topic parameters. + const message_no_edits = { + stream_id: 1, + topic: "topic match", + }; + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_no_edits, 2, "no match"), + false, + ); + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_no_edits, 1, "topic match"), + false, + ); + + // A non-stream message (object has no stream_id or topic) + // with content edit history, should return false. + const private_message = { + edit_history: [{prev_content: "content edit to PM"}], + }; + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(private_message, 1, "topic match"), + false, + ); + + // A stream message with only content edits should return false, + // even if the message's current stream_id and topic are a match. + const message_content_edit = { + stream_id: 1, + topic: "topic match", + edit_history: [{prev_content: "content edit"}], + }; + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_content_edit, 1, "topic match"), + false, + ); + + const message_stream_edit = { + stream_id: 6, + topic: "topic match", + edit_history: [{stream: 6, prev_stream: 1}], + }; + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_stream_edit, 2, "topic match"), + false, + ); + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_stream_edit, 1, "topic match"), + true, + ); + + const message_topic_edit = { + stream_id: 1, + topic: "final topic", + edit_history: [{topic: "final topic", prev_topic: "topic match"}], + }; + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_topic_edit, 1, "no match"), + false, + ); + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_topic_edit, 1, "topic match"), + true, + ); + + const message_many_edits = { + stream_id: 6, + topic: "final topic", + edit_history: [ + {stream: 6, prev_stream: 5}, + {prev_content: "content only edit"}, + {topic: "final topic", prev_topic: "topic match"}, + {stream: 5, prev_stream: 1}, + ], + }; + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_many_edits, 1, "no match"), + false, + ); + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_many_edits, 2, "topic match"), + false, + ); + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history(message_many_edits, 1, "topic match"), + true, + ); + + // When the topic and stream_id exist in the message's edit history + // individually, but not together in a historical state, it should return false. + const message_no_historical_match = { + stream_id: 6, + topic: "final topic", + edit_history: [ + {stream: 6, prev_stream: 1}, // stream matches, topic does not + {stream: 1, prev_stream: 5}, // neither match + {topic: "final topic", prev_topic: "topic match"}, // topic matches, stream does not + ], + }; + assert.equal( + message_edit.stream_and_topic_exist_in_edit_history( + message_no_historical_match, + 1, + "topic match", + ), + false, + ); +}); diff --git a/static/js/message_edit.js b/static/js/message_edit.js index f1f2db07a2..736db7d1ab 100644 --- a/static/js/message_edit.js +++ b/static/js/message_edit.js @@ -33,6 +33,7 @@ import * as stream_data from "./stream_data"; import * as sub_store from "./sub_store"; import * as ui_report from "./ui_report"; import * as upload from "./upload"; +import * as util from "./util"; const currently_editing_messages = new Map(); let currently_deleting_messages = []; @@ -185,6 +186,53 @@ export function get_deletability(message) { return false; } +export function stream_and_topic_exist_in_edit_history(message, stream_id, topic) { + /* Checks to see if a stream_id and a topic match any historical + stream_id and topic state in the message's edit history. + + Does not check the message's current stream_id and topic for + a match to the stream_id and topic parameters. + */ + const narrow_dict = {stream_id, topic}; + const message_dict = {stream_id: message.stream_id, topic: message.topic}; + + if (!message.edit_history) { + // If message edit history is disabled in the organization, + // the client does not have the information locally to answer + // this question correctly. + return false; + } + + for (const edit_history_event of message.edit_history) { + if (!edit_history_event.prev_stream && !edit_history_event.prev_topic) { + // Message was not moved in this edit event. + continue; + } + + if (edit_history_event.prev_stream) { + // This edit event changed the stream. We expect the + // following to be true due to the invariants of the edit + // history data structure: + // edit_history_event.stream === message_dict.stream_id + message_dict.stream_id = edit_history_event.prev_stream; + } + + if (edit_history_event.prev_topic) { + // This edit event changed the topic. We expect the + // following to be true due to the invariants of the edit + // history data structure: + // util.lower_same(edit_history_event.topic, message_dict.topic) + message_dict.topic = edit_history_event.prev_topic; + } + + if (util.same_stream_and_topic(narrow_dict, message_dict)) { + return true; + } + } + + return false; +} + export function update_message_topic_editing_pencil() { if (page_params.realm_allow_message_editing) { $(".on_hover_topic_edit, .always_visible_topic_edit").show(); diff --git a/static/js/util.js b/static/js/util.js index 547a643c5f..cee7fa0dd0 100644 --- a/static/js/util.js +++ b/static/js/util.js @@ -36,9 +36,9 @@ export function lower_bound(array, value, less) { return first; } -function lower_same(a, b) { +export const lower_same = function lower_same(a, b) { return a.toLowerCase() === b.toLowerCase(); -} +}; export const same_stream_and_topic = function util_same_stream_and_topic(a, b) { // Streams and topics are case-insensitive.