From b712bcce668a5c2c38b7aa8f388dea1e7e50fcd4 Mon Sep 17 00:00:00 2001 From: Lukas Brabec Date: Jan 12 2021 15:46:50 +0000 Subject: cli: add close-inactive-discussions command This allows to close Pagure tickets related to inactive releases (which haven't been processed yet). Co-authored-by: Kamil Páral Fixes: https://pagure.io/fedora-qa/blockerbugs/issue/114 Merges: https://pagure.io/fedora-qa/blockerbugs/pull-request/165 --- diff --git a/alembic/versions/4eac64cdecd3_added_discussions_closed_column_to_.py b/alembic/versions/4eac64cdecd3_added_discussions_closed_column_to_.py new file mode 100644 index 0000000..bf0acd3 --- /dev/null +++ b/alembic/versions/4eac64cdecd3_added_discussions_closed_column_to_.py @@ -0,0 +1,27 @@ +"""Added discussions_closed column to releases + +Revision ID: 4eac64cdecd3 +Revises: c88341bbda26 +Create Date: 2021-01-11 19:59:28.960876 + +""" + +# revision identifiers, used by Alembic. +revision = '4eac64cdecd3' +down_revision = 'c88341bbda26' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('release', sa.Column('discussions_closed', sa.Boolean(), + server_default=sa.sql.expression.false(), nullable=False)) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('release', 'discussions_closed') + # ### end Alembic commands ### diff --git a/blockerbugs/cli.py b/blockerbugs/cli.py index 60ba89e..c1655c9 100644 --- a/blockerbugs/cli.py +++ b/blockerbugs/cli.py @@ -220,11 +220,16 @@ def recreate_discussion(args): discussion_sync.recreate_discussion(bugid) + def update_discussion(args): ticket_id = int(args.ticket_id) pagure_bot.webhook_handler(ticket_id) +def close_inactive_discussions(args): + discussion_sync.close_discussions_inactive_releases(args.dryrun) + + def main(): parser = ArgumentParser() @@ -302,12 +307,20 @@ def main(): recreate_discussion_parser.set_defaults(func=recreate_discussion) update_discussion_parser = subparsers.add_parser('update-discussion', - help='Update discussion for a given ticket') + help='Update discussion for a given ticket') update_discussion_parser.add_argument('ticket_id', metavar='', help='Ticket ID for which to update dicsussion') update_discussion_parser.set_defaults(func=update_discussion) + close_inactive_discussions_parser = subparsers.add_parser('close-inactive-discussions', + help="Close discussions of inactive releases (which haven't been processed yet)") + close_inactive_discussions_parser.add_argument('--dryrun', action='store_true', + default=False, + help="Don't make any actual changes") + close_inactive_discussions_parser.set_defaults(func=close_inactive_discussions) + + args = parser.parse_args() if not hasattr(args, 'func'): diff --git a/blockerbugs/models/release.py b/blockerbugs/models/release.py index 4887bb6..2d408c4 100644 --- a/blockerbugs/models/release.py +++ b/blockerbugs/models/release.py @@ -27,11 +27,13 @@ class Release(db.Model): id = db.Column(db.Integer, primary_key=True) number = db.Column(db.Integer) active = db.Column(db.Boolean) + discussions_closed = db.Column(db.Boolean) last_update_sync = db.Column(db.DateTime) - def __init__(self, number, active=True): + def __init__(self, number, active=True, discussions_closed=False): self.number = number self.active = active + self.discussions_closed = discussions_closed # initialize last_update_time so there's at least something there self.last_update_sync = datetime(1990, 1, 1, 1, 0, 0, 0) diff --git a/blockerbugs/util/discussion_sync.py b/blockerbugs/util/discussion_sync.py index 2d4a4c0..7830958 100644 --- a/blockerbugs/util/discussion_sync.py +++ b/blockerbugs/util/discussion_sync.py @@ -4,6 +4,7 @@ from blockerbugs import db, app from blockerbugs.config import Config as bb_Config from blockerbugs.util import pagure_interface from blockerbugs.models.milestone import Milestone +from blockerbugs.models.release import Release from blockerbugs.models.bug import Bug @@ -76,3 +77,57 @@ def recreate_discussion(bugid): for milestone in milestones: bugs = Bug.query.filter_by(bugid=bugid, milestone=milestone).all() create_discussions_links(milestone, bugs) + + +def close_discussions_inactive_releases(dry_run=False): + '''Close all Pagure discussion tickets for all inactive releases which are + not yet marked with ``Release.discussions_closed=True``. + ''' + app.logger.info('Closing discussion tickets in inactive releases...') + inactive_releases = Release.query.filter_by(active=False, discussions_closed=False).all() + + for inactive_release in inactive_releases: + app.logger.info(f'Closing discussion tickets in release F{inactive_release.number}...') + all_closed = True + milestones = Milestone.query.filter_by(release=inactive_release).all() + for milestone in milestones: + bugs = Bug.query.filter_by(milestone=milestone) + for bug in bugs: + if not bug.discussion_link: + app.logger.debug(f"Skipping bug {bug.bugid}, discussion doesn't exist") + continue + + try: + # FIXME investigate storing issue_id directly in db instead of complete url + issue_id = bug.discussion_link.split('/')[-1] + app.logger.debug(f'Closing Pagure discussion #{issue_id} for bug {bug.bugid}') + + status = pagure_interface.get_issue(issue_id)['status'] + if status == "Closed": + app.logger.debug(f"Discussion issue #{issue_id} already closed, skipping") + continue + + comment = f"Release F{inactive_release.number} is no longer tracked by "\ + f"[BlockerBugs]({app.config['BLOCKERBUGS_URL']}), closing this "\ + "ticket." + + if dry_run: + app.logger.debug(f"[DRY RUN] Closing issue #{issue_id}") + else: + app.logger.debug(f"Closing issue #{issue_id}") + pagure_interface.post_comment(issue_id, comment) + pagure_interface.close_issue(issue_id) + except pagure_interface.PagureAPIException as e: + app.logger.error(f'Unable to close Pagure discussion #{issue_id} for bug ' + f'{bug.bugid}. Pagure error: {e}') + all_closed = False + continue + + if all_closed and not dry_run: + app.logger.debug("Setting dicussions_closed=True for release F%d" % + inactive_release.number) + inactive_release.discussions_closed = True + db.session.add(inactive_release) + db.session.commit() + + app.logger.info('Closing discussion tickets in inactive releases done.') diff --git a/blockerbugs/util/pagure_interface.py b/blockerbugs/util/pagure_interface.py index 3327a89..8b9be74 100644 --- a/blockerbugs/util/pagure_interface.py +++ b/blockerbugs/util/pagure_interface.py @@ -85,6 +85,29 @@ def create_issue(title, content, tag=None): return issue_url +def close_issue(issue_id): + change_issue_status(issue_id, 'Closed') + + +def change_issue_status(issue_id, status, close_status=None): + change_issue_status_url = app.config['PAGURE_API'] + \ + app.config['PAGURE_REPO'] + '/issue/%s/status' % issue_id + data = { + 'status': status, + } + if close_status: + data['close_status'] = close_status + + resp = requests.post(change_issue_status_url, + headers={ + 'Authorization': 'token %s' % app.config['PAGURE_REPO_TOKEN'] + }, + data=data) + if resp.status_code != 200: + raise PagureAPIException('Unable to change issue status, response code: %d, ' + 'response text:\n%s' % (resp.status_code, resp.text)) + + def get_issue(issue_id): get_issue_url = app.config['PAGURE_API'] + app.config['PAGURE_REPO'] + '/issue/%s' % issue_id resp = requests.get(get_issue_url) diff --git a/testing/test_discussion_sync.py b/testing/test_discussion_sync.py new file mode 100644 index 0000000..efbca5f --- /dev/null +++ b/testing/test_discussion_sync.py @@ -0,0 +1,111 @@ +import mock +from blockerbugs import db +from testing.test_controllers import add_release, add_milestone, add_bug +from blockerbugs.util import pagure_interface +from blockerbugs.util import discussion_sync +from blockerbugs.models.release import Release + +stub_issue_open = { + 'status': 'Open' +} + +stub_issue_closed = { + 'status': 'Closed' +} + + +class TestInactiveDiscussions(object): + def setup_method(self): + db.session.rollback() + db.drop_all() + db.create_all() + + self.release_a = add_release(134) + self.release_b = add_release(133) + self.release_b.active = False + + self.milestone_a = add_milestone(self.release_a, 'final', 100, 101, '133-final', True) + self.milestone_b = add_milestone(self.release_b, 'final', 200, 201, '134-final', True) + + self.issue_id_a = "1" + self.issue_id_b = "2" + + bug1 = add_bug(9000, 'testbug1', self.milestone_a) + bug1.discussion_link = "https://example.com/blockers/issue/%s" % self.issue_id_a + bug2 = add_bug(9001, 'testbug2', self.milestone_b) + bug2.discussion_link = "https://example.com/blockers/issue/%s" % self.issue_id_b + + db.session.commit() + + def teardown_method(self): + db.session.rollback() + db.drop_all() + + def test_closing_commenting(self, monkeypatch): + stub_get_issue_call = mock.MagicMock(return_value=stub_issue_open) + monkeypatch.setattr(pagure_interface, 'get_issue', stub_get_issue_call) + + stub_comment_call = mock.MagicMock() + monkeypatch.setattr(pagure_interface, 'post_comment', stub_comment_call) + + stub_close_call = mock.MagicMock() + monkeypatch.setattr(pagure_interface, 'close_issue', stub_close_call) + + discussion_sync.close_discussions_inactive_releases() + + assert mock.call(self.issue_id_b) in stub_close_call.call_args_list + assert mock.call(self.issue_id_a) not in stub_close_call.call_args_list + + issues_commented = [c.args[0] for c in stub_comment_call.call_args_list] + assert self.issue_id_b in issues_commented + assert self.issue_id_a not in issues_commented + + def test_issues_closed_no_comment(self, monkeypatch): + stub_get_issue_call = mock.MagicMock(return_value=stub_issue_closed) + monkeypatch.setattr(pagure_interface, 'get_issue', stub_get_issue_call) + + stub_comment_call = mock.MagicMock() + monkeypatch.setattr(pagure_interface, 'post_comment', stub_comment_call) + + monkeypatch.setattr(pagure_interface, 'close_issue', mock.MagicMock()) + + discussion_sync.close_discussions_inactive_releases() + + assert stub_comment_call.call_count == 0 + + def test_dry_run(self, monkeypatch): + stub_get_issue_call = mock.MagicMock(return_value=stub_issue_open) + monkeypatch.setattr(pagure_interface, 'get_issue', stub_get_issue_call) + + stub_comment_call = mock.MagicMock() + monkeypatch.setattr(pagure_interface, 'post_comment', stub_comment_call) + + stub_close_call = mock.MagicMock() + monkeypatch.setattr(pagure_interface, 'close_issue', stub_close_call) + + stub_db = mock.MagicMock() + monkeypatch.setattr(discussion_sync, 'db', stub_db) + + discussion_sync.close_discussions_inactive_releases(dry_run=True) + + assert stub_comment_call.call_count == 0 + assert stub_close_call.call_count == 0 + assert not stub_db.mock_calls + + def test_release_discussions_closed(self, monkeypatch): + stub_get_issue_call = mock.MagicMock(return_value=stub_issue_open) + monkeypatch.setattr(pagure_interface, 'get_issue', stub_get_issue_call) + + stub_comment_call = mock.MagicMock() + monkeypatch.setattr(pagure_interface, 'post_comment', stub_comment_call) + + stub_close_call = mock.MagicMock() + monkeypatch.setattr(pagure_interface, 'close_issue', stub_close_call) + + release = db.session.query(Release).filter_by(number=133).first() + assert not release.discussions_closed + + discussion_sync.close_discussions_inactive_releases() + + release = db.session.query(Release).filter_by(number=133).first() + assert release.discussions_closed