diff --git a/data/model/notification.py b/data/model/notification.py index 409d6c000..c42ddd07d 100644 --- a/data/model/notification.py +++ b/data/model/notification.py @@ -1,7 +1,5 @@ import json -from peewee import JOIN_LEFT_OUTER - from data.model import InvalidNotificationException, db_transaction from data.database import (Notification, NotificationKind, User, Team, TeamMember, TeamRole, RepositoryNotification, ExternalNotificationEvent, Repository, @@ -17,7 +15,7 @@ def create_notification(kind_name, target, metadata={}): def create_unique_notification(kind_name, target, metadata={}): with db_transaction(): - if list_notifications(target, kind_name, limit=1).count() == 0: + if list_notifications(target, kind_name).count() == 0: create_notification(kind_name, target, metadata) @@ -31,45 +29,44 @@ def lookup_notification(user, uuid): def list_notifications(user, kind_name=None, id_filter=None, include_dismissed=False, page=None, limit=None): + + base_query = Notification.select().join(NotificationKind) + + if kind_name is not None: + base_query = base_query.where(NotificationKind.name == kind_name) + + if id_filter is not None: + base_query = base_query.where(Notification.uuid == id_filter) + + if not include_dismissed: + base_query = base_query.where(Notification.dismissed == False) + + # Lookup directly for the user. + user_direct = base_query.clone().where(Notification.target == user) + + # Lookup via organizations admined by the user. Org = User.alias() AdminTeam = Team.alias() AdminTeamMember = TeamMember.alias() AdminUser = User.alias() - query = (Notification.select() - .join(User) - .switch(Notification) - .join(Org, JOIN_LEFT_OUTER, on=(Org.id == Notification.target)) - .join(AdminTeam, JOIN_LEFT_OUTER, on=(Org.id == AdminTeam.organization)) - .join(TeamRole, JOIN_LEFT_OUTER, on=(AdminTeam.role == TeamRole.id)) - .switch(AdminTeam) - .join(AdminTeamMember, JOIN_LEFT_OUTER, on=(AdminTeam.id == AdminTeamMember.team)) - .join(AdminUser, JOIN_LEFT_OUTER, on=(AdminTeamMember.user == AdminUser.id)) - .where((Notification.target == user) | - ((AdminUser.id == user) & (TeamRole.name == 'admin'))) - .order_by(Notification.created) - .desc()) + via_orgs = (base_query.clone() + .join(Org, on=(Org.id == Notification.target)) + .join(AdminTeam, on=(Org.id == AdminTeam.organization)) + .join(TeamRole, on=(AdminTeam.role == TeamRole.id)) + .switch(AdminTeam) + .join(AdminTeamMember, on=(AdminTeam.id == AdminTeamMember.team)) + .join(AdminUser, on=(AdminTeamMember.user == AdminUser.id)) + .where((AdminUser.id == user) & (TeamRole.name == 'admin'))) - if not include_dismissed: - query = query.switch(Notification).where(Notification.dismissed == False) - - if kind_name: - query = (query - .switch(Notification) - .join(NotificationKind) - .where(NotificationKind.name == kind_name)) - - if id_filter: - query = (query - .switch(Notification) - .where(Notification.uuid == id_filter)) + query = user_direct | via_orgs if page: query = query.paginate(page, limit) elif limit: query = query.limit(limit) - return query + return query.order_by(base_query.c.created.desc()) def delete_all_notifications_by_kind(kind_name): diff --git a/data/model/repository.py b/data/model/repository.py index 69274a138..9ce184381 100644 --- a/data/model/repository.py +++ b/data/model/repository.py @@ -254,7 +254,7 @@ def get_visible_repositories(username, namespace=None, include_public=False): return [] query = (Repository - .select(Repository.name, Repository.id, Repository.description, Namespace.username, + .select(Repository.name, Repository.id.alias('id'), Repository.description, Namespace.username, Repository.visibility) .distinct() .switch(Repository) diff --git a/endpoints/api/repository.py b/endpoints/api/repository.py index 96ff8b161..a11c2c441 100644 --- a/endpoints/api/repository.py +++ b/endpoints/api/repository.py @@ -164,7 +164,7 @@ class RepositoryList(ApiResource): # Note: We only limit repositories when there isn't a namespace or starred filter, as they # result in far smaller queries. if not parsed_args['namespace'] and not parsed_args['starred']: - repos, next_page_token = model.modelutil.paginate(repo_query, RepositoryTable, + repos, next_page_token = model.modelutil.paginate(repo_query, repo_query.c, page_token=page_token, limit=REPOS_PER_PAGE) else: repos = list(repo_query) diff --git a/test/test_api_usage.py b/test/test_api_usage.py index cd700fc1c..3367d4ec0 100644 --- a/test/test_api_usage.py +++ b/test/test_api_usage.py @@ -314,11 +314,30 @@ class TestUserNotification(ApiTestCase): assert json['notifications'] assert not json['notifications'][0]['dismissed'] + notification = json['notifications'][0] pjson = self.putJsonResponse(UserNotification, params=dict(uuid=notification['id']), data=dict(dismissed=True)) self.assertEquals(True, pjson['dismissed']) + def test_org_notifications(self): + # Create a notification on the organization. + org = model.user.get_user_or_org(ORGANIZATION) + model.notification.create_notification('test_notification', org, {'org': 'notification'}) + + # Ensure it is visible to the org admin. + self.login(ADMIN_ACCESS_USER) + json = self.getJsonResponse(UserNotificationList) + notification = json['notifications'][0] + + self.assertEquals(notification['kind'], 'test_notification') + self.assertEquals(notification['metadata'], {'org': 'notification'}) + + # Ensure it is not visible to an org member. + self.login(READ_ACCESS_USER) + json = self.getJsonResponse(UserNotificationList) + self.assertEquals(0, len(json['notifications'])) + class TestGetUserPrivateAllowed(ApiTestCase): def test_nonallowed(self):