diff --git a/swh/web/ui/tests/test_utils.py b/swh/web/ui/tests/test_utils.py --- a/swh/web/ui/tests/test_utils.py +++ b/swh/web/ui/tests/test_utils.py @@ -476,15 +476,26 @@ call('api_entity_by_uuid', uuid='uuid-parent')]) + @nottest + def url_for_context_test(self, fn, **data): + if fn == 'api_revision': + if 'context' in data and data['context'] is not None: + return '/api/revision/%s/prev/%s/' % (data['sha1_git'], data['context']) # noqa + else: + return '/api/revision/%s/' % data['sha1_git'] + elif fn == 'api_revision_log': + if 'prev_sha1s' in data: + return '/api/revision/%s/prev/%s/log/' % (data['sha1_git'], data['prev_sha1s']) # noqa + else: + return '/api/revision/%s/log/' % data['sha1_git'] + @patch('swh.web.ui.utils.flask') @istest def enrich_revision_without_children_or_parent(self, mock_flask): # given def url_for_test(fn, **data): - if fn == 'api_revision': - return '/api/revision/' + data['sha1_git'] + '/' - elif fn == 'api_revision_log': - return '/api/revision/' + data['sha1_git'] + '/log/' + if fn == 'api_revision' or fn == 'api_revision_log': + return self.url_for_context_test(fn, **data) elif fn == 'api_directory': return '/api/directory/' + data['sha1_git'] + '/' elif fn == 'api_person': @@ -500,8 +511,7 @@ 'committer': {'id': '2'}, }) - # then - self.assertEqual(actual_revision, { + expected_revision = { 'id': 'rev-id', 'directory': '123', 'url': '/api/revision/rev-id/', @@ -511,18 +521,22 @@ 'author_url': '/api/person/1/', 'committer': {'id': '2'}, 'committer_url': '/api/person/2/' - }) + } + + # then + self.assertEqual(actual_revision, expected_revision) - mock_flask.url_for.assert_has_calls([call('api_revision', - sha1_git='rev-id'), - call('api_revision_log', - sha1_git='rev-id'), - call('api_person', - person_id='1'), - call('api_person', - person_id='2'), - call('api_directory', - sha1_git='123')]) + mock_flask.url_for.assert_has_calls( + [call('api_revision', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id'), + call('api_person', + person_id='1'), + call('api_person', + person_id='2'), + call('api_directory', + sha1_git='123')]) @patch('swh.web.ui.utils.flask') @istest @@ -530,10 +544,8 @@ mock_flask): # given def url_for_test(fn, **data): - if fn == 'api_revision': - return '/api/revision/' + data['sha1_git'] + '/' - elif fn == 'api_revision_log': - return '/api/revision/' + data['sha1_git'] + '/log/' + if fn == 'api_revision' or fn == 'api_revision_log': + return self.url_for_context_test(fn, **data) else: return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa @@ -544,37 +556,182 @@ 'id': 'rev-id', 'parents': ['123'], 'children': ['456'], - }, context='sha1_git_root') + }, context='prev-rev') - # then - self.assertEqual(actual_revision, { + expected_revision = { 'id': 'rev-id', 'url': '/api/revision/rev-id/', 'history_url': '/api/revision/rev-id/log/', + 'history_context_url': '/api/revision/rev-id/prev/prev-rev/log/', + 'parents': ['123'], + 'parent_urls': ['/api/revision/123/prev/prev-rev/rev-id/'], + 'children': ['456'], + 'children_urls': ['/api/revision/456/', + '/api/revision/prev-rev/'], + } + + # then + self.assertEqual(actual_revision, expected_revision) + + mock_flask.url_for.assert_has_calls( + [call('api_revision', + sha1_git='prev-rev'), + call('api_revision', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id', + prev_sha1s='prev-rev'), + call('api_revision', + sha1_git='123', + context='prev-rev/rev-id'), + call('api_revision', + sha1_git='456')]) + + @patch('swh.web.ui.utils.flask') + @istest + def enrich_revision_no_context(self, mock_flask): + # given + def url_for_test(fn, **data): + if fn == 'api_revision' or fn == 'api_revision_log': + return self.url_for_context_test(fn, **data) + else: + return '/api/revision/' + data['sha1_git_root'] + '/history/' + data['sha1_git'] + '/' # noqa + + mock_flask.url_for.side_effect = url_for_test + + # when + actual_revision = utils.enrich_revision({ + 'id': 'rev-id', 'parents': ['123'], - 'parent_urls': ['/api/revision/sha1_git_root/history/123/'], 'children': ['456'], - 'children_urls': ['/api/revision/sha1_git_root/history/456/'], }) + expected_revision = { + 'id': 'rev-id', + 'url': '/api/revision/rev-id/', + 'history_url': '/api/revision/rev-id/log/', + 'parents': ['123'], + 'parent_urls': ['/api/revision/123/prev/rev-id/'], + 'children': ['456'], + 'children_urls': ['/api/revision/456/'] + } + + # then + self.assertEqual(actual_revision, expected_revision) + mock_flask.url_for.assert_has_calls( [call('api_revision', sha1_git='rev-id'), call('api_revision_log', sha1_git='rev-id'), - call('api_revision_history', - sha1_git_root='sha1_git_root', - sha1_git='123'), - call('api_revision_history', - sha1_git_root='sha1_git_root', + call('api_revision', + sha1_git='123', + context='rev-id'), + call('api_revision', + sha1_git='456')]) + + @patch('swh.web.ui.utils.flask') + @istest + def enrich_revision_context_empty_prev_list(self, mock_flask): + # given + mock_flask.url_for.side_effect = self.url_for_context_test + + # when + expected_revision = { + 'id': 'rev-id', + 'url': '/api/revision/rev-id/', + 'history_url': '/api/revision/rev-id/log/', + 'history_context_url': ('/api/revision/rev-id/' + 'prev/prev-rev/log/'), + 'parents': ['123'], + 'parent_urls': ['/api/revision/123/prev/prev-rev/rev-id/'], + 'children': ['456'], + 'children_urls': ['/api/revision/456/', '/api/revision/prev-rev/'], + } + + actual_revision = utils.enrich_revision({ + 'id': 'rev-id', + 'url': '/api/revision/rev-id/', + 'parents': ['123'], + 'children': ['456']}, context='prev-rev') + + # then + self.assertEqual(actual_revision, expected_revision) + mock_flask.url_for.assert_has_calls( + [call('api_revision', + sha1_git='prev-rev'), + call('api_revision', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id', + prev_sha1s='prev-rev'), + call('api_revision', + sha1_git='123', + context='prev-rev/rev-id'), + call('api_revision', + sha1_git='456')]) + + @patch('swh.web.ui.utils.flask') + @istest + def enrich_revision_context_some_prev_list(self, mock_flask): + # given + mock_flask.url_for.side_effect = self.url_for_context_test + + # when + expected_revision = { + 'id': 'rev-id', + 'url': '/api/revision/rev-id/', + 'history_url': '/api/revision/rev-id/log/', + 'history_context_url': ('/api/revision/rev-id/' + 'prev/prev1-rev/prev0-rev/log/'), + 'parents': ['123'], + 'parent_urls': ['/api/revision/123/prev/' + 'prev1-rev/prev0-rev/rev-id/'], + 'children': ['456'], + 'children_urls': ['/api/revision/456/', + '/api/revision/prev0-rev/prev/prev1-rev/'], + } + + actual_revision = utils.enrich_revision({ + 'id': 'rev-id', + 'parents': ['123'], + 'children': ['456']}, context='prev1-rev/prev0-rev') + + # then + self.assertEqual(actual_revision, expected_revision) + mock_flask.url_for.assert_has_calls( + [call('api_revision', + sha1_git='prev0-rev', + context='prev1-rev'), + call('api_revision', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id'), + call('api_revision_log', + sha1_git='rev-id', + prev_sha1s='prev1-rev/prev0-rev'), + call('api_revision', + sha1_git='123', + context='prev1-rev/prev0-rev/rev-id'), + call('api_revision', sha1_git='456')]) @nottest def _url_for_rev_message_test(self, fn, **data): if fn == 'api_revision': - return '/api/revision/' + data['sha1_git'] + '/' + if 'context' in data and data['context'] is not None: + return '/api/revision/%s/prev/%s/' % (data['sha1_git'], data['context']) # noqa + else: + return '/api/revision/%s/' % data['sha1_git'] elif fn == 'api_revision_log': - return '/api/revision/' + data['sha1_git'] + '/log/' + if 'prev_sha1s' in data and data['prev_sha1s'] is not None: + return '/api/revision/%s/prev/%s/log/' % (data['sha1_git'], data['prev_sha1s']) # noqa + else: + return '/api/revision/%s/log/' % data['sha1_git'] elif fn == 'api_revision_raw_message': return '/api/revision/' + data['sha1_git'] + '/raw/' else: @@ -587,35 +744,43 @@ mock_flask.url_for.side_effect = self._url_for_rev_message_test # when - actual_revision = utils.enrich_revision({ + expected_revision = { 'id': 'rev-id', + 'url': '/api/revision/rev-id/', + 'history_url': '/api/revision/rev-id/log/', + 'history_context_url': ('/api/revision/rev-id/' + 'prev/prev-rev/log/'), 'message': None, 'parents': ['123'], + 'parent_urls': ['/api/revision/123/prev/prev-rev/rev-id/'], 'children': ['456'], - }, context='sha1_git_root') + 'children_urls': ['/api/revision/456/', '/api/revision/prev-rev/'], + } - # then - self.assertEqual(actual_revision, { + actual_revision = utils.enrich_revision({ 'id': 'rev-id', - 'url': '/api/revision/rev-id/', 'message': None, - 'history_url': '/api/revision/rev-id/log/', 'parents': ['123'], - 'parent_urls': ['/api/revision/sha1_git_root/history/123/'], 'children': ['456'], - 'children_urls': ['/api/revision/sha1_git_root/history/456/'], - }) + }, context='prev-rev') + + # then + self.assertEqual(actual_revision, expected_revision) mock_flask.url_for.assert_has_calls( [call('api_revision', + sha1_git='prev-rev'), + call('api_revision', sha1_git='rev-id'), call('api_revision_log', sha1_git='rev-id'), - call('api_revision_history', - sha1_git_root='sha1_git_root', - sha1_git='123'), - call('api_revision_history', - sha1_git_root='sha1_git_root', + call('api_revision_log', + sha1_git='rev-id', + prev_sha1s='prev-rev'), + call('api_revision', + sha1_git='123', + context='prev-rev/rev-id'), + call('api_revision', sha1_git='456')]) @patch('swh.web.ui.utils.flask') @@ -631,30 +796,38 @@ 'message_decoding_failed': True, 'parents': ['123'], 'children': ['456'], - }, context='sha1_git_root') + }, context='prev-rev') - # then - self.assertEqual(actual_revision, { + expected_revision = { 'id': 'rev-id', 'url': '/api/revision/rev-id/', + 'history_url': '/api/revision/rev-id/log/', + 'history_context_url': ('/api/revision/rev-id/' + 'prev/prev-rev/log/'), 'message': None, 'message_decoding_failed': True, 'message_url': '/api/revision/rev-id/raw/', - 'history_url': '/api/revision/rev-id/log/', 'parents': ['123'], - 'parent_urls': ['/api/revision/sha1_git_root/history/123/'], + 'parent_urls': ['/api/revision/123/prev/prev-rev/rev-id/'], 'children': ['456'], - 'children_urls': ['/api/revision/sha1_git_root/history/456/'], - }) + 'children_urls': ['/api/revision/456/', '/api/revision/prev-rev/'], + } + + # then + self.assertEqual(actual_revision, expected_revision) mock_flask.url_for.assert_has_calls( [call('api_revision', + sha1_git='prev-rev'), + call('api_revision', sha1_git='rev-id'), call('api_revision_log', sha1_git='rev-id'), - call('api_revision_history', - sha1_git_root='sha1_git_root', - sha1_git='123'), - call('api_revision_history', - sha1_git_root='sha1_git_root', + call('api_revision_log', + sha1_git='rev-id', + prev_sha1s='prev-rev'), + call('api_revision', + sha1_git='123', + context='prev-rev/rev-id'), + call('api_revision', sha1_git='456')]) diff --git a/swh/web/ui/utils.py b/swh/web/ui/utils.py --- a/swh/web/ui/utils.py +++ b/swh/web/ui/utils.py @@ -200,14 +200,45 @@ def enrich_revision(revision, context=None): """Enrich revision with links where it makes sense (directory, parents). + Keep track of the navigation breadcrumbs if they are specified. + Args: + revision: the revision as a dict + context: the navigation breadcrumbs as a /-separated string of revision + sha1_git """ + + context_for_parents = None + context_for_children = None + url_imm_child = None + if not context: - context = revision['id'] + context_for_parents = revision['id'] + else: + context_for_parents = '%s/%s' % (context, revision['id']) + prev_for_children = context.split('/')[:-1] + if len(prev_for_children) > 0: + context_for_children = '/'.join(prev_for_children) + # This commit is not the first commit in the path + if context_for_children: + url_imm_child = flask.url_for( + 'api_revision', + sha1_git=context.split('/')[-1], + context=context_for_children) + # This commit is the first commit in the path + else: + url_imm_child = flask.url_for( + 'api_revision', + sha1_git=context.split('/')[-1]) revision['url'] = flask.url_for('api_revision', sha1_git=revision['id']) revision['history_url'] = flask.url_for('api_revision_log', sha1_git=revision['id']) + if context is not None: + revision['history_context_url'] = flask.url_for( + 'api_revision_log', + sha1_git=revision['id'], + prev_sha1s=context) if 'author' in revision: author = revision['author'] @@ -227,21 +258,30 @@ if 'parents' in revision: parents = [] for parent in revision['parents']: - parents.append(flask.url_for('api_revision_history', - sha1_git_root=context, - sha1_git=parent)) - + parents.append(flask.url_for('api_revision', + sha1_git=parent, + context=context_for_parents)) revision['parent_urls'] = parents if 'children' in revision: children = [] for child in revision['children']: - children.append(flask.url_for('api_revision_history', - sha1_git_root=context, - sha1_git=child)) - + # child not the breadcrumb-indicated child > new breadcrumb start + if context: + if child != context.split('/')[-1]: + children.append(flask.url_for('api_revision', + sha1_git=child)) + else: + children.append(flask.url_for('api_revision', sha1_git=child)) + + if url_imm_child: + children.append(url_imm_child) revision['children_urls'] = children + else: + if url_imm_child: + revision['children_urls'] = [url_imm_child] + if 'message_decoding_failed' in revision: revision['message_url'] = flask.url_for( 'api_revision_raw_message',