From bcb142e68e5ea53870123cc4366f2d64174afb09 Mon Sep 17 00:00:00 2001 From: Steve Howell Date: Tue, 4 Dec 2018 22:24:03 +0000 Subject: [PATCH] hashchange: Move parse_narrow to hash_util.js. The goal here was to enforce 100% coverage on parse_narrow, but the code has an unreachable line and is overly tolerant of bogus urls. This will be fixed in the next commit. --- frontend_tests/node_tests/hash_util.js | 13 +++++++++++ frontend_tests/node_tests/hashchange.js | 10 ++++----- static/js/hash_util.js | 27 +++++++++++++++++++++++ static/js/hashchange.js | 29 +------------------------ tools/test-js-with-node | 2 +- 5 files changed, 47 insertions(+), 34 deletions(-) diff --git a/frontend_tests/node_tests/hash_util.js b/frontend_tests/node_tests/hash_util.js index 8fd583fce5..d8b62eaa8b 100644 --- a/frontend_tests/node_tests/hash_util.js +++ b/frontend_tests/node_tests/hash_util.js @@ -63,6 +63,19 @@ run_test('hash_util', () => { encode_decode_operand(operator, operand, 'testing.20123'); }); +run_test('test_parse_narrow', () => { + assert.deepEqual( + hash_util.parse_narrow(['narrow', 'stream', '11-social']), + [{negated: false, operator: 'stream', operand: '11-social'}] + ); + + // This should probably return undefined. + assert.deepEqual( + hash_util.parse_narrow(['narrow', 'BOGUS']), + [{negated: false, operator: 'BOGUS', operand: ''}] + ); +}); + run_test('test_stream_edit_uri', () => { var sub = { name: 'research & development', diff --git a/frontend_tests/node_tests/hashchange.js b/frontend_tests/node_tests/hashchange.js index 6c87eed8e5..f961a1c6ab 100644 --- a/frontend_tests/node_tests/hashchange.js +++ b/frontend_tests/node_tests/hashchange.js @@ -37,7 +37,7 @@ run_test('operators_round_trip', () => { hash = hash_util.operators_to_hash(operators); assert.equal(hash, '#narrow/stream/devel/topic/algol'); - narrow = hashchange.parse_narrow(hash.split('/')); + narrow = hash_util.parse_narrow(hash.split('/')); assert.deepEqual(narrow, [ {operator: 'stream', operand: 'devel', negated: false}, {operator: 'topic', operand: 'algol', negated: false}, @@ -50,7 +50,7 @@ run_test('operators_round_trip', () => { hash = hash_util.operators_to_hash(operators); assert.equal(hash, '#narrow/stream/devel/-topic/visual.20c.2B.2B'); - narrow = hashchange.parse_narrow(hash.split('/')); + narrow = hash_util.parse_narrow(hash.split('/')); assert.deepEqual(narrow, [ {operator: 'stream', operand: 'devel', negated: false}, {operator: 'topic', operand: 'visual c++', negated: true}, @@ -67,7 +67,7 @@ run_test('operators_round_trip', () => { ]; hash = hash_util.operators_to_hash(operators); assert.equal(hash, '#narrow/stream/987-Florida.2C-USA'); - narrow = hashchange.parse_narrow(hash.split('/')); + narrow = hash_util.parse_narrow(hash.split('/')); assert.deepEqual(narrow, [ {operator: 'stream', operand: 'Florida, USA', negated: false}, ]); @@ -78,7 +78,7 @@ run_test('operators_trailing_slash', () => { var narrow; hash = '#narrow/stream/devel/topic/algol/'; - narrow = hashchange.parse_narrow(hash.split('/')); + narrow = hash_util.parse_narrow(hash.split('/')); assert.deepEqual(narrow, [ {operator: 'stream', operand: 'devel', negated: false}, {operator: 'topic', operand: 'algol', negated: false}, @@ -102,7 +102,7 @@ run_test('people_slugs', () => { ]; hash = hash_util.operators_to_hash(operators); assert.equal(hash, '#narrow/sender/42-alice'); - narrow = hashchange.parse_narrow(hash.split('/')); + narrow = hash_util.parse_narrow(hash.split('/')); assert.deepEqual(narrow, [ {operator: 'sender', operand: 'alice@example.com', negated: false}, ]); diff --git a/static/js/hash_util.js b/static/js/hash_util.js index 05bf1ad09f..89516f5cec 100644 --- a/static/js/hash_util.js +++ b/static/js/hash_util.js @@ -127,6 +127,33 @@ exports.stream_edit_uri = function (sub) { return hash; }; +exports.parse_narrow = function (hash) { + var i; + var operators = []; + for (i = 1; i < hash.length; i += 2) { + // We don't construct URLs with an odd number of components, + // but the user might write one. + try { + var operator = exports.decodeHashComponent(hash[i]); + // Do not parse further if empty operator encountered. + if (operator === '') { + break; + } + + var operand = exports.decode_operand(operator, hash[i + 1] || ''); + var negated = false; + if (operator[0] === '-') { + negated = true; + operator = operator.slice(1); + } + operators.push({negated: negated, operator: operator, operand: operand}); + } catch (err) { + return; + } + } + return operators; +}; + return exports; }()); diff --git a/static/js/hashchange.js b/static/js/hashchange.js index 96c1b16a9b..1b53294dd5 100644 --- a/static/js/hashchange.js +++ b/static/js/hashchange.js @@ -47,33 +47,6 @@ exports.save_narrow = function (operators) { exports.changehash(new_hash); }; -exports.parse_narrow = function (hash) { - var i; - var operators = []; - for (i = 1; i < hash.length; i += 2) { - // We don't construct URLs with an odd number of components, - // but the user might write one. - try { - var operator = hash_util.decodeHashComponent(hash[i]); - // Do not parse further if empty operator encountered. - if (operator === '') { - break; - } - - var operand = hash_util.decode_operand(operator, hash[i + 1] || ''); - var negated = false; - if (operator[0] === '-') { - negated = true; - operator = operator.slice(1); - } - operators.push({negated: negated, operator: operator, operand: operand}); - } catch (err) { - return; - } - } - return operators; -}; - function activate_home_tab() { ui_util.change_tab_to("#home"); narrow.deactivate(); @@ -118,7 +91,7 @@ function do_hashchange_normal(from_reload) { switch (hash[0]) { case "#narrow": ui_util.change_tab_to("#home"); - var operators = exports.parse_narrow(hash); + var operators = hash_util.parse_narrow(hash); if (operators === undefined) { // If the narrow URL didn't parse, clear // window.location.hash and send them to the home tab diff --git a/tools/test-js-with-node b/tools/test-js-with-node index 4219341cfc..baa2242872 100755 --- a/tools/test-js-with-node +++ b/tools/test-js-with-node @@ -44,7 +44,7 @@ enforce_fully_covered = { 'static/js/fenced_code.js', 'static/js/fetch_status.js', 'static/js/filter.js', - 'static/js/hash_util.js', + # 'static/js/hash_util.js', 'static/js/keydown_util.js', 'static/js/input_pill.js', 'static/js/list_cursor.js',