Repository: allura Updated Branches: refs/heads/master 6b5b8c527 -> 650e181ce [#8214] determine merge request commits as background task Project: http://git-wip-us.apache.org/repos/asf/allura/repo Commit: http://git-wip-us.apache.org/repos/asf/allura/commit/650e181c Tree: http://git-wip-us.apache.org/repos/asf/allura/tree/650e181c Diff: http://git-wip-us.apache.org/repos/asf/allura/diff/650e181c Branch: refs/heads/master Commit: 650e181ceb9cec3a98e600c8f864bb3935264735 Parents: 6b5b8c5 Author: Dave Brondsema Authored: Fri Jun 29 17:32:15 2018 -0400 Committer: Kenton Taylor Committed: Mon Jul 9 15:44:53 2018 +0000 ---------------------------------------------------------------------- Allura/allura/controllers/repository.py | 38 +++++++++--- Allura/allura/model/repository.py | 20 +++--- Allura/allura/tasks/repo_tasks.py | 7 +++ Allura/allura/templates/repo/merge_request.html | 39 ++++++++++-- .../tests/functional/test_controllers.py | 65 +++++++++++++++----- 5 files changed, 127 insertions(+), 42 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/Allura/allura/controllers/repository.py ---------------------------------------------------------------------- diff --git a/Allura/allura/controllers/repository.py b/Allura/allura/controllers/repository.py index 2c24dbe..72de27c 100644 --- a/Allura/allura/controllers/repository.py +++ b/Allura/allura/controllers/repository.py @@ -401,16 +401,23 @@ class MergeRequestController(object): limit=limit, count=self.req.discussion_thread.post_count, subscribed=subscribed, + commits_task_started=False, ) - try: - result['commits'] = self.req.commits - except Exception: - log.info( - "Can't get commits for merge request %s", - self.req.url(), - exc_info=True) + if self.req.new_commits is not None: + try: + result['commits'] = self.req.commits + except Exception: + log.info( + "Can't get commits for merge request %s", + self.req.url(), + exc_info=True) + result['commits'] = [] + result['error'] = True + else: + if self.req.commits_task_status() not in ('busy', 'ready'): + allura.tasks.repo_tasks.determine_mr_commits.post(self.req._id) result['commits'] = [] - result['error'] = True + result['commits_task_started'] = True return result @property @@ -515,6 +522,21 @@ class MergeRequestController(object): """Return result from the cache. Used by js, after task was completed.""" return {'can_merge': self.req.can_merge()} + @expose() + def commits_html(self, **kw): + if self.req.new_commits is not None: + with self.req.push_downstream_context(): + downstream_app = c.app + return SCMLogWidget().display(value=self.req.commits, app=downstream_app) + + task_status = self.req.commits_task_status() + if task_status is None: + raise exc.HTTPNotFound + elif task_status == 'error': + raise exc.HTTPInternalServerError + elif task_status in ('busy', 'ready'): + raise exc.HTTPAccepted + @expose('json:') @require_post() @validate(subscribe_form) http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/Allura/allura/model/repository.py ---------------------------------------------------------------------- diff --git a/Allura/allura/model/repository.py b/Allura/allura/model/repository.py index 8ec5959..493f38e 100644 --- a/Allura/allura/model/repository.py +++ b/Allura/allura/model/repository.py @@ -953,10 +953,10 @@ class MergeRequest(VersionedArtifact, ActivityObject): from allura.tasks import repo_tasks repo_tasks.merge.post(self._id) - def merge_task_status(self): + def _task_status(self, task_name): task = MonQTask.query.find({ 'state': {'$in': ['busy', 'complete', 'error', 'ready']}, # needed to use index - 'task_name': 'allura.tasks.repo_tasks.merge', + 'task_name': task_name, 'args': [self._id], 'time_queue': {'$gt': datetime.utcnow() - timedelta(days=1)}, # constrain on index further }).sort('_id', -1).limit(1).first() @@ -964,16 +964,14 @@ class MergeRequest(VersionedArtifact, ActivityObject): return task.state return None + def merge_task_status(self): + return self._task_status('allura.tasks.repo_tasks.merge') + def can_merge_task_status(self): - task = MonQTask.query.find({ - 'state': {'$in': ['busy', 'complete', 'error', 'ready']}, # needed to use index - 'task_name': 'allura.tasks.repo_tasks.can_merge', - 'args': [self._id], - 'time_queue': {'$gt': datetime.utcnow() - timedelta(days=1)}, # constrain on index further - }).sort('_id', -1).limit(1).first() - if task: - return task.state - return None + return self._task_status('allura.tasks.repo_tasks.can_merge') + + def commits_task_status(self): + return self._task_status('allura.tasks.repo_tasks.determine_mr_commits') def add_meta_post(self, changes): tmpl = g.jinja2_env.get_template('allura:templates/repo/merge_request_changed.html') http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/Allura/allura/tasks/repo_tasks.py ---------------------------------------------------------------------- diff --git a/Allura/allura/tasks/repo_tasks.py b/Allura/allura/tasks/repo_tasks.py index f84de1a..41a1ba7 100644 --- a/Allura/allura/tasks/repo_tasks.py +++ b/Allura/allura/tasks/repo_tasks.py @@ -169,3 +169,10 @@ def can_merge(merge_request_id): mr = M.MergeRequest.query.get(_id=merge_request_id) result = mr.app.repo.can_merge(mr) mr.set_can_merge_cache(result) + + +@task +def determine_mr_commits(merge_request_id): + from allura import model as M + mr = M.MergeRequest.query.get(_id=merge_request_id) + mr.commits # build & cache the commits http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/Allura/allura/templates/repo/merge_request.html ---------------------------------------------------------------------- diff --git a/Allura/allura/templates/repo/merge_request.html b/Allura/allura/templates/repo/merge_request.html index c408ba4..2b8e70d 100644 --- a/Allura/allura/templates/repo/merge_request.html +++ b/Allura/allura/templates/repo/merge_request.html @@ -119,9 +119,16 @@ Merge Request #{{req.request_number}}: {{req.summary}} ({{req.status}}) - {{ c.log_widget.display(value=commits, app=downstream_app) }} - -
Discuss
+ {% if commits_task_started %} +
+

+ + Determining commits... +

+
+ {% else %} + {{ c.log_widget.display(value=commits, app=downstream_app) }} + {% endif %} {% if h.has_access(c.app, 'write')() %}
@@ -171,6 +178,7 @@ Merge Request #{{req.request_number}}: {{req.summary}} ({{req.status}}) .merge-conflicts { color: red; } .can-merge-in-progress { color: grey; } .merge-instructions { width:80%; height:60px; } + .merge-toolbar { padding-bottom: 1em; } #merge_task_status .{{ merge_status }} { display: inline-block; } #can_merge_task_status .{{ can_merge_status }} { display: inline-block; } @@ -186,9 +194,10 @@ Merge Request #{{req.request_number}}: {{req.summary}} ({{req.status}}) {{ super() }} {% endblock %} http://git-wip-us.apache.org/repos/asf/allura/blob/650e181c/ForgeGit/forgegit/tests/functional/test_controllers.py ---------------------------------------------------------------------- diff --git a/ForgeGit/forgegit/tests/functional/test_controllers.py b/ForgeGit/forgegit/tests/functional/test_controllers.py index b4b6f9b..fb4377d 100644 --- a/ForgeGit/forgegit/tests/functional/test_controllers.py +++ b/ForgeGit/forgegit/tests/functional/test_controllers.py @@ -619,21 +619,12 @@ class TestFork(_TestCase): def test_merge_request_detail_view(self): r, mr_num = self._request_merge() - assert 'wants to merge' in r, r.showbrowser() - assert 'Improve documentation' in r, r.showbrowser() - revs = r.html.findAll('tr', attrs={'class': 'rev'}) - assert_equal(len(revs), 1) - rev_links = revs[0].findAll('a', attrs={'class': 'rev'}) - browse_links = revs[0].findAll('a', attrs={'class': 'browse'}) - c_id = self.forked_repo.get_heads()[0]['object_id'] - assert_equal(rev_links[0].get('href'), '/p/test2/code/ci/%s/' % c_id) - assert_equal(rev_links[0].getText(), '[%s]' % c_id[:6]) - assert_equal(browse_links[0].get('href'), - '/p/test2/code/ci/%s/tree' % c_id) - assert_equal(browse_links[0].getText(), 'Tree') + assert_in('wants to merge', r) + merge_instructions = r.html.findAll('textarea')[0].getText() assert_in('git checkout master', merge_instructions) assert_in('git fetch /srv/git/p/test2/code master', merge_instructions) + c_id = self.forked_repo.get_heads()[0]['object_id'] assert_in('git merge {}'.format(c_id), merge_instructions) assert_regexp_matches(str(r), r'[0-9]+ seconds? ago') @@ -641,6 +632,33 @@ class TestFork(_TestCase): assert merge_form assert_in('Merge request has no conflicts. You can merge automatically.', merge_form.getText()) + assert_not_in('Improve documentation', r) # no details yet + + # a task is busy/ready to compute + r = self.app.get('/p/test/src-git/merge-requests/1/commits_html', status=202) # 202 used for "busy" + # run task to compute the commits list + task = M.MonQTask.query.get(task_name='allura.tasks.repo_tasks.determine_mr_commits', state='ready') + task() + ThreadLocalORMSession.close_all() # close ming connections so that new data gets loaded later + + def assert_commit_details(r): + assert_in('Improve documentation', r.body) + revs = r.html.findAll('tr', attrs={'class': 'rev'}) + assert_equal(len(revs), 1) + rev_links = revs[0].findAll('a', attrs={'class': 'rev'}) + browse_links = revs[0].findAll('a', attrs={'class': 'browse'}) + assert_equal(rev_links[0].get('href'), '/p/test2/code/ci/%s/' % c_id) + assert_equal(rev_links[0].getText(), '[%s]' % c_id[:6]) + assert_equal(browse_links[0].get('href'), + '/p/test2/code/ci/%s/tree' % c_id) + assert_equal(browse_links[0].getText(), 'Tree') + + r = self.app.get('/p/test/src-git/merge-requests/1/commits_html', status=200) + assert_commit_details(r) + + r = self.app.get('/p/test/src-git/merge-requests/1/', status=200) + assert_commit_details(r) + def test_merge_request_detail_noslash(self): self._request_merge() r = self.app.get('/p/test/src-git/merge-requests/1', status=301) @@ -691,20 +709,32 @@ class TestFork(_TestCase): assert_equal(_select_val(r, 'target_branch'), 'zz') def test_merge_request_with_branch(self): + def get_mr_page(r): + r = r.follow() # get merge request page; creates bg task for determining commits + task = M.MonQTask.query.get(task_name='allura.tasks.repo_tasks.determine_mr_commits', state='ready') + task() + ThreadLocalORMSession.close_all() # close ming connections so that new data gets loaded later + r = self.app.get(r.request.url) # refresh, data should be there now + return r + r = self.app.post('/p/test2/code/do_request_merge', params={ 'source_branch': 'zz', 'target_branch': 'zz', 'summary': 'summary', - 'description': 'description'}).follow() + 'description': 'description'}) + r = get_mr_page(r) assert '[5c4724]' not in r + + # again with different branch r = self.app.post('/p/test2/code/do_request_merge', params={ 'source_branch': 'zz', 'target_branch': 'master', 'summary': 'summary', - 'description': 'description'}).follow() - assert '[5c4724]' in r + 'description': 'description'}) + r = get_mr_page(r) + assert '[5c4724]' in r, r def test_merge_request_edit(self): r = self.app.post('/p/test2/code/do_request_merge', @@ -775,8 +805,9 @@ class TestFork(_TestCase): r, mr_num = self._request_merge() mr_commits.side_effect = Exception r = self.app.get('/p/test/src-git/merge-requests/%s/' % mr_num) - err = r.html.find('div', attrs={'class': 'grid-19 error'}) - assert_in("Can't find commits to merge", err.getText()) + # errors don't show up on the page directly any more, so just assert that the bg task is there + assert_in('commits-loading', r) + self.app.get('/p/test/src-git/merge-requests/%s/commits_html' % mr_num, status=202) # 202 used for "busy" class TestDiff(TestController):