From ef1fe1a5158da393189913bb449b20ecb349857f Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Nov 24 2017 08:35:42 +0000 Subject: Make the methods to tag/untag an object work on Issue and PR objects This allows us to reuse most of the code or at least be consistent in our path. There are still some differences between issues, pull-requests and projects but the code should be able to handle all of these fine now. Signed-off-by: Pierre-Yves Chibon --- diff --git a/pagure/api/issue.py b/pagure/api/issue.py index d121b3d..3eba96e 100644 --- a/pagure/api/issue.py +++ b/pagure/api/issue.py @@ -791,10 +791,10 @@ def api_change_status_issue(repo, issueid, username=None, namespace=None): if message: pagure.lib.add_metadata_update_notif( session=SESSION, - issue=issue, + obj=issue, messages=message, user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'] + gitfolder=APP.config['TICKETS_FOLDER'] ) except pagure.exceptions.PagureException as err: raise pagure.exceptions.APIError( @@ -895,10 +895,10 @@ def api_change_milestone_issue(repo, issueid, username=None, namespace=None): if message: pagure.lib.add_metadata_update_notif( session=SESSION, - issue=issue, + obj=issue, messages=message, user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'] + gitfolder=APP.config['TICKETS_FOLDER'] ) except pagure.exceptions.PagureException as err: raise pagure.exceptions.APIError( @@ -1065,10 +1065,10 @@ def api_assign_issue(repo, issueid, username=None, namespace=None): if message: pagure.lib.add_metadata_update_notif( session=SESSION, - issue=issue, + obj=issue, messages=message, user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'] + gitfolder=APP.config['TICKETS_FOLDER'] ) output['message'] = message else: @@ -1247,10 +1247,10 @@ def api_update_custom_field( output['message'] = message pagure.lib.add_metadata_update_notif( session=SESSION, - issue=issue, + obj=issue, messages=message, user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'] + gitfolder=APP.config['TICKETS_FOLDER'] ) else: output['message'] = 'No changes' @@ -1371,10 +1371,10 @@ def api_update_custom_fields( output['messages'].append({key.name: message}) pagure.lib.add_metadata_update_notif( session=SESSION, - issue=issue, + obj=issue, messages=message, user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'] + gitfolder=APP.config['TICKETS_FOLDER'] ) else: output['messages'].append({key.name: 'No changes'}) diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 6f4e8c3..a8bcc17 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -384,7 +384,7 @@ def add_issue_comment(session, issue, comment, user, ticketfolder, return 'Comment added' -def add_tag_obj(session, obj, tags, user, ticketfolder): +def add_tag_obj(session, obj, tags, user, gitfolder): ''' Add a tag to an object (either an issue or a project). ''' user_obj = get_user(session, user) @@ -426,10 +426,17 @@ def add_tag_obj(session, obj, tags, user, ticketfolder): session.add(tagobj) session.flush() - dbobjtag = model.TagIssueColored( - issue_uid=obj.uid, - tag_id=tagobj.id - ) + if obj.isa == 'issue': + dbobjtag = model.TagIssueColored( + issue_uid=obj.uid, + tag_id=tagobj.id + ) + else: + dbobjtag = model.TagPullRequest( + request_uid=obj.uid, + tag_id=tagobj.id + ) + added_tags_color.append(tagobj.tag_color) session.add(dbobjtag) @@ -439,7 +446,7 @@ def add_tag_obj(session, obj, tags, user, ticketfolder): if isinstance(obj, model.Issue): pagure.lib.git.update_git( - obj, repo=obj.project, repofolder=ticketfolder) + obj, repo=obj.project, repofolder=gitfolder) if not obj.private: pagure.lib.notify.log( @@ -462,9 +469,35 @@ def add_tag_obj(session, obj, tags, user, ticketfolder): 'added_tags_color': added_tags_color, } )) + elif isinstance(obj, model.PullRequest): + pagure.lib.git.update_git( + obj, repo=obj.project, repofolder=gitfolder) + + if not obj.private: + pagure.lib.notify.log( + obj.project, + topic='pull-request.tag.added', + msg=dict( + pull_request=obj.to_json(public=True), + project=obj.project.to_json(public=True), + tags=added_tags, + agent=user_obj.username, + ), + redis=REDIS, + ) + + # Send notification for the event-source server + if REDIS and not obj.project.private: + REDIS.publish('pagure.%s' % obj.uid, json.dumps( + { + 'added_tags': added_tags, + 'added_tags_color': added_tags_color, + } + )) if added_tags: - return 'Issue tagged with: %s' % ', '.join(added_tags) + return '%s tagged with: %s' % ( + obj.isa.capitalize(), ', '.join(added_tags)) else: return 'Nothing to add' @@ -720,7 +753,7 @@ def remove_issue_dependency( [str(id) for id in parent_del]) -def remove_tags(session, project, tags, ticketfolder, user): +def remove_tags(session, project, tags, gitfolder, user): ''' Removes the specified tag of a project. ''' user_obj = get_user(session, user) @@ -751,7 +784,7 @@ def remove_tags(session, project, tags, ticketfolder, user): tag = issue_tag.tag session.delete(issue_tag) pagure.lib.git.update_git( - issue, repo=issue.project, repofolder=ticketfolder) + issue, repo=issue.project, repofolder=gitfolder) pagure.lib.notify.log( project, @@ -767,7 +800,7 @@ def remove_tags(session, project, tags, ticketfolder, user): return msgs -def remove_tags_obj(session, obj, tags, ticketfolder, user): +def remove_tags_obj(session, obj, tags, gitfolder, user): ''' Removes the specified tag(s) of a given object. ''' user_obj = get_user(session, user) @@ -781,16 +814,22 @@ def remove_tags_obj(session, obj, tags, ticketfolder, user): tag = objtag.tag removed_tags.append(tag) session.delete(objtag) - else: + elif obj.isa == 'issue': for objtag in obj.tags_issues_colored: if objtag.tag.tag in tags: tag = objtag.tag.tag removed_tags.append(tag) session.delete(objtag) + elif obj.isa == 'pull-request': + for objtag in obj.tags_pr_colored: + if objtag.tag.tag in tags: + tag = objtag.tag.tag + removed_tags.append(tag) + session.delete(objtag) if isinstance(obj, model.Issue): pagure.lib.git.update_git( - obj, repo=obj.project, repofolder=ticketfolder) + obj, repo=obj.project, repofolder=gitfolder) pagure.lib.notify.log( obj.project, @@ -808,8 +847,29 @@ def remove_tags_obj(session, obj, tags, ticketfolder, user): if REDIS and not obj.project.private: REDIS.publish('pagure.%s' % obj.uid, json.dumps( {'removed_tags': removed_tags})) + elif isinstance(obj, model.PullRequest): + pagure.lib.git.update_git( + obj, repo=obj.project, repofolder=gitfolder) + + pagure.lib.notify.log( + obj.project, + topic='pull-request.tag.removed', + msg=dict( + pull_request=obj.to_json(public=True), + project=obj.project.to_json(public=True), + tags=removed_tags, + agent=user_obj.username, + ), + redis=REDIS, + ) + + # Send notification for the event-source server + if REDIS and not obj.project.private: + REDIS.publish('pagure.%s' % obj.uid, json.dumps( + {'removed_tags': removed_tags})) - return 'Issue **un**tagged with: %s' % ', '.join(removed_tags) + return '%s **un**tagged with: %s' % ( + obj.isa.capitalize(), ', '.join(removed_tags)) def edit_issue_tags( @@ -3001,7 +3061,7 @@ def avatar_url_from_email(email, size=64, default='retro', dns=False): hashhex, query) -def update_tags(session, obj, tags, username, ticketfolder): +def update_tags(session, obj, tags, username, gitfolder): """ Update the tags of a specified object (adding or removing them). This object can be either an issue or a project. @@ -3018,9 +3078,10 @@ def update_tags(session, obj, tags, username, ticketfolder): obj=obj, tags=toadd, user=username, - ticketfolder=ticketfolder, + gitfolder=gitfolder, ) - messages.append('Issue tagged with: %s' % ', '.join(sorted(toadd))) + messages.append('%s tagged with: %s' % ( + obj.isa.capitalize(), ', '.join(sorted(toadd)))) if torm: remove_tags_obj( @@ -3028,10 +3089,10 @@ def update_tags(session, obj, tags, username, ticketfolder): obj=obj, tags=torm, user=username, - ticketfolder=ticketfolder, + gitfolder=gitfolder, ) - messages.append('Issue **un**tagged with: %s' % ', '.join( - sorted(torm))) + messages.append('%s **un**tagged with: %s' % ( + obj.isa.capitalize(), ', '.join(sorted(torm)))) session.commit() @@ -4366,7 +4427,7 @@ def get_active_milestones(session, project): return sorted([item[0] for item in query.distinct()]) -def add_metadata_update_notif(session, issue, messages, user, ticketfolder): +def add_metadata_update_notif(session, obj, messages, user, gitfolder): ''' Add a notification to the specified issue with the given messages which should reflect changes made to the meta-data of the issue. ''' @@ -4381,16 +4442,25 @@ def add_metadata_update_notif(session, issue, messages, user, ticketfolder): user_obj = get_user(session, user) user_id = user_obj.id - issue_comment = model.IssueComment( - issue_uid=issue.uid, - comment='**Metadata Update from @%s**:\n- %s' % ( - user, '\n- '.join(sorted(messages))), - user_id=user_id, - notification=True, - ) - issue.last_updated = datetime.datetime.utcnow() - session.add(issue) - session.add(issue_comment) + if obj.isa == 'issue': + obj_comment = model.IssueComment( + issue_uid=obj.uid, + comment='**Metadata Update from @%s**:\n- %s' % ( + user, '\n- '.join(sorted(messages))), + user_id=user_id, + notification=True, + ) + elif obj.isa == 'pull-request': + obj_comment = model.PullRequestComment( + pull_request_uid=obj.uid, + comment='**Metadata Update from @%s**:\n- %s' % ( + user, '\n- '.join(sorted(messages))), + user_id=user_id, + notification=True, + ) + obj.last_updated = datetime.datetime.utcnow() + session.add(obj) + session.add(obj_comment) # Make sure we won't have SQLAlchemy error before we continue session.commit() diff --git a/pagure/lib/git.py b/pagure/lib/git.py index f5e98a5..1fdcb21 100644 --- a/pagure/lib/git.py +++ b/pagure/lib/git.py @@ -433,7 +433,7 @@ def get_project_from_json( if tags: pagure.lib.add_tag_obj( session, project, tags=tags, user=user.username, - ticketfolder=None) + gitfolder=None) return project @@ -620,7 +620,7 @@ def update_ticket_from_git( # Update tags tags = json_data.get('tags', []) msgs = pagure.lib.update_tags( - session, issue, tags, username=user.user, ticketfolder=None) + session, issue, tags, username=user.user, gitfolder=None) if msgs: messages.extend(msgs) @@ -668,10 +668,10 @@ def update_ticket_from_git( if messages: pagure.lib.add_metadata_update_notif( session=session, - issue=issue, + obj=issue, messages=messages, user=agent.username, - ticketfolder=None + gitfolder=None ) session.commit() diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index f6395fd..1439581 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -235,7 +235,7 @@ def update_issue(repo, issueid, username=None, namespace=None): msgs = pagure.lib.update_tags( SESSION, issue, tags, username=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'], + gitfolder=APP.config['TICKETS_FOLDER'], ) messages = messages.union(set(msgs)) @@ -340,10 +340,10 @@ def update_issue(repo, issueid, username=None, namespace=None): not_needed = set(['Comment added', 'Updated comment']) pagure.lib.add_metadata_update_notif( session=SESSION, - issue=issue, + obj=issue, messages=messages - not_needed, user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'] + gitfolder=APP.config['TICKETS_FOLDER'] ) messages.add('Metadata fields updated') @@ -567,7 +567,7 @@ def remove_tag(repo, username=None, namespace=None): msgs = pagure.lib.remove_tags( SESSION, repo, tags, user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'] + gitfolder=APP.config['TICKETS_FOLDER'] ) try: @@ -1200,10 +1200,10 @@ def edit_issue(repo, issueid, username=None, namespace=None): if messages: pagure.lib.add_metadata_update_notif( session=SESSION, - issue=issue, + obj=issue, messages=messages, user=flask.g.fas_user.username, - ticketfolder=APP.config['TICKETS_FOLDER'] + gitfolder=APP.config['TICKETS_FOLDER'] ) # If there is a file attached, attach it. diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 5dfc70c..eb4e2f9 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1151,7 +1151,7 @@ def update_project(repo, username=None, namespace=None): SESSION, repo, tags=[t.strip() for t in form.tags.data.split(',')], username=flask.g.fas_user.username, - ticketfolder=None, + gitfolder=None, ) SESSION.add(repo) SESSION.commit() diff --git a/tests/test_pagure_flask_api_project.py b/tests/test_pagure_flask_api_project.py index ab7fe80..6d19539 100644 --- a/tests/test_pagure_flask_api_project.py +++ b/tests/test_pagure_flask_api_project.py @@ -351,7 +351,7 @@ class PagureFlaskApiProjecttests(tests.Modeltests): output = pagure.lib.update_tags( self.session, repo, 'infra', 'pingou', None) - self.assertEqual(output, ['Issue tagged with: infra']) + self.assertEqual(output, ['Project tagged with: infra']) # Check after adding repo = pagure.get_authorized_project(self.session, 'test') @@ -801,8 +801,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): # Adding a tag output = pagure.lib.update_tags( self.session, repo, 'infra', 'pingou', - ticketfolder=None) - self.assertEqual(output, ['Issue tagged with: infra']) + gitfolder=None) + self.assertEqual(output, ['Project tagged with: infra']) # Check after adding repo = pagure.get_authorized_project(self.session, 'test') @@ -871,8 +871,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): # Adding a tag output = pagure.lib.update_tags( self.session, repo, 'infra', 'pingou', - ticketfolder=None) - self.assertEqual(output, ['Issue tagged with: infra']) + gitfolder=None) + self.assertEqual(output, ['Project tagged with: infra']) # Check after adding repo = pagure.get_authorized_project(self.session, 'test') @@ -967,8 +967,8 @@ class PagureFlaskApiProjecttests(tests.Modeltests): # Adding a tag output = pagure.lib.update_tags( self.session, repo, 'infra', 'pingou', - ticketfolder=None) - self.assertEqual(output, ['Issue tagged with: infra']) + gitfolder=None) + self.assertEqual(output, ['Project tagged with: infra']) # Check after adding repo = pagure.get_authorized_project(self.session, 'test') diff --git a/tests/test_pagure_flask_api_ui_private_repo.py b/tests/test_pagure_flask_api_ui_private_repo.py index f5ad6c9..16586c0 100644 --- a/tests/test_pagure_flask_api_ui_private_repo.py +++ b/tests/test_pagure_flask_api_ui_private_repo.py @@ -986,8 +986,8 @@ class PagurePrivateRepotest(tests.Modeltests): # Adding a tag output = pagure.lib.update_tags( self.session, repo, 'infra', 'pingou', - ticketfolder=None) - self.assertEqual(output, ['Issue tagged with: infra']) + gitfolder=None) + self.assertEqual(output, ['Project tagged with: infra']) # Check after adding repo = pagure.lib._get_project(self.session, 'test4') diff --git a/tests/test_pagure_flask_dump_load_ticket.py b/tests/test_pagure_flask_dump_load_ticket.py index fde473f..e11ce4b 100644 --- a/tests/test_pagure_flask_dump_load_ticket.py +++ b/tests/test_pagure_flask_dump_load_ticket.py @@ -133,7 +133,7 @@ class PagureFlaskDumpLoadTicketTests(tests.Modeltests): obj=issue, tags=[' feature ', 'future '], user='pingou', - ticketfolder=repopath, + gitfolder=repopath, ) self.session.commit() self.assertEqual(msg, 'Issue tagged with: feature, future') diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index 27526c6..2111f0b 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -2670,7 +2670,7 @@ class PagureFlaskIssuestests(tests.Modeltests): obj=issue, tags='tag1', user='pingou', - ticketfolder=None) + gitfolder=None) self.session.commit() self.assertEqual(msg, 'Issue tagged with: tag1') @@ -2772,7 +2772,7 @@ class PagureFlaskIssuestests(tests.Modeltests): obj=issue, tags='tag1', user='pingou', - ticketfolder=None) + gitfolder=None) self.session.commit() self.assertEqual(msg, 'Issue tagged with: tag1') diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py index 5e016a3..553dfef 100644 --- a/tests/test_pagure_flask_ui_repo.py +++ b/tests/test_pagure_flask_ui_repo.py @@ -3548,7 +3548,7 @@ index 0000000..fb7093d obj=issue, tags='tag1', user='pingou', - ticketfolder=None) + gitfolder=None) self.session.commit() self.assertEqual(msg, 'Issue tagged with: tag1') diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 57409b5..a7a5d73 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -993,7 +993,7 @@ class PagureLibtests(tests.Modeltests): obj=issue, tags='tag1', user='pingou', - ticketfolder=None) + gitfolder=None) self.session.commit() self.assertEqual(msg, 'Issue tagged with: tag1') @@ -1012,7 +1012,7 @@ class PagureLibtests(tests.Modeltests): obj=issue, tags='tag1', user='pingou', - ticketfolder=None) + gitfolder=None) self.session.commit() self.assertEqual(msg, 'Nothing to add') @@ -1040,14 +1040,14 @@ class PagureLibtests(tests.Modeltests): project=repo, tags='foo', user='pingou', - ticketfolder=None) + gitfolder=None) msgs = pagure.lib.remove_tags( session=self.session, project=repo, tags='tag1', user='pingou', - ticketfolder=None) + gitfolder=None) self.assertEqual(msgs, ['Issue **un**tagged with: tag1']) @@ -1066,7 +1066,7 @@ class PagureLibtests(tests.Modeltests): obj=issue, tags='tag1', user='pingou', - ticketfolder=None) + gitfolder=None) self.assertEqual(msgs, 'Issue **un**tagged with: tag1') @patch('pagure.lib.git.update_git') @@ -1084,8 +1084,8 @@ class PagureLibtests(tests.Modeltests): self.session, repo, tags=['pagure', 'test'], user='pingou', - ticketfolder=None) - self.assertEqual(msg, 'Issue tagged with: pagure, test') + gitfolder=None) + self.assertEqual(msg, 'Project tagged with: pagure, test') self.session.commit() # Check the tags @@ -1098,8 +1098,8 @@ class PagureLibtests(tests.Modeltests): obj=repo, tags='test', user='pingou', - ticketfolder=None) - self.assertEqual(msgs, 'Issue **un**tagged with: test') + gitfolder=None) + self.assertEqual(msgs, 'Project **un**tagged with: test') self.session.commit() # Check the tags @@ -1179,7 +1179,7 @@ class PagureLibtests(tests.Modeltests): obj=issue, tags='tag3', user='pingou', - ticketfolder=None) + gitfolder=None) self.session.commit() self.assertEqual(msg, 'Issue tagged with: tag3') self.assertEqual([tag.tag for tag in issue.tags], ['tag2', 'tag3']) @@ -2026,7 +2026,7 @@ class PagureLibtests(tests.Modeltests): obj=issue, tags='tag1', user='pingou', - ticketfolder=None) + gitfolder=None) self.session.commit() self.assertEqual(msg, 'Issue tagged with: tag1') @@ -2036,7 +2036,7 @@ class PagureLibtests(tests.Modeltests): obj=issue, tags='tag2', user='pingou', - ticketfolder=None) + gitfolder=None) self.session.commit() self.assertEqual(msg, 'Issue tagged with: tag2') @@ -2895,7 +2895,7 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(issue.tags_text, []) messages = pagure.lib.update_tags( - self.session, issue, 'tag', 'pingou', ticketfolder=None) + self.session, issue, 'tag', 'pingou', gitfolder=None) self.assertEqual(messages, ['Issue tagged with: tag']) # after @@ -2909,7 +2909,7 @@ class PagureLibtests(tests.Modeltests): # Replace the tag by two others messages = pagure.lib.update_tags( self.session, issue, ['tag2', 'tag3'], 'pingou', - ticketfolder=None) + gitfolder=None) self.assertEqual( messages, [ 'Issue tagged with: tag2, tag3', diff --git a/tests/test_pagure_lib_drop_issue.py b/tests/test_pagure_lib_drop_issue.py index abc9965..4c5bb4b 100644 --- a/tests/test_pagure_lib_drop_issue.py +++ b/tests/test_pagure_lib_drop_issue.py @@ -120,7 +120,7 @@ class PagureLibDropIssuetests(tests.Modeltests): issue, tags=['red'], username='pingou', - ticketfolder=None, + gitfolder=None, ) self.session.commit() @@ -164,7 +164,7 @@ class PagureLibDropIssuetests(tests.Modeltests): issue, tags=['red'], username='pingou', - ticketfolder=None, + gitfolder=None, ) self.session.commit() self.assertEqual(msgs, ['Issue tagged with: red']) @@ -175,7 +175,7 @@ class PagureLibDropIssuetests(tests.Modeltests): issue, tags=['red'], username='pingou', - ticketfolder=None, + gitfolder=None, ) self.session.commit() self.assertEqual(msgs, ['Issue tagged with: red'])