From 701b8316788756f8ff5a618d77965f000174bc2d Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 14 2015 16:13:36 +0000 Subject: [PATCH 1/6] Add to project the properties of how many tickets/requests are opened For non-repo admins, we even distinguish between public and not-always public tickets. --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index d02a13e..aa2d19b 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -460,6 +460,25 @@ class Issue(BASE): backref=backref( 'issues', cascade="delete, delete-orphan", single_parent=True) ) + + _p = relation( + 'Project', foreign_keys=[project_id], remote_side=[Project.id], + primaryjoin="and_( " + "Project.id==Issue.project_id, " + "Issue.status=='Open')", + backref=backref( + 'open_tickets', cascade="delete, delete-orphan", single_parent=True) + ) + _p2 = relation( + 'Project', foreign_keys=[project_id], remote_side=[Project.id], + primaryjoin="and_( " + "Project.id==Issue.project_id, " + "Issue.status=='Open', " + "Issue.private==False)", + backref=backref( + 'open_tickets_public', cascade="delete, delete-orphan", single_parent=True) + ) + user = relation('User', foreign_keys=[user_id], remote_side=[User.id], backref='issues') assignee = relation('User', foreign_keys=[assignee_id], @@ -796,6 +815,16 @@ class PullRequest(BASE): single_parent=True) project_from = relation( 'Project', foreign_keys=[project_id_from], remote_side=[Project.id]) + + _p = relation( + 'Project', foreign_keys=[project_id], remote_side=[Project.id], + primaryjoin="and_( " + "Project.id==PullRequest.project_id, " + "PullRequest.status=='Open')", + backref=backref( + 'open_requests', cascade="delete, delete-orphan", single_parent=True) + ) + user = relation('User', foreign_keys=[user_id], remote_side=[User.id], backref='pull_requests') assignee = relation('User', foreign_keys=[assignee_id], From d5cbac342c9452c6f186d24fe24432ffbb9e280a Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 14 2015 16:13:36 +0000 Subject: [PATCH 2/6] Display the number of issues/pull-requests opened on the template --- diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 406977d..465af04 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -64,7 +64,9 @@ and repo.settings.get('issue_tracker', True) %}
  • Issues + repo=repo.name) }}">Issues ({{ + repo.open_tickets |length if repo_admin else repo.open_tickets_public | length + }})
  • {% endif %} @@ -72,7 +74,7 @@
  • - Pull-requests + Pull-requests ({{repo.open_requests |length }})
  • {% endif %} From 29d753bdb0640c2e08c393083623f93f8e21d3a6 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 14 2015 16:29:30 +0000 Subject: [PATCH 3/6] Add unit-tests checks for .open_tickets .open_tickets_public, and .open_requests --- diff --git a/tests/test_progit_lib.py b/tests/test_progit_lib.py index 0f00f10..50df3ef 100644 --- a/tests/test_progit_lib.py +++ b/tests/test_progit_lib.py @@ -146,6 +146,8 @@ class PagureLibtests(tests.Modeltests): # Before issues = pagure.lib.search_issues(self.session, repo) self.assertEqual(len(issues), 0) + self.assertEqual(len(repo.open_tickets), 0) + self.assertEqual(len(repo.open_tickets_public), 0) # See where it fails self.assertRaises( @@ -193,6 +195,8 @@ class PagureLibtests(tests.Modeltests): ) self.session.commit() self.assertEqual(msg.title, 'Test issue') + self.assertEqual(len(repo.open_tickets), 1) + self.assertEqual(len(repo.open_tickets_public), 1) msg = pagure.lib.new_issue( session=self.session, @@ -205,6 +209,8 @@ class PagureLibtests(tests.Modeltests): ) self.session.commit() self.assertEqual(msg.title, 'Test issue #2') + self.assertEqual(len(repo.open_tickets), 2) + self.assertEqual(len(repo.open_tickets_public), 2) # After issues = pagure.lib.search_issues(self.session, repo) @@ -218,9 +224,13 @@ class PagureLibtests(tests.Modeltests): p_ugt.return_value = True self.test_new_issue() + repo = pagure.lib.get_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=2) + self.assertEqual(len(repo.open_tickets), 2) + self.assertEqual(len(repo.open_tickets_public), 2) + # Edit the issue msg = pagure.lib.edit_issue( session=self.session, @@ -250,11 +260,14 @@ class PagureLibtests(tests.Modeltests): title='Foo issue #2', content='We should work on this period', status='Invalid', - private=True + private=True, ) self.session.commit() self.assertEqual(msg, 'Successfully edited issue #2') + self.assertEqual(len(repo.open_tickets), 1) + self.assertEqual(len(repo.open_tickets_public), 1) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_add_issue_dependency(self, p_send_email, p_ugt): @@ -1274,6 +1287,8 @@ class PagureLibtests(tests.Modeltests): # Add an extra user to project `foo` repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual(len(repo.open_requests), 0) + msg = pagure.lib.add_user_to_project( session=self.session, project=repo, @@ -1300,6 +1315,7 @@ class PagureLibtests(tests.Modeltests): self.session.commit() self.assertEqual(req.id, 1) self.assertEqual(req.title, 'test pull-request') + self.assertEqual(len(repo.open_requests), 1) @patch('pagure.lib.notify.send_email') def test_add_pull_request_comment(self, mockemail): @@ -1435,6 +1451,8 @@ class PagureLibtests(tests.Modeltests): self.test_new_pull_request() + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual(len(repo.open_requests), 1) request = pagure.lib.search_pull_requests(self.session, requestid=1) pagure.lib.close_pull_request( @@ -1445,6 +1463,8 @@ class PagureLibtests(tests.Modeltests): merged=True, ) self.session.commit() + repo = pagure.lib.get_project(self.session, 'test') + self.assertEqual(len(repo.open_requests), 0) prs = pagure.lib.search_pull_requests( session=self.session, @@ -1605,6 +1625,9 @@ class PagureLibtests(tests.Modeltests): repo = pagure.lib.get_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) + self.assertEqual(len(repo.open_tickets), 2) + self.assertEqual(len(repo.open_tickets_public), 2) + # Create issues to play with msg = pagure.lib.new_issue( session=self.session, @@ -1618,6 +1641,9 @@ class PagureLibtests(tests.Modeltests): self.session.commit() self.assertEqual(msg.title, 'Test issue #3') + self.assertEqual(len(repo.open_tickets), 3) + self.assertEqual(len(repo.open_tickets_public), 2) + # before self.assertEqual(issue.tags_text, []) self.assertEqual(issue.depends_text, []) From 1dbdfea55f488b063454104ae19730b32aa197db Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 15 2015 10:04:04 +0000 Subject: [PATCH 4/6] Make the open_tickets, open_tickets_public and open_request properties This avoids using a relationship instead and allows loading just the row count instead of the entire DB records. --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 82030ed..2ac7776 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -85,6 +85,7 @@ def create_session(db_url, debug=False, pool_recycle=3600): engine = sqlalchemy.create_engine( db_url, echo=debug, pool_recycle=pool_recycle) scopedsession = scoped_session(sessionmaker(bind=engine)) + model.BASE.metadata.bind = scopedsession return scopedsession diff --git a/pagure/lib/model.py b/pagure/lib/model.py index aa2d19b..220e4e2 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -370,6 +370,42 @@ class Project(BASE): ''' Ensures the settings are properly saved. ''' self._settings = json.dumps(settings) + @property + def open_requests(self): + ''' Returns the number of open pull-requests for this project. ''' + return BASE.metadata.bind.query( + PullRequest + ).filter( + self.id == PullRequest.project_id + ).filter( + PullRequest.status == 'Open' + ).count() + + @property + def open_tickets(self): + ''' Returns the number of open tickets for this project. ''' + return BASE.metadata.bind.query( + Issue + ).filter( + self.id == Issue.project_id + ).filter( + Issue.status == 'Open' + ).count() + + @property + def open_tickets_public(self): + ''' Returns the number of open tickets for this project. ''' + return BASE.metadata.bind.query( + Issue + ).filter( + self.id == Issue.project_id + ).filter( + Issue.status == 'Open' + ).filter( + Issue.private == False + ).count() + + def to_json(self, public=False, api=False): ''' Return a representation of the project as JSON. ''' @@ -461,24 +497,6 @@ class Issue(BASE): 'issues', cascade="delete, delete-orphan", single_parent=True) ) - _p = relation( - 'Project', foreign_keys=[project_id], remote_side=[Project.id], - primaryjoin="and_( " - "Project.id==Issue.project_id, " - "Issue.status=='Open')", - backref=backref( - 'open_tickets', cascade="delete, delete-orphan", single_parent=True) - ) - _p2 = relation( - 'Project', foreign_keys=[project_id], remote_side=[Project.id], - primaryjoin="and_( " - "Project.id==Issue.project_id, " - "Issue.status=='Open', " - "Issue.private==False)", - backref=backref( - 'open_tickets_public', cascade="delete, delete-orphan", single_parent=True) - ) - user = relation('User', foreign_keys=[user_id], remote_side=[User.id], backref='issues') assignee = relation('User', foreign_keys=[assignee_id], @@ -816,15 +834,6 @@ class PullRequest(BASE): project_from = relation( 'Project', foreign_keys=[project_id_from], remote_side=[Project.id]) - _p = relation( - 'Project', foreign_keys=[project_id], remote_side=[Project.id], - primaryjoin="and_( " - "Project.id==PullRequest.project_id, " - "PullRequest.status=='Open')", - backref=backref( - 'open_requests', cascade="delete, delete-orphan", single_parent=True) - ) - user = relation('User', foreign_keys=[user_id], remote_side=[User.id], backref='pull_requests') assignee = relation('User', foreign_keys=[assignee_id], diff --git a/pagure/templates/repo_master.html b/pagure/templates/repo_master.html index 465af04..861f9dd 100644 --- a/pagure/templates/repo_master.html +++ b/pagure/templates/repo_master.html @@ -65,7 +65,7 @@
  • Issues ({{ - repo.open_tickets |length if repo_admin else repo.open_tickets_public | length + repo.open_tickets if repo_admin else repo.open_tickets_public }})
  • {% endif %} @@ -74,7 +74,7 @@
  • - Pull-requests ({{repo.open_requests |length }}) + Pull-requests ({{repo.open_requests }})
  • {% endif %} From 980f59f99bc4c21ec1beea9f0bf36787bcc00774 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 15 2015 10:12:18 +0000 Subject: [PATCH 5/6] Use the magic of metadata.bind --- diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 220e4e2..4b8c212 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -72,6 +72,7 @@ def create_tables(db_url, alembic_ini=None, acls=None, debug=False): command.stamp(alembic_cfg, "head") scopedsession = scoped_session(sessionmaker(bind=engine)) + BASE.metadata.bind = scopedsession # Insert the default data into the db create_default_status(scopedsession, acls=acls) return scopedsession @@ -373,8 +374,7 @@ class Project(BASE): @property def open_requests(self): ''' Returns the number of open pull-requests for this project. ''' - return BASE.metadata.bind.query( - PullRequest + return PullRequest.query( ).filter( self.id == PullRequest.project_id ).filter( @@ -384,8 +384,7 @@ class Project(BASE): @property def open_tickets(self): ''' Returns the number of open tickets for this project. ''' - return BASE.metadata.bind.query( - Issue + return Issue.query( ).filter( self.id == Issue.project_id ).filter( @@ -395,8 +394,7 @@ class Project(BASE): @property def open_tickets_public(self): ''' Returns the number of open tickets for this project. ''' - return BASE.metadata.bind.query( - Issue + return Issue.query( ).filter( self.id == Issue.project_id ).filter( From 3c5ebcf2e7a87afa9fc26b15d82c3a2f85b62afb Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 15 2015 10:12:31 +0000 Subject: [PATCH 6/6] Adjust the unit-tests for the change in API (relationship to property) --- diff --git a/tests/test_progit_lib.py b/tests/test_progit_lib.py index 50df3ef..ef54583 100644 --- a/tests/test_progit_lib.py +++ b/tests/test_progit_lib.py @@ -146,8 +146,8 @@ class PagureLibtests(tests.Modeltests): # Before issues = pagure.lib.search_issues(self.session, repo) self.assertEqual(len(issues), 0) - self.assertEqual(len(repo.open_tickets), 0) - self.assertEqual(len(repo.open_tickets_public), 0) + self.assertEqual(repo.open_tickets, 0) + self.assertEqual(repo.open_tickets_public, 0) # See where it fails self.assertRaises( @@ -195,8 +195,8 @@ class PagureLibtests(tests.Modeltests): ) self.session.commit() self.assertEqual(msg.title, 'Test issue') - self.assertEqual(len(repo.open_tickets), 1) - self.assertEqual(len(repo.open_tickets_public), 1) + self.assertEqual(repo.open_tickets, 1) + self.assertEqual(repo.open_tickets_public, 1) msg = pagure.lib.new_issue( session=self.session, @@ -209,8 +209,8 @@ class PagureLibtests(tests.Modeltests): ) self.session.commit() self.assertEqual(msg.title, 'Test issue #2') - self.assertEqual(len(repo.open_tickets), 2) - self.assertEqual(len(repo.open_tickets_public), 2) + self.assertEqual(repo.open_tickets, 2) + self.assertEqual(repo.open_tickets_public, 2) # After issues = pagure.lib.search_issues(self.session, repo) @@ -228,8 +228,8 @@ class PagureLibtests(tests.Modeltests): repo = pagure.lib.get_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=2) - self.assertEqual(len(repo.open_tickets), 2) - self.assertEqual(len(repo.open_tickets_public), 2) + self.assertEqual(repo.open_tickets, 2) + self.assertEqual(repo.open_tickets_public, 2) # Edit the issue msg = pagure.lib.edit_issue( @@ -265,8 +265,8 @@ class PagureLibtests(tests.Modeltests): self.session.commit() self.assertEqual(msg, 'Successfully edited issue #2') - self.assertEqual(len(repo.open_tickets), 1) - self.assertEqual(len(repo.open_tickets_public), 1) + self.assertEqual(repo.open_tickets, 1) + self.assertEqual(repo.open_tickets_public, 1) @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') @@ -1287,7 +1287,7 @@ class PagureLibtests(tests.Modeltests): # Add an extra user to project `foo` repo = pagure.lib.get_project(self.session, 'test') - self.assertEqual(len(repo.open_requests), 0) + self.assertEqual(repo.open_requests, 0) msg = pagure.lib.add_user_to_project( session=self.session, @@ -1315,7 +1315,7 @@ class PagureLibtests(tests.Modeltests): self.session.commit() self.assertEqual(req.id, 1) self.assertEqual(req.title, 'test pull-request') - self.assertEqual(len(repo.open_requests), 1) + self.assertEqual(repo.open_requests, 1) @patch('pagure.lib.notify.send_email') def test_add_pull_request_comment(self, mockemail): @@ -1452,7 +1452,7 @@ class PagureLibtests(tests.Modeltests): self.test_new_pull_request() repo = pagure.lib.get_project(self.session, 'test') - self.assertEqual(len(repo.open_requests), 1) + self.assertEqual(repo.open_requests, 1) request = pagure.lib.search_pull_requests(self.session, requestid=1) pagure.lib.close_pull_request( @@ -1464,7 +1464,7 @@ class PagureLibtests(tests.Modeltests): ) self.session.commit() repo = pagure.lib.get_project(self.session, 'test') - self.assertEqual(len(repo.open_requests), 0) + self.assertEqual(repo.open_requests, 0) prs = pagure.lib.search_pull_requests( session=self.session, @@ -1625,8 +1625,8 @@ class PagureLibtests(tests.Modeltests): repo = pagure.lib.get_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) - self.assertEqual(len(repo.open_tickets), 2) - self.assertEqual(len(repo.open_tickets_public), 2) + self.assertEqual(repo.open_tickets, 2) + self.assertEqual(repo.open_tickets_public, 2) # Create issues to play with msg = pagure.lib.new_issue( @@ -1641,8 +1641,8 @@ class PagureLibtests(tests.Modeltests): self.session.commit() self.assertEqual(msg.title, 'Test issue #3') - self.assertEqual(len(repo.open_tickets), 3) - self.assertEqual(len(repo.open_tickets_public), 2) + self.assertEqual(repo.open_tickets, 3) + self.assertEqual(repo.open_tickets_public, 2) # before self.assertEqual(issue.tags_text, [])