From 886b9dcc0be023b3e289f8ed9bf832f8295d8c96 Mon Sep 17 00:00:00 2001 From: Adam Coldrick Date: Sun, 12 Mar 2017 00:23:18 +0000 Subject: [PATCH] Allow stories and tasks to be filtered by boards/worklists containing them Currently this is only supported for the internal database API surface, with the intention of using it to reduce the amount of work done by the boards and worklists APIs. This patch also names some aliases we create to make debugging less painful. It also modifies the join to the StorySummary query such that the column name is no longer hardcoded. This handles the case when we have filtered out private worklists from the result (when joining to worklists), which results in an anonymous column name we cannot hardcode. Change-Id: I3aaa874ae623a44edd668471794f8146c6dabda7 --- storyboard/api/v1/search/sqlalchemy_impl.py | 34 +++++++---- storyboard/db/api/base.py | 30 ++++++---- storyboard/db/api/stories.py | 62 +++++++++++++++------ storyboard/db/api/tasks.py | 44 +++++++++++---- 4 files changed, 119 insertions(+), 51 deletions(-) diff --git a/storyboard/api/v1/search/sqlalchemy_impl.py b/storyboard/api/v1/search/sqlalchemy_impl.py index 2ddee57a..84f87178 100644 --- a/storyboard/api/v1/search/sqlalchemy_impl.py +++ b/storyboard/api/v1/search/sqlalchemy_impl.py @@ -77,6 +77,7 @@ class SqlAlchemySearchImpl(search_engine.SearchEngine): tags=tags, updated_since=updated_since, tags_filter_type=tags_filter_type, + current_user=current_user, session=session) # Filter by subscriber ID @@ -89,17 +90,24 @@ class SqlAlchemySearchImpl(search_engine.SearchEngine): subs = subs.subquery() subquery = subquery.join(subs, subs.c.target_id == models.Story.id) - subquery = self._build_fulltext_search(models.Story, subquery, q) + # Make a query that isn't full of aliases so that fulltext works + query = api_base.model_query(models.Story) + query = api_base.apply_query_filters( + query=query, + model=models.Story, + id=[story.id for story in subquery.all()]) - # Filter out stories that the current user can't see - subquery = api_base.filter_private_stories(subquery, current_user) + clean_query = self._build_fulltext_search(models.Story, query, q) - subquery = subquery.subquery('stories_with_idx') + # Turn the whole shebang into a subquery. + clean_query = clean_query.subquery('filtered_stories') + # Return the story summary. query = api_base.model_query(models.StorySummary)\ .options(subqueryload(models.StorySummary.tags)) - query = query.join(subquery, - models.StorySummary.id == subquery.c.stories_id) + id_col = tuple(clean_query.c)[0] + query = query.join(clean_query, + models.StorySummary.id == id_col) if status: query = query.filter(models.StorySummary.status.in_(status)) @@ -120,7 +128,7 @@ class SqlAlchemySearchImpl(search_engine.SearchEngine): sort_field='id', sort_dir='asc'): session = api_base.get_session() - query = tasks_api.task_build_query( + subquery = tasks_api.task_build_query( project_group_id=project_group_id, story_id=story_id, assignee_id=assignee_id, @@ -128,13 +136,17 @@ class SqlAlchemySearchImpl(search_engine.SearchEngine): branch_id=branch_id, milestone_id=milestone_id, status=status, + current_user=current_user, session=session) - query = self._build_fulltext_search(models.Task, query, q) + # Make a query that isn't full of aliases so that fulltext works + clean_query = api_base.model_query(models.Task) + clean_query = api_base.apply_query_filters( + query=clean_query, + model=models.Task, + id=[task.id for task in subquery.all()]) - # Filter out stories that the current user can't see - query = query.outerjoin(models.Story) - query = api_base.filter_private_stories(query, current_user) + query = self._build_fulltext_search(models.Task, clean_query, q) query = self._apply_pagination( models.Task, diff --git a/storyboard/db/api/base.py b/storyboard/db/api/base.py index 7253dec2..ca567251 100644 --- a/storyboard/db/api/base.py +++ b/storyboard/db/api/base.py @@ -409,7 +409,7 @@ def filter_private_stories(query, current_user, story_model=models.Story): ) # Now filter based on membership of teams with permissions - users = aliased(models.User) + users = aliased(models.User, name="story_users") query = query.outerjoin(models.team_permissions, models.Team, models.team_membership, @@ -448,13 +448,17 @@ def filter_private_worklists(query, current_user, hide_lanes=True): """ # Alias all the things we're joining, in case they've been # joined already. - board_worklists = aliased(models.BoardWorklist) - boards = aliased(models.Board) - board_permissions = aliased(models.board_permissions) - worklist_permissions = aliased(models.worklist_permissions) - permissions = aliased(models.Permission) - user_permissions = aliased(models.user_permissions) - users = aliased(models.User) + board_worklists = aliased(models.BoardWorklist, + name="worklist_boardworklists") + boards = aliased(models.Board, name="worklist_boards") + board_permissions = aliased(models.board_permissions, + name="worklist_boardpermissions") + worklist_permissions = aliased(models.worklist_permissions, + name="worklist_worklistpermissions") + permissions = aliased(models.Permission, name="worklist_permissions") + user_permissions = aliased(models.user_permissions, + name="worklist_userpermissions") + users = aliased(models.User, name="worklist_users") # Worklists permissions must be inherited from the board which # contains the list (if any). To handle this we split the query @@ -527,10 +531,12 @@ def filter_private_boards(query, current_user): :param current_user: The ID of the user requesting the result. """ - board_permissions = aliased(models.board_permissions) - permissions = aliased(models.Permission) - user_permissions = aliased(models.user_permissions) - users = aliased(models.User) + board_permissions = aliased(models.board_permissions, + name="board_boardpermissions") + permissions = aliased(models.Permission, name="board_permissions") + user_permissions = aliased(models.user_permissions, + name="board_userpermissions") + users = aliased(models.User, name="board_users") query = (query .outerjoin((board_permissions, diff --git a/storyboard/db/api/stories.py b/storyboard/db/api/stories.py index 1a5f9767..88f9850e 100644 --- a/storyboard/db/api/stories.py +++ b/storyboard/db/api/stories.py @@ -69,7 +69,7 @@ def story_get(story_id, session=None, current_user=None): def story_get_all(title=None, description=None, status=None, assignee_id=None, creator_id=None, project_group_id=None, project_id=None, subscriber_id=None, tags=None, updated_since=None, - marker=None, offset=None, + board_id=None, worklist_id=None, marker=None, offset=None, limit=None, tags_filter_type="all", sort_field='id', sort_dir='asc', current_user=None): # Sanity checks, in case someone accidentally explicitly passes in 'None' @@ -90,11 +90,11 @@ def story_get_all(title=None, description=None, status=None, assignee_id=None, project_id=project_id, tags=tags, updated_since=updated_since, + board_id=board_id, + worklist_id=worklist_id, + current_user=current_user, tags_filter_type=tags_filter_type) - # Filter out stories that the current user can't see - subquery = api_base.filter_private_stories(subquery, current_user) - # Filter by subscriber ID if subscriber_id is not None: subs = api_base.model_query(models.Subscription) @@ -111,8 +111,9 @@ def story_get_all(title=None, description=None, status=None, assignee_id=None, # Return the story summary. query = api_base.model_query(models.StorySummary)\ .options(subqueryload(models.StorySummary.tags)) + id_col = tuple(subquery.c)[0] query = query.join(subquery, - models.StorySummary.id == subquery.c.stories_id) + models.StorySummary.id == id_col) if status: query = query.filter(models.StorySummary.status.in_(status)) @@ -143,10 +144,8 @@ def story_get_count(title=None, description=None, status=None, project_id=project_id, updated_since=updated_since, tags=tags, - tags_filter_type=tags_filter_type) - - # Filter out stories that the current user can't see - query = api_base.filter_private_stories(query, current_user) + tags_filter_type=tags_filter_type, + current_user=current_user) # Filter by subscriber ID if subscriber_id is not None: @@ -158,14 +157,18 @@ def story_get_count(title=None, description=None, status=None, subs = subs.subquery() query = query.join(subs, subs.c.target_id == models.Story.id) - # If we're also asking for status, we have to attach storysummary here, - # since story status is derived. + # Turn the whole shebang into a subquery. + subquery = query.subquery('filtered_stories') + + # Return the story summary. + query = api_base.model_query(models.StorySummary)\ + .options(subqueryload(models.StorySummary.tags)) + id_col = tuple(subquery.c)[0] + query = query.join(subquery, + models.StorySummary.id == id_col) + if status: - query = query.subquery() - summary_query = api_base.model_query(models.StorySummary) - summary_query = summary_query \ - .join(query, models.StorySummary.id == query.c.stories_id) - query = summary_query.filter(models.StorySummary.status.in_(status)) + query = query.filter(models.StorySummary.status.in_(status)) return query.count() @@ -173,7 +176,8 @@ def story_get_count(title=None, description=None, status=None, def _story_build_query(title=None, description=None, assignee_id=None, creator_id=None, project_group_id=None, project_id=None, updated_since=None, tags=None, - tags_filter_type='all', session=None): + board_id=None, worklist_id=None, tags_filter_type='all', + current_user=None, session=None): # First build a standard story query. query = api_base.model_query(models.Story.id, session=session).distinct() @@ -186,6 +190,9 @@ def _story_build_query(title=None, description=None, assignee_id=None, if updated_since: query = query.filter(models.Story.updated_at > updated_since) + # Filter out stories that the current user can't see + query = api_base.filter_private_stories(query, current_user) + # Filtering by tags if tags: if tags_filter_type == 'all': @@ -216,6 +223,27 @@ def _story_build_query(title=None, description=None, assignee_id=None, if project_id: query = query.filter(models.Task.project_id == project_id) + if worklist_id or board_id: + query = query.outerjoin( + (models.WorklistItem, + models.WorklistItem.item_id == models.Story.id)) + query = query.filter(models.WorklistItem.item_type == "story") + query = query.outerjoin(models.Worklist) + + # Filter by worklist + if worklist_id: + query = query.filter(models.Worklist.id == worklist_id) + query = api_base.filter_private_worklists( + query, current_user, hide_lanes=False) + + # Filter by board + if board_id: + query = query.outerjoin(models.BoardWorklist, models.Board) + query = api_base.filter_private_boards(query, current_user) + query = query.filter(models.Board.id == board_id) + query = api_base.filter_private_worklists( + query, current_user, hide_lanes=False) + return query.distinct() diff --git a/storyboard/db/api/tasks.py b/storyboard/db/api/tasks.py index af43c472..ec04f0ca 100644 --- a/storyboard/db/api/tasks.py +++ b/storyboard/db/api/tasks.py @@ -38,11 +38,9 @@ def task_get_all(marker=None, limit=None, sort_field=None, sort_dir=None, sort_dir = 'asc' # Construct the query - query = task_build_query(project_group_id, **kwargs) - - # Filter out tasks or stories that the current user can't see - query = query.outerjoin(models.Story) - query = api_base.filter_private_stories(query, current_user) + query = task_build_query(project_group_id, + current_user=current_user, + **kwargs) query = api_base.paginate_query(query=query, model=models.Task, @@ -56,11 +54,9 @@ def task_get_all(marker=None, limit=None, sort_field=None, sort_dir=None, def task_get_count(project_group_id=None, current_user=None, **kwargs): - query = task_build_query(project_group_id, **kwargs) - - # Filter out tasks or stories that the current user can't see - query = query.outerjoin(models.Story) - query = api_base.filter_private_stories(query, current_user) + query = task_build_query(project_group_id, + current_user=current_user, + **kwargs) return query.count() @@ -89,7 +85,8 @@ def task_delete(task_id): api_base.entity_hard_delete(models.Task, task_id) -def task_build_query(project_group_id, session=None, **kwargs): +def task_build_query(project_group_id=None, board_id=None, worklist_id=None, + current_user=None, session=None, **kwargs): # Construct the query query = api_base.model_query(models.Task, session=session) @@ -104,6 +101,31 @@ def task_build_query(project_group_id, session=None, **kwargs): model=models.Task, **kwargs) + # Filter out tasks or stories that the current user can't see + query = query.outerjoin(models.Story) + query = api_base.filter_private_stories(query, current_user) + + if worklist_id or board_id: + query = query.outerjoin( + (models.WorklistItem, + models.WorklistItem.item_id == models.Task.id)) + query = query.filter(models.WorklistItem.item_type == "task") + query = query.outerjoin(models.Worklist) + + # Filter by worklist + if worklist_id: + query = query.filter(models.Worklist.id == worklist_id) + query = api_base.filter_private_worklists( + query, current_user, hide_lanes=False) + + # Filter by board + if board_id: + query = query.outerjoin(models.BoardWorklist, models.Board) + query = api_base.filter_private_boards(query, current_user) + query = query.filter(models.Board.id == board_id) + query = api_base.filter_private_worklists( + query, current_user, hide_lanes=False) + return query