From ca7bbc1a5e95cab2c17867e42668873e475a11b5 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 23 2018 09:43:22 +0000 Subject: Fix notifications and refreshing the cached merge status upon updates Basically, before this commit, when we pushed a new commit to the git repo, we would update all the open pull-requests involved with this branch so that we can link them to tickets, then we would refresh all the open pull-requests of the project (ie: clear out their cached merge status). With this commit, before we link PRs to ticket, we refresh them, so it sends notifications about rebase to the configured bus (if any) and comments about it in the UI. This process also refreshes the cached merge status. So we have to be careful not to clear this updated cached merge status later otherwise we updated it for nothing. So this commit ensures we only clear cached merge status of PRs we have not refreshed manually. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/hooks/default.py b/pagure/hooks/default.py index 9ada8cc..fc35c72 100644 --- a/pagure/hooks/default.py +++ b/pagure/hooks/default.py @@ -145,6 +145,8 @@ def inform_pull_request_urls( if project.is_fork: target_repo = project.parent + pr_uids = [] + if ( commits and refname != default_branch @@ -153,13 +155,25 @@ def inform_pull_request_urls( print() prs = pagure.lib.query.search_pull_requests( session, - project_id_from=project.id, + project_id_from=target_repo.id, status="Open", branch_from=refname, ) + if project.id != target_repo.id: + prs.extend( + pagure.lib.query.search_pull_requests( + session, + project_id_from=project.id, + status="Open", + branch_from=refname, + ) + ) # Link to existing PRs if there are any seen = len(prs) != 0 for pr in prs: + # Refresh the PR in the db and everywhere else where needed + pagure.lib.tasks.update_pull_request.delay(pr.uid) + # Link tickets with pull-requests if the commit mentions it pagure.lib.tasks.link_pr_to_ticket.delay(pr.uid) @@ -169,8 +183,7 @@ def inform_pull_request_urls( " %s/%s/pull-request/%s" % (_config["APP_URL"].rstrip("/"), pr.project.url_path, pr.id) ) - # Refresh the PR in the db and everywhere else where needed - pagure.lib.tasks.update_pull_request.delay(pr.uid) + pr_uids.append(pr.uid) # If no existing PRs, provide the link to open one if not seen: @@ -186,6 +199,8 @@ def inform_pull_request_urls( ) print() + return pr_uids + class DefaultRunner(BaseRunner): """ Runner for the default hook.""" @@ -208,6 +223,8 @@ class DefaultRunner(BaseRunner): if not repo_obj.is_empty and not repo_obj.head_is_unborn: default_branch = repo_obj.head.shorthand + pr_uids = [] + for refname in changes: (oldrev, newrev) = changes[refname] @@ -264,17 +281,21 @@ class DefaultRunner(BaseRunner): # Now display to the user if this isn't the default branch links to # open a new pr or review the existing one - inform_pull_request_urls( - session, project, commits, refname, default_branch + pr_uids.extend( + inform_pull_request_urls( + session, project, commits, refname, default_branch + ) ) - # Schedule refresh of all opened PRs + # Refresh of all opened PRs parent = project.parent or project - pagure.lib.tasks.refresh_pr_cache.delay( + pagure.lib.tasks.refresh_pr_cache( parent.name, parent.namespace, parent.user.user if parent.is_fork else None, + but_uids=pr_uids, ) + if not project.is_on_repospanner and \ _config.get("GIT_GARBAGE_COLLECT", False): pagure.lib.tasks.git_garbage_collect.delay( diff --git a/pagure/lib/query.py b/pagure/lib/query.py index 19c1167..03baa60 100644 --- a/pagure/lib/query.py +++ b/pagure/lib/query.py @@ -3275,13 +3275,20 @@ def close_pull_request(session, request, user, merged=True): ) -def reset_status_pull_request(session, project): +def reset_status_pull_request(session, project, but_uids=None): """ Reset the status of all opened Pull-Requests of a project. """ - session.query(model.PullRequest).filter( - model.PullRequest.project_id == project.id - ).filter(model.PullRequest.status == "Open").update( - {model.PullRequest.merge_status: None} + query = ( + session.query(model.PullRequest) + .filter(model.PullRequest.project_id == project.id) + .filter(model.PullRequest.status == "Open") + ) + + if but_uids: + query = query.filter(model.PullRequest.uid.notin_(but_uids)) + + query.update( + {model.PullRequest.merge_status: None}, synchronize_session=False ) session.commit() diff --git a/pagure/lib/tasks.py b/pagure/lib/tasks.py index b26f284..28546b9 100644 --- a/pagure/lib/tasks.py +++ b/pagure/lib/tasks.py @@ -691,14 +691,16 @@ def move_to_repospanner(self, session, name, namespace, user, region): @conn.task(queue=pagure_config.get("FAST_CELERY_QUEUE", None), bind=True) @pagure_task -def refresh_pr_cache(self, session, name, namespace, user): +def refresh_pr_cache(self, session, name, namespace, user, but_uids=None): """ Refresh the merge status cached of pull-requests. """ project = pagure.lib.query._get_project( session, namespace=namespace, name=name, user=user ) - pagure.lib.query.reset_status_pull_request(session, project) + pagure.lib.query.reset_status_pull_request( + session, project, but_uids=but_uids + ) @conn.task(queue=pagure_config.get("FAST_CELERY_QUEUE", None), bind=True)