diff --git a/storyboard/api/v1/search/sqlalchemy_impl.py b/storyboard/api/v1/search/sqlalchemy_impl.py index ed3d344a..810942b8 100644 --- a/storyboard/api/v1/search/sqlalchemy_impl.py +++ b/storyboard/api/v1/search/sqlalchemy_impl.py @@ -58,10 +58,6 @@ class SqlAlchemySearchImpl(search_engine.SearchEngine): session = api_base.get_session() subquery = api_base.model_query(models.Story, session) - - # Filter out stories that the current user can't see - subquery = api_base.filter_private_stories(subquery, current_user) - subquery = self._build_fulltext_search(models.Story, subquery, q) subquery = self._apply_pagination(models.Story, subquery, marker, offset, limit) @@ -73,6 +69,9 @@ class SqlAlchemySearchImpl(search_engine.SearchEngine): query = query.join(subquery, models.StorySummary.id == subquery.c.id) + # Filter out stories that the current user can't see + query = api_base.filter_private_stories(query, current_user) + stories = query.all() return stories @@ -81,11 +80,12 @@ class SqlAlchemySearchImpl(search_engine.SearchEngine): session = api_base.get_session() query = api_base.model_query(models.Task, session) + query = self._build_fulltext_search(models.Task, query, q) + # 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 = self._build_fulltext_search(models.Task, query, q) query = self._apply_pagination( models.Task, query, marker, offset, limit) diff --git a/storyboard/api/v1/stories.py b/storyboard/api/v1/stories.py index 2505a711..f7eb17a3 100644 --- a/storyboard/api/v1/stories.py +++ b/storyboard/api/v1/stories.py @@ -52,8 +52,10 @@ def create_story_wmodel(story): story_model.summarize_task_statuses(story) if story.permissions: story_model.resolve_users(story) + story_model.resolve_teams(story) else: story_model.users = [] + story_model.teams = [] return story_model @@ -213,17 +215,22 @@ class StoriesController(rest.RestController): if "due_dates" in story_dict: del story_dict['due_dates'] - users = [] + users = None + teams = None if "users" in story_dict: users = story_dict.pop("users") if users is None: users = [wmodels.User.from_db_model(users_api.user_get(user_id))] + if "teams" in story_dict: + teams = story_dict.pop("teams") + if teams is None: + teams = [] created_story = stories_api.story_create(story_dict) events_api.story_created_event(created_story.id, user_id, story.title) if story.private: - stories_api.create_permission(created_story, users) + stories_api.create_permission(created_story, users, teams) return wmodels.Story.from_db_model(created_story) @@ -243,6 +250,7 @@ class StoriesController(rest.RestController): :param story_id: An ID of the story. :param story: A story within the request body. """ + user_id = request.current_user_id # Reject private story types while ACL is not created. if (story.story_type_id and @@ -251,7 +259,7 @@ class StoriesController(rest.RestController): story.story_type_id) original_story = stories_api.story_get_simple( - story_id, current_user=request.current_user_id) + story_id, current_user=user_id) if not original_story: raise exc.NotFound(_("Story %s not found") % story_id) @@ -273,28 +281,54 @@ class StoriesController(rest.RestController): if 'tags' in story_dict: story_dict.pop('tags') - users = story_dict.get("users", []) - ids = [user.id for user in users] - if story.private: - if request.current_user_id not in ids \ - and not original_story.permissions: - users.append(wmodels.User.from_db_model( - users_api.user_get(request.current_user_id))) + users = story_dict.get("users") + teams = story_dict.get("teams") + + private = story_dict.get("private", original_story.private) + if private: + # If trying to make a story private with no permissions set, add + # the user making the change to the permission so that at least + # the story isn't lost to everyone. + if not users and not teams and not original_story.permissions: + users = [wmodels.User.from_db_model( + users_api.user_get(user_id))] + + original_teams = None + original_users = None + if original_story.permissions: + original_teams = original_story.permissions[0].teams + original_users = original_story.permissions[0].users + + # Don't allow both permission lists to be deliberately emptied + # on a private story, to make sure the story remains visible to + # at least someone. + valid = True + if users == [] and teams == []: + valid = False + elif users == [] and original_teams == []: + valid = False + elif teams == [] and original_users == []: + valid = False + if not valid and original_story.private: + abort(400, + _("Can't make a private story have no users or teams")) + + # If the story doesn't already have permissions, create them. if not original_story.permissions: - stories_api.create_permission(original_story, users) + stories_api.create_permission(original_story, users, teams) updated_story = stories_api.story_update( story_id, story_dict, - current_user=request.current_user_id) + current_user=user_id) - if users == [] and updated_story.private: - abort(400, _("Can't make a private story with no users")) + # If the story is private and already has some permissions, update + # them as needed. This is done after updating the story in case the + # request is trying to both update some story fields and also remove + # the user making the change from the ACL. + if private and original_story.permissions: + stories_api.update_permission(updated_story, users, teams) - if story.private: - stories_api.update_permission(updated_story, users) - - user_id = request.current_user_id events_api.story_details_changed_event(story_id, user_id, updated_story.title) diff --git a/storyboard/api/v1/wmodels.py b/storyboard/api/v1/wmodels.py index 81d0d194..f4ac476d 100644 --- a/storyboard/api/v1/wmodels.py +++ b/storyboard/api/v1/wmodels.py @@ -182,6 +182,24 @@ class User(base.APIBase): last_login=datetime(2014, 1, 1, 16, 42)) +class Team(base.APIBase): + """The Team is a group of Users with a fixed set of permissions.""" + + name = NameType() + """The Team unique name. This name will be displayed in the URL. + At least 3 alphanumeric symbols. Minus and dot symbols are allowed as + separators.""" + + description = wtypes.text + """Details about the team.""" + + @classmethod + def sample(cls): + return cls( + name="StoryBoard-core", + description="Core reviewers of StoryBoard team.") + + class Story(base.APIBase): """The Story is the main element of StoryBoard. It represents a user story (generally a bugfix or a feature) that needs to be implemented. It will be @@ -222,6 +240,9 @@ class Story(base.APIBase): users = wtypes.ArrayType(User) """The set of users with permission to see this story if it is private.""" + teams = wtypes.ArrayType(Team) + """The set of teams with permission to see this story if it is private.""" + @classmethod def sample(cls): return cls( @@ -252,6 +273,12 @@ class Story(base.APIBase): for user in story.permissions[0].users] self.users = [User.from_db_model(user) for user in users] + @nodoc + def resolve_teams(self, story): + """Resolve the teams who can see the story.""" + self.teams = [Team.from_db_model(team) + for team in story.permissions[0].teams] + class Tag(base.APIBase): @@ -389,24 +416,6 @@ class Milestone(base.APIBase): ) -class Team(base.APIBase): - """The Team is a group of Users with a fixed set of permissions.""" - - name = NameType() - """The Team unique name. This name will be displayed in the URL. - At least 3 alphanumeric symbols. Minus and dot symbols are allowed as - separators.""" - - description = wtypes.text - """Details about the team.""" - - @classmethod - def sample(cls): - return cls( - name="StoryBoard-core", - description="Core reviewers of StoryBoard team.") - - class TimeLineEvent(base.APIBase): """An event object should be created each time a story or a task state changes. diff --git a/storyboard/db/api/base.py b/storyboard/db/api/base.py index 022701c1..6eb4daa5 100644 --- a/storyboard/db/api/base.py +++ b/storyboard/db/api/base.py @@ -384,12 +384,13 @@ def filter_private_stories(query, current_user, story_model=models.Story): :param story_model: The database model used for stories in the query. """ + # First filter based on users with permissions set directly query = query.outerjoin(models.story_permissions, models.Permission, models.user_permissions, models.User) if current_user: - query = query.filter( + visible_to_users = query.filter( or_( and_( models.User.id == current_user, @@ -400,14 +401,40 @@ def filter_private_stories(query, current_user, story_model=models.Story): ) ) else: - query = query.filter( + visible_to_users = query.filter( or_( story_model.private == false(), story_model.id.is_(None) ) ) - return query + # Now filter based on membership of teams with permissions + users = aliased(models.User) + query = query.outerjoin(models.team_permissions, + models.Team, + models.team_membership, + (users, + users.id == models.team_membership.c.user_id)) + if current_user: + visible_to_teams = query.filter( + or_( + and_( + users.id == current_user, + story_model.private == true() + ), + story_model.private == false(), + story_model.id.is_(None) + ) + ) + else: + visible_to_teams = query.filter( + or_( + story_model.private == false(), + story_model.id.is_(None) + ) + ) + + return visible_to_users.union(visible_to_teams) def filter_private_worklists(query, current_user, hide_lanes=True): diff --git a/storyboard/db/api/stories.py b/storyboard/db/api/stories.py index 1823f7ff..1bc63c99 100644 --- a/storyboard/db/api/stories.py +++ b/storyboard/db/api/stories.py @@ -22,6 +22,7 @@ from storyboard.common import exception as exc from storyboard.db.api import base as api_base from storyboard.db.api import story_tags from storyboard.db.api import story_types +from storyboard.db.api import teams as teams_api from storyboard.db.api import users as users_api from storyboard.db import models from storyboard.openstack.common.gettextutils import _ # noqa @@ -109,7 +110,7 @@ def story_get_all(title=None, description=None, status=None, assignee_id=None, query = api_base.model_query(models.StorySummary)\ .options(subqueryload(models.StorySummary.tags)) query = query.join(subquery, - models.StorySummary.id == subquery.c.id) + models.StorySummary.id == subquery.c.stories_id) if status: query = query.filter(models.StorySummary.status.in_(status)) @@ -159,7 +160,7 @@ def story_get_count(title=None, description=None, status=None, query = query.subquery() summary_query = api_base.model_query(models.StorySummary) summary_query = summary_query \ - .join(query, models.StorySummary.id == query.c.id) + .join(query, models.StorySummary.id == query.c.stories_id) query = summary_query.filter(models.StorySummary.status.in_(status)) return query.count() @@ -214,7 +215,7 @@ def _story_build_query(title=None, description=None, assignee_id=None, if project_id: query = query.filter(models.Task.project_id == project_id) - return query + return query.distinct() def story_create(values): @@ -363,7 +364,7 @@ def story_can_mutate(story, new_story_type_id): return False -def create_permission(story, users, session=None): +def create_permission(story, users, teams, session=None): story = api_base.model_query(models.Story, session) \ .options(subqueryload(models.Story.tags)) \ .filter_by(id=story.id).first() @@ -373,13 +374,18 @@ def create_permission(story, users, session=None): } permission = api_base.entity_create(models.Permission, permission_dict) story.permissions.append(permission) - for user in users: - user = users_api.user_get(user.id) - user.permissions.append(permission) + if users is not None: + for user in users: + user = users_api.user_get(user.id) + user.permissions.append(permission) + if teams is not None: + for team in teams: + team = teams_api.team_get(team.id) + team.permissions.append(permission) return permission -def update_permission(story, users, session=None): +def update_permission(story, users, teams, session=None): story = api_base.model_query(models.Story, session) \ .options(subqueryload(models.Story.tags)) \ .filter_by(id=story.id).first() @@ -389,9 +395,14 @@ def update_permission(story, users, session=None): permission = story.permissions[0] permission_dict = { 'name': permission.name, - 'codename': permission.codename, - 'users': [users_api.user_get(user.id) for user in users] + 'codename': permission.codename } + if users is not None: + permission_dict['users'] = [users_api.user_get(user.id) + for user in users] + if teams is not None: + permission_dict['teams'] = [teams_api.team_get(team.id) + for team in teams] return api_base.entity_update(models.Permission, permission.id, diff --git a/storyboard/db/models.py b/storyboard/db/models.py index 086f0f35..eb0f8103 100644 --- a/storyboard/db/models.py +++ b/storyboard/db/models.py @@ -212,7 +212,8 @@ class Team(ModelBuilder, Base): ) name = Column(Unicode(CommonLength.top_large_length)) users = relationship("User", secondary="team_membership") - permissions = relationship("Permission", secondary="team_permissions") + permissions = relationship("Permission", secondary="team_permissions", + backref="teams") project_group_mapping = Table(