From 179785f77a3a7a0f27d4c4a110fb0aedea36469a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 09 2017 15:41:19 +0000 Subject: [PATCH 1/3] Fix deleting tickets that have a tag attached We had a bug where we could not delete issues/tickets that had been tagged because of a lack of propagation of the change at the DB level. With this commit we can now safely delete issues/tickets that have been tagged and we have unit-tests to prevent this from happening again. --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 51d9b5e..032bba7 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1194,7 +1194,9 @@ class TagIssueColored(BASE): issue = relation( 'Issue', foreign_keys=[issue_uid], remote_side=[Issue.uid], - backref="tags_issues_colored" + backref=backref( + 'tags_issues_colored', cascade="delete, delete-orphan" + ) ) tag = relation( 'TagColored', foreign_keys=[tag_id], remote_side=[TagColored.id], diff --git a/tests/test_pagure_lib_drop_issue.py b/tests/test_pagure_lib_drop_issue.py new file mode 100644 index 0000000..a473bcf --- /dev/null +++ b/tests/test_pagure_lib_drop_issue.py @@ -0,0 +1,133 @@ +# -*- coding: utf-8 -*- + +""" + (c) 2017 - Copyright Red Hat Inc + + Authors: + Pierre-Yves Chibon + +""" + +__requires__ = ['SQLAlchemy >= 0.8'] +import pkg_resources + +import unittest +import shutil +import sys +import os + +from mock import patch, MagicMock + +sys.path.insert(0, os.path.join(os.path.dirname( + os.path.abspath(__file__)), '..')) + +import pagure.lib +import pagure.lib.model +import tests + + +class PagureLibDropIssuetests(tests.Modeltests): + """ Tests for pagure.lib.drop_issue """ + + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_drop_issue(self, p_send_email, p_ugt): + """ Test the drop_issue of pagure.lib. + + We had an issue where we could not delete issue that had been tagged + with this test, we create two issues, tag one of them and delete + it, ensuring it all goes well. + """ + p_send_email.return_value = True + p_ugt.return_value = True + + tests.create_projects(self.session) + repo = pagure.lib.get_project(self.session, 'test') + + # Before + issues = pagure.lib.search_issues(self.session, repo) + self.assertEqual(len(issues), 0) + self.assertEqual(repo.open_tickets, 0) + self.assertEqual(repo.open_tickets_public, 0) + + # Create two issues to play with + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='We should work on this', + user='pingou', + ticketfolder=None + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + self.assertEqual(repo.open_tickets, 1) + self.assertEqual(repo.open_tickets_public, 1) + + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue #2', + content='We should work on this for the second time', + user='foo', + status='Open', + ticketfolder=None + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #2') + self.assertEqual(repo.open_tickets, 2) + self.assertEqual(repo.open_tickets_public, 2) + + # After + issues = pagure.lib.search_issues(self.session, repo) + self.assertEqual(len(issues), 2) + + # Add tag to the project + pagure.lib.new_tag( + self.session, + 'red', + 'red tag', + '#ff0000', + repo.id + ) + self.session.commit() + + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual( + str(repo.tags_colored), + '[TagColored(id: 1, tag:red, tag_description:red tag, color:#ff0000)]' + ) + + # Add tag to the second issue + issue = pagure.lib.search_issues(self.session, repo, issueid=2) + msgs = pagure.lib.update_tags( + self.session, + issue, + tags=['red'], + username='pingou', + ticketfolder=None, + ) + self.session.commit() + + self.assertEqual(msgs, ['Issue tagged with: red']) + + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual(len(repo.issues), 2) + issue = pagure.lib.search_issues(self.session, repo, issueid=2) + self.assertEqual( + str(issue.tags), + '[TagColored(id: 1, tag:red, tag_description:red tag, color:#ff0000)]' + ) + + # Drop the issue #2 + issue = pagure.lib.search_issues(self.session, repo, issueid=2) + pagure.lib.drop_issue( + self.session, issue, user='pingou', ticketfolder=None) + self.session.commit() + + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual(len(repo.issues), 1) + + +if __name__ == '__main__': + unittest.main(verbosity=2) From a1c98db13cb619328a4456ceffcf7c4ebf142afd Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 09 2017 15:41:19 +0000 Subject: [PATCH 2/3] Enhance the unit-tests around deleting an issue We're just adding a test here checking that if we remove a tagged issue the other tagged issues are not affected (ie: that their tag isn't being removed). --- diff --git a/tests/test_pagure_lib_drop_issue.py b/tests/test_pagure_lib_drop_issue.py index a473bcf..b55132e 100644 --- a/tests/test_pagure_lib_drop_issue.py +++ b/tests/test_pagure_lib_drop_issue.py @@ -128,6 +128,128 @@ class PagureLibDropIssuetests(tests.Modeltests): repo = pagure.lib.get_project(self.session, 'test') self.assertEqual(len(repo.issues), 1) + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_drop_issue_two_issues_one_tag(self, p_send_email, p_ugt): + """ Test the drop_issue of pagure.lib. + + We had an issue where we could not delete issue that had been tagged + with this test, we create two issues, tag them both and delete one + then we check that the other issue is still tagged. + """ + p_send_email.return_value = True + p_ugt.return_value = True + + tests.create_projects(self.session) + repo = pagure.lib.get_project(self.session, 'test') + + # Before + issues = pagure.lib.search_issues(self.session, repo) + self.assertEqual(len(issues), 0) + self.assertEqual(repo.open_tickets, 0) + self.assertEqual(repo.open_tickets_public, 0) + + # Create two issues to play with + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue', + content='We should work on this', + user='pingou', + ticketfolder=None + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue') + self.assertEqual(repo.open_tickets, 1) + self.assertEqual(repo.open_tickets_public, 1) + + msg = pagure.lib.new_issue( + session=self.session, + repo=repo, + title='Test issue #2', + content='We should work on this for the second time', + user='foo', + status='Open', + ticketfolder=None + ) + self.session.commit() + self.assertEqual(msg.title, 'Test issue #2') + self.assertEqual(repo.open_tickets, 2) + self.assertEqual(repo.open_tickets_public, 2) + + # After + issues = pagure.lib.search_issues(self.session, repo) + self.assertEqual(len(issues), 2) + + # Add tag to the project + pagure.lib.new_tag( + self.session, + 'red', + 'red tag', + '#ff0000', + repo.id + ) + self.session.commit() + + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual( + str(repo.tags_colored), + '[TagColored(id: 1, tag:red, tag_description:red tag, color:#ff0000)]' + ) + + # Add the tag to both issues + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + msgs = pagure.lib.update_tags( + self.session, + issue, + tags=['red'], + username='pingou', + ticketfolder=None, + ) + self.session.commit() + self.assertEqual(msgs, ['Issue tagged with: red']) + + issue = pagure.lib.search_issues(self.session, repo, issueid=2) + msgs = pagure.lib.update_tags( + self.session, + issue, + tags=['red'], + username='pingou', + ticketfolder=None, + ) + self.session.commit() + self.assertEqual(msgs, ['Issue tagged with: red']) + + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual(len(repo.issues), 2) + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual( + str(issue.tags), + '[TagColored(id: 1, tag:red, tag_description:red tag, color:#ff0000)]' + ) + issue = pagure.lib.search_issues(self.session, repo, issueid=2) + self.assertEqual( + str(issue.tags), + '[TagColored(id: 1, tag:red, tag_description:red tag, color:#ff0000)]' + ) + + # Drop the issue #2 + issue = pagure.lib.search_issues(self.session, repo, issueid=2) + pagure.lib.drop_issue( + self.session, issue, user='pingou', ticketfolder=None) + self.session.commit() + + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual(len(repo.issues), 1) + + issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual( + str(issue.tags), + '[TagColored(id: 1, tag:red, tag_description:red tag, color:#ff0000)]' + ) + issue = pagure.lib.search_issues(self.session, repo, issueid=2) + self.assertIsNone(issue) + if __name__ == '__main__': unittest.main(verbosity=2) From 9881b9fd0ebbd6daa8c52ba98128058a2bdfa6de Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Feb 09 2017 15:41:19 +0000 Subject: [PATCH 3/3] Restructure the tests to rely a little more on setUp to reduce code duplication --- diff --git a/tests/test_pagure_lib_drop_issue.py b/tests/test_pagure_lib_drop_issue.py index b55132e..56a103b 100644 --- a/tests/test_pagure_lib_drop_issue.py +++ b/tests/test_pagure_lib_drop_issue.py @@ -31,16 +31,16 @@ class PagureLibDropIssuetests(tests.Modeltests): @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') - def test_drop_issue(self, p_send_email, p_ugt): - """ Test the drop_issue of pagure.lib. - - We had an issue where we could not delete issue that had been tagged - with this test, we create two issues, tag one of them and delete - it, ensuring it all goes well. + def setUp(self, p_send_email, p_ugt): + """ Create a couple of tickets and add tag to the project so we can + play with them later. """ + super(PagureLibDropIssuetests, self).setUp() + p_send_email.return_value = True p_ugt.return_value = True + tests.create_projects(self.session) repo = pagure.lib.get_project(self.session, 'test') @@ -98,6 +98,20 @@ class PagureLibDropIssuetests(tests.Modeltests): '[TagColored(id: 1, tag:red, tag_description:red tag, color:#ff0000)]' ) + @patch('pagure.lib.git.update_git') + @patch('pagure.lib.notify.send_email') + def test_drop_issue(self, p_send_email, p_ugt): + """ Test the drop_issue of pagure.lib. + + We had an issue where we could not delete issue that had been tagged + with this test, we create two issues, tag one of them and delete + it, ensuring it all goes well. + """ + p_send_email.return_value = True + p_ugt.return_value = True + + repo = pagure.lib.get_project(self.session, 'test') + # Add tag to the second issue issue = pagure.lib.search_issues(self.session, repo, issueid=2) msgs = pagure.lib.update_tags( @@ -140,63 +154,8 @@ class PagureLibDropIssuetests(tests.Modeltests): p_send_email.return_value = True p_ugt.return_value = True - tests.create_projects(self.session) repo = pagure.lib.get_project(self.session, 'test') - # Before - issues = pagure.lib.search_issues(self.session, repo) - self.assertEqual(len(issues), 0) - self.assertEqual(repo.open_tickets, 0) - self.assertEqual(repo.open_tickets_public, 0) - - # Create two issues to play with - msg = pagure.lib.new_issue( - session=self.session, - repo=repo, - title='Test issue', - content='We should work on this', - user='pingou', - ticketfolder=None - ) - self.session.commit() - self.assertEqual(msg.title, 'Test issue') - self.assertEqual(repo.open_tickets, 1) - self.assertEqual(repo.open_tickets_public, 1) - - msg = pagure.lib.new_issue( - session=self.session, - repo=repo, - title='Test issue #2', - content='We should work on this for the second time', - user='foo', - status='Open', - ticketfolder=None - ) - self.session.commit() - self.assertEqual(msg.title, 'Test issue #2') - self.assertEqual(repo.open_tickets, 2) - self.assertEqual(repo.open_tickets_public, 2) - - # After - issues = pagure.lib.search_issues(self.session, repo) - self.assertEqual(len(issues), 2) - - # Add tag to the project - pagure.lib.new_tag( - self.session, - 'red', - 'red tag', - '#ff0000', - repo.id - ) - self.session.commit() - - repo = pagure.lib.get_project(self.session, 'test') - self.assertEqual( - str(repo.tags_colored), - '[TagColored(id: 1, tag:red, tag_description:red tag, color:#ff0000)]' - ) - # Add the tag to both issues issue = pagure.lib.search_issues(self.session, repo, issueid=1) msgs = pagure.lib.update_tags(