Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

util: Prepare to remove get_message_topic(). 

I am not quite ready to declare victory on
the subject/topic migration, but we are super
close.  In this commit I bump a blueslip
warning to a blueslip error, so that we'll
be notified of any codepath that is still
using the janky fall-back-to-subject defensive
code here.

If we go a couple days without any errors, then
we can remove the blueslip warning and the
defensive code immediately and then inline
the callers at our leisure.  I wouldn't be
wildly against keeping these wrappers in some
parts of the code, but that debate is out of
the scope of this immediate fix, and I haven't
thought hard about it yet.

We can basically sweep set_message_topic() now,
if we wanted to, since it's truly just a one-liner.
(At one point it was encapsulating something
like `message.subject = foo`).

This required a tiny change to compose_fade
test setup.
This commit is contained in:
Steve Howell 2020-02-14 12:59:37 +00:00
parent 8deac44a54
commit 39405fccdc
3 changed files with 16 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
frontend_tests/node_tests/compose_fade.js
View File

@ -68,12 +68,12 @@ run_test('set_focused_recipient', () => {
const good_msg = {
type: 'stream',
stream_id: 101,
subject: 'lunch',
topic: 'lunch',
};
const bad_msg = {
type: 'stream',
stream_id: 999,
subject: 'lunch',
topic: 'lunch',
};
assert(!compose_fade.should_fade_message(good_msg));
assert(compose_fade.should_fade_message(bad_msg));

2
frontend_tests/node_tests/util.js
View File

@ -109,7 +109,7 @@ run_test('robust_uri_decode', () => {
});
run_test('get_message_topic', () => {
blueslip.set_test_data('warn', 'programming error: message has no topic');
blueslip.set_test_data('error', 'programming error: message has no topic');
assert.equal(util.get_message_topic({subject: 'foo'}), 'foo');
blueslip.clear_test_data();
assert.equal(util.get_message_topic({topic: 'bar'}), 'bar');

14
static/js/util.js
View File

@ -268,8 +268,20 @@ exports.set_message_topic = function (obj, topic) {
};
exports.get_message_topic = function (obj) {
// TODO: Kill off the below defensive code by the
// end of February 2020 (giving ourselves a
// little bit of time to make sure we don't
// have any obscure codepaths left that weren't
// properly handled in our subject/topic migration).
//
// Also, once we are confident that all of our
// internal objects have been migrated from
// "subject" to "topic", we can start having
// our callers just do `foo = message.topic`
// and phase out the use of this function. Or
// we can just sweep the codebase.
if (obj.topic === undefined) {
blueslip.warn('programming error: message has no topic');
blueslip.error('programming error: message has no topic');
return obj.subject;
}