From ea1a3926accaa193801f43ae94273f39a67a8a31 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 31 Aug 2018 14:12:02 -0400 Subject: [PATCH 01/14] Wire up selector for updating a workspace user's role --- atst/forms/update_member.py | 16 ++ atst/models/workspace.py | 3 +- atst/models/workspace_user.py | 17 ++ atst/routes/workspaces.py | 29 +++ templates/member_edit.html | 302 +++++++++++-------------------- templates/workspace_members.html | 6 +- 6 files changed, 177 insertions(+), 196 deletions(-) create mode 100644 atst/forms/update_member.py diff --git a/atst/forms/update_member.py b/atst/forms/update_member.py new file mode 100644 index 00000000..236cfbb8 --- /dev/null +++ b/atst/forms/update_member.py @@ -0,0 +1,16 @@ +from flask_wtf import Form +from wtforms.fields import StringField +from wtforms.fields.html5 import EmailField +from wtforms.validators import Required, Email, Length + +from atst.forms.validators import IsNumber +from atst.forms.fields import SelectField + +from .data import WORKSPACE_ROLES + + +class UpdateMemberForm(Form): + + workspace_role = SelectField( + "Workspace Role", choices=WORKSPACE_ROLES, validators=[Required()], default="" + ) diff --git a/atst/models/workspace.py b/atst/models/workspace.py index aca024a4..1128079a 100644 --- a/atst/models/workspace.py +++ b/atst/models/workspace.py @@ -5,6 +5,7 @@ from atst.models import Base from atst.models.types import Id from atst.models.mixins import TimestampsMixin from atst.utils import first_or_none +from atst.models.workspace_user import WorkspaceUser MOCK_MEMBERS = [ @@ -68,4 +69,4 @@ class Workspace(Base, TimestampsMixin): @property def members(self): - return MOCK_MEMBERS + return [ WorkspaceUser(role.user, role) for role in self.roles] diff --git a/atst/models/workspace_user.py b/atst/models/workspace_user.py index 5e3ee1ed..59576a17 100644 --- a/atst/models/workspace_user.py +++ b/atst/models/workspace_user.py @@ -16,3 +16,20 @@ class WorkspaceUser(object): def workspace_id(self): return self.workspace_role.workspace_id + + @property + def user_id(self): + return self.user.id + + @property + def user_name(self): + return self.user.full_name + + @property + def role(self): + return self.workspace_role.role.name + + @property + def status(self): + return "radical" + diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 665d266c..f6e8a5f1 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -9,9 +9,12 @@ from flask import ( from atst.domain.exceptions import UnauthorizedError from atst.domain.workspaces import Workspaces +from atst.domain.workspace_users import WorkspaceUsers from atst.domain.projects import Projects from atst.forms.new_project import NewProjectForm from atst.forms.new_member import NewMemberForm +from atst.forms.update_member import UpdateMemberForm +from atst.forms.forms import ValidatedForm from atst.domain.authz import Authorization from atst.models.permissions import Permissions @@ -119,3 +122,29 @@ def create_member(workspace_id): ) else: return render_template("member_new.html", workspace=workspace, form=form) + + +@bp.route("/workspaces//members//member_edit") +def view_member(workspace_id, member_id): + workspace = Workspaces.get(g.current_user, workspace_id) + member = WorkspaceUsers.get(workspace_id, member_id) + form = NewMemberForm(http_request.form) + + return render_template("member_edit.html", form=form, workspace=workspace, member=member) + +@bp.route("/workspaces//members//member_edit", methods=['POST']) +def update_member(workspace_id, member_id): + workspace = Workspaces.get(g.current_user, workspace_id) + member = WorkspaceUsers.get(workspace_id, member_id) + form = UpdateMemberForm(http_request.form) + + if form.validate(): + return redirect( + url_for( + "workspaces.workspace_members", + workspace_id=workspace.id, + ) + ) + else: + return render_template("member_edit.html", form=form, workspace=workspace, member=member) + diff --git a/templates/member_edit.html b/templates/member_edit.html index db52347b..12eb0245 100644 --- a/templates/member_edit.html +++ b/templates/member_edit.html @@ -2,48 +2,44 @@ {% from "components/icon.html" import Icon %} {% from "components/modal.html" import Modal %} +{% from "components/selector.html" import Selector %} {% block content %} -
-
-

Danny Knight

+
+ {{ form.csrf_token }} + +
+
+

Danny Knight

+ +
+ + {{ Selector(form.workspace_role) }} +
-
- -
- +
+
+
+
DOD ID:
+
789
+
+
+
Email:
+
knight@mil.gov
+
+
+ edit account details +
-
-
-
-
DOD ID:
-
789
-
-
-
Email:
-
knight@mil.gov
-
-
- edit account details -
-
-
-
-

Manage Access
Grant access to an environment

+
+
+

Manage Access
Grant access to an environment

+
-
-
@@ -51,174 +47,96 @@ Search
- -{% call Modal(name='rolesModal', dismissable=False) %} -
-
-

- Environment access for Danny Knight -
Project Name - Environment Name
-

-
- -
-
    -
  • - - +
    +
    -{% endcall %} +
    + +
    -
    - -
    +
    + + + {{ Icon('x') }} + Cancel + +
    -
    - -
    - -
    - - {% if is_new_member %}Create{% else %}Save{% endif %} - - - {{ Icon('x') }} - Cancel - -
    +
    • diff --git a/templates/workspace_members.html b/templates/workspace_members.html index bf61ea3d..403a0f97 100644 --- a/templates/workspace_members.html +++ b/templates/workspace_members.html @@ -77,10 +77,10 @@ {% for m in workspace.members %} - {{ m['first_name'] }} {{ m['last_name'] }} + {{ m.user_name }} {% if m['num_projects'] == '0' %} No Project Access {% endif %} - {{ m['status'] }} - {{ m['workspace_role'] }} + {{ m.status }} + {{ m.role }} {% endfor %} From b379972446d02e973891bb71c8c7014c4fdf5008 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 31 Aug 2018 15:44:15 -0400 Subject: [PATCH 02/14] Check permissions before viewing member edit page --- atst/domain/authz.py | 6 ++++++ atst/models/workspace.py | 2 +- atst/models/workspace_user.py | 1 - atst/routes/workspaces.py | 24 ++++++++++++++++-------- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/atst/domain/authz.py b/atst/domain/authz.py index 20bd539f..65db9894 100644 --- a/atst/domain/authz.py +++ b/atst/domain/authz.py @@ -1,5 +1,6 @@ from atst.domain.workspace_users import WorkspaceUsers from atst.models.permissions import Permissions +from atst.domain.exceptions import UnauthorizedError class Authorization(object): @@ -23,3 +24,8 @@ class Authorization(object): return True return False + + @classmethod + def check_workspace_permission(cls, user, workspace, permission, message): + if not Authorization.has_workspace_permission(user, workspace, permission): + raise UnauthorizedError(user, message) diff --git a/atst/models/workspace.py b/atst/models/workspace.py index 1128079a..9052e204 100644 --- a/atst/models/workspace.py +++ b/atst/models/workspace.py @@ -69,4 +69,4 @@ class Workspace(Base, TimestampsMixin): @property def members(self): - return [ WorkspaceUser(role.user, role) for role in self.roles] + return [WorkspaceUser(role.user, role) for role in self.roles] diff --git a/atst/models/workspace_user.py b/atst/models/workspace_user.py index 59576a17..3c92707e 100644 --- a/atst/models/workspace_user.py +++ b/atst/models/workspace_user.py @@ -32,4 +32,3 @@ class WorkspaceUser(object): @property def status(self): return "radical" - diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index f6e8a5f1..7506aa45 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -127,12 +127,22 @@ def create_member(workspace_id): @bp.route("/workspaces//members//member_edit") def view_member(workspace_id, member_id): workspace = Workspaces.get(g.current_user, workspace_id) + Authorization.check_workspace_permission( + g.current_user, + workspace, + Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, + "edit this workspace user", + ) member = WorkspaceUsers.get(workspace_id, member_id) form = NewMemberForm(http_request.form) + return render_template( + "member_edit.html", form=form, workspace=workspace, member=member + ) - return render_template("member_edit.html", form=form, workspace=workspace, member=member) -@bp.route("/workspaces//members//member_edit", methods=['POST']) +@bp.route( + "/workspaces//members//member_edit", methods=["POST"] +) def update_member(workspace_id, member_id): workspace = Workspaces.get(g.current_user, workspace_id) member = WorkspaceUsers.get(workspace_id, member_id) @@ -140,11 +150,9 @@ def update_member(workspace_id, member_id): if form.validate(): return redirect( - url_for( - "workspaces.workspace_members", - workspace_id=workspace.id, - ) + url_for("workspaces.workspace_members", workspace_id=workspace.id) ) else: - return render_template("member_edit.html", form=form, workspace=workspace, member=member) - + return render_template( + "member_edit.html", form=form, workspace=workspace, member=member + ) From 8faed87e00a2624e14fb47c32813205b279f46f2 Mon Sep 17 00:00:00 2001 From: Montana Date: Fri, 31 Aug 2018 16:45:19 -0400 Subject: [PATCH 03/14] Update workspace role in the DB, add tests --- atst/domain/workspace_users.py | 25 +++++++++++++++++++++++ atst/domain/workspaces.py | 9 +++++++++ atst/routes/workspaces.py | 3 +++ tests/domain/test_workspaces.py | 35 +++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+) diff --git a/atst/domain/workspace_users.py b/atst/domain/workspace_users.py index 6fb761c3..d37ec736 100644 --- a/atst/domain/workspace_users.py +++ b/atst/domain/workspace_users.py @@ -30,6 +30,21 @@ class WorkspaceUsers(object): return WorkspaceUser(user, workspace_role) + @classmethod + def _get_workspace_role(cls, user, workspace_id): + try: + existing_workspace_role = ( + db.session.query(WorkspaceRole) + .filter( + WorkspaceRole.user == user, + WorkspaceRole.workspace_id == workspace_id, + ) + .one() + ) + return existing_workspace_role + except NoResultFound: + raise NotFoundError("workspace role") + @classmethod def add(cls, user, workspace_id, role_name): role = Roles.get(role_name) @@ -57,6 +72,16 @@ class WorkspaceUsers(object): return WorkspaceUser(user, new_workspace_role) + @classmethod + def update_role(cls, member, workspace_id, role_name): + new_role = Roles.get(role_name) + workspace_role = WorkspaceUsers._get_workspace_role(member.user, workspace_id) + workspace_role.role = new_role + + db.session.add(workspace_role) + db.session.commit() + return WorkspaceUser(member.user, workspace_role) + @classmethod def add_many(cls, workspace_id, workspace_user_dicts): workspace_users = [] diff --git a/atst/domain/workspaces.py b/atst/domain/workspaces.py index 6570a831..75a6225b 100644 --- a/atst/domain/workspaces.py +++ b/atst/domain/workspaces.py @@ -81,6 +81,15 @@ class Workspaces(object): ) return workspace_user + @classmethod + def update_member(cls, user, workspace, member, role_name): + if not Authorization.has_workspace_permission( + user, workspace, Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE + ): + raise UnauthorizedError(user, "update workspace member") + + return WorkspaceUsers.update_role(member, workspace.id, role_name) + @classmethod def _create_workspace_role(cls, user, workspace, role_name): role = Roles.get(role_name) diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 7506aa45..06b32008 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -149,6 +149,9 @@ def update_member(workspace_id, member_id): form = UpdateMemberForm(http_request.form) if form.validate(): + Workspaces.update_member( + g.current_user, workspace, member, form.data["workspace_role"] + ) return redirect( url_for("workspaces.workspace_members", workspace_id=workspace.id) ) diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index f3cca7f9..dc48ee48 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -119,3 +119,38 @@ def test_need_permission_to_create_workspace_user(): with pytest.raises(UnauthorizedError): Workspaces.create_member(random_user, workspace, user_data) + + +def test_can_update_workspace_user_role(): + owner = UserFactory.create() + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + user_data = { + "first_name": "New", + "last_name": "User", + "email": "new.user@mail.com", + "workspace_role": "developer", + "dod_id": "1234567890", + } + member = Workspaces.create_member(owner, workspace, user_data) + role_name = "developer" + + updated_member = Workspaces.update_member(owner, workspace, member, role_name) + assert updated_member.workspace == workspace + + +def test_need_permission_to_update_workspace_user_role(): + owner = UserFactory.create() + workspace = Workspaces.create(RequestFactory.create(creator=owner)) + random_user = UserFactory.create() + user_data = { + "first_name": "New", + "last_name": "User", + "email": "new.user@mail.com", + "workspace_role": "developer", + "dod_id": "1234567890", + } + member = Workspaces.create_member(owner, workspace, user_data) + role_name = "developer" + + with pytest.raises(UnauthorizedError): + Workspaces.update_member(random_user, workspace, member, role_name) From ce44e4537a6514f82941e76b2958328bd1f1832a Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 4 Sep 2018 10:34:30 -0400 Subject: [PATCH 04/14] Add an alert for successful role update --- atst/routes/workspaces.py | 6 +++++- templates/workspace_members.html | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 06b32008..f3ad9092 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -153,7 +153,11 @@ def update_member(workspace_id, member_id): g.current_user, workspace, member, form.data["workspace_role"] ) return redirect( - url_for("workspaces.workspace_members", workspace_id=workspace.id) + url_for( + "workspaces.workspace_members", + workspace_id=workspace.id, + memberName=member.user.full_name, + ) ) else: return render_template( diff --git a/templates/workspace_members.html b/templates/workspace_members.html index 403a0f97..0c1f3c7e 100644 --- a/templates/workspace_members.html +++ b/templates/workspace_members.html @@ -33,6 +33,18 @@ ) }} {% endif %} +{% set updated_member_name = request.args.get("memberName") %} +{% if updated_member_name %} + {% set message -%} +

    {{ updated_member_name }}'s role was successfully updated.

    + {%- endset %} + + {{ Alert('Workspace role updated successfully', + message=message, + level='success' + ) }} +{% endif %} +
    From 1f752cac229899f097ecf468fafe46b1a3506ae3 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 4 Sep 2018 14:34:35 -0400 Subject: [PATCH 05/14] Fill out user data in the member edit template --- templates/member_edit.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/member_edit.html b/templates/member_edit.html index 12eb0245..0dc47768 100644 --- a/templates/member_edit.html +++ b/templates/member_edit.html @@ -11,7 +11,7 @@
    -

    Danny Knight

    +

    {{ member.user.full_name }}

    @@ -23,11 +23,11 @@
    DOD ID:
    -
    789
    +
    {{ member.user.dod_id }}
    Email:
    -
    knight@mil.gov
    +
    {{ member.user.email }}
    edit account details From 03715d8e9bfe32dc7424c8db045d89b2a0730354 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 4 Sep 2018 14:39:39 -0400 Subject: [PATCH 06/14] Remove MOCK_MEMBERS data --- atst/models/workspace.py | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/atst/models/workspace.py b/atst/models/workspace.py index 9052e204..edeba529 100644 --- a/atst/models/workspace.py +++ b/atst/models/workspace.py @@ -8,37 +8,6 @@ from atst.utils import first_or_none from atst.models.workspace_user import WorkspaceUser -MOCK_MEMBERS = [ - { - "first_name": "Danny", - "last_name": "Knight", - "email": "dknight@thenavy.mil", - "dod_id": "1257892124", - "workspace_role": "Developer", - "status": "Pending", - "num_projects": "4", - }, - { - "first_name": "Mario", - "last_name": "Hudson", - "email": "mhudson@thearmy.mil", - "dod_id": "4357892125", - "workspace_role": "CCPO", - "status": "Active", - "num_projects": "0", - }, - { - "first_name": "Louise", - "last_name": "Greer", - "email": "lgreer@theairforce.mil", - "dod_id": "7257892125", - "workspace_role": "Admin", - "status": "Pending", - "num_projects": "43", - }, -] - - class Workspace(Base, TimestampsMixin): __tablename__ = "workspaces" From 1808acd5f47c1723f79ae33f349a3caa2cc28d2e Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 4 Sep 2018 14:56:36 -0400 Subject: [PATCH 07/14] Use new method for checking workspace permissions --- atst/domain/workspaces.py | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/atst/domain/workspaces.py b/atst/domain/workspaces.py index 75a6225b..ccdbcd3c 100644 --- a/atst/domain/workspaces.py +++ b/atst/domain/workspaces.py @@ -38,10 +38,13 @@ class Workspaces(object): @classmethod def get_for_update(cls, user, workspace_id): workspace = Workspaces.get(user, workspace_id) - if not Authorization.has_workspace_permission( - user, workspace, Permissions.ADD_APPLICATION_IN_WORKSPACE - ): - raise UnauthorizedError(user, "add project") + Authorization.check_workspace_permission( + user, + workspace, + Permissions.ADD_APPLICATION_IN_WORKSPACE, + "add project", + ) + return workspace @classmethod @@ -65,10 +68,12 @@ class Workspaces(object): @classmethod def create_member(cls, user, workspace, data): - if not Authorization.has_workspace_permission( - user, workspace, Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE - ): - raise UnauthorizedError(user, "create workspace member") + Authorization.check_workspace_permission( + user, + workspace, + Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, + "create workspace member", + ) new_user = Users.get_or_create_by_dod_id( data["dod_id"], @@ -83,10 +88,12 @@ class Workspaces(object): @classmethod def update_member(cls, user, workspace, member, role_name): - if not Authorization.has_workspace_permission( - user, workspace, Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE - ): - raise UnauthorizedError(user, "update workspace member") + Authorization.check_workspace_permission( + user, + workspace, + Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, + "update workspace member", + ) return WorkspaceUsers.update_role(member, workspace.id, role_name) From 75ea8025c17c80da8e8adc883972476f0f24a7c2 Mon Sep 17 00:00:00 2001 From: Montana Date: Tue, 4 Sep 2018 15:21:46 -0400 Subject: [PATCH 08/14] Authorization check for edit member post route --- atst/routes/workspaces.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index f3ad9092..3cfb8a46 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -145,6 +145,12 @@ def view_member(workspace_id, member_id): ) def update_member(workspace_id, member_id): workspace = Workspaces.get(g.current_user, workspace_id) + Authorization.check_workspace_permission( + g.current_user, + workspace, + Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, + "edit this workspace user", + ) member = WorkspaceUsers.get(workspace_id, member_id) form = UpdateMemberForm(http_request.form) From 1b8233af896d966ae8bfe56e9717c538a4dabac1 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 5 Sep 2018 09:10:41 -0400 Subject: [PATCH 09/14] Use update member form --- atst/forms/update_member.py | 7 ++----- atst/routes/workspaces.py | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/atst/forms/update_member.py b/atst/forms/update_member.py index 236cfbb8..e569d05b 100644 --- a/atst/forms/update_member.py +++ b/atst/forms/update_member.py @@ -1,9 +1,6 @@ from flask_wtf import Form -from wtforms.fields import StringField -from wtforms.fields.html5 import EmailField -from wtforms.validators import Required, Email, Length +from wtforms.validators import Optional -from atst.forms.validators import IsNumber from atst.forms.fields import SelectField from .data import WORKSPACE_ROLES @@ -12,5 +9,5 @@ from .data import WORKSPACE_ROLES class UpdateMemberForm(Form): workspace_role = SelectField( - "Workspace Role", choices=WORKSPACE_ROLES, validators=[Required()], default="" + "Workspace Role", choices=WORKSPACE_ROLES, validators=[Optional()], default="" ) diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 3cfb8a46..3be5c097 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -134,7 +134,7 @@ def view_member(workspace_id, member_id): "edit this workspace user", ) member = WorkspaceUsers.get(workspace_id, member_id) - form = NewMemberForm(http_request.form) + form = UpdateMemberForm(http_request.form) return render_template( "member_edit.html", form=form, workspace=workspace, member=member ) From 109249d95344ab1bd92b82a078f10dc866bd8f6a Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 5 Sep 2018 13:51:07 -0400 Subject: [PATCH 10/14] Allow user to save without changing user's role --- atst/forms/update_member.py | 2 +- atst/models/user.py | 4 ++++ atst/routes/workspaces.py | 9 +++++++-- templates/workspace_members.html | 7 ++++--- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/atst/forms/update_member.py b/atst/forms/update_member.py index e569d05b..2d60d33d 100644 --- a/atst/forms/update_member.py +++ b/atst/forms/update_member.py @@ -9,5 +9,5 @@ from .data import WORKSPACE_ROLES class UpdateMemberForm(Form): workspace_role = SelectField( - "Workspace Role", choices=WORKSPACE_ROLES, validators=[Optional()], default="" + "Workspace Role", choices=WORKSPACE_ROLES, validators=[Optional()] ) diff --git a/atst/models/user.py b/atst/models/user.py index 643fd3f5..0d8f0dc6 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -21,6 +21,10 @@ class User(Base): first_name = Column(String) last_name = Column(String) + @property + def role(self): + return self.atat_role.name + @property def atat_permissions(self): return self.atat_role.permissions diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 3be5c097..80f35741 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -134,7 +134,7 @@ def view_member(workspace_id, member_id): "edit this workspace user", ) member = WorkspaceUsers.get(workspace_id, member_id) - form = UpdateMemberForm(http_request.form) + form = UpdateMemberForm(workspace_role=member.role) return render_template( "member_edit.html", form=form, workspace=workspace, member=member ) @@ -155,14 +155,19 @@ def update_member(workspace_id, member_id): form = UpdateMemberForm(http_request.form) if form.validate(): + if form.data["workspace_role"]: + role = form.data["workspace_role"] + else: + role = member.user.role Workspaces.update_member( - g.current_user, workspace, member, form.data["workspace_role"] + g.current_user, workspace, member, role ) return redirect( url_for( "workspaces.workspace_members", workspace_id=workspace.id, memberName=member.user.full_name, + updatedRole=form.data["workspace_role"], ) ) else: diff --git a/templates/workspace_members.html b/templates/workspace_members.html index 0c1f3c7e..34761341 100644 --- a/templates/workspace_members.html +++ b/templates/workspace_members.html @@ -33,10 +33,11 @@ ) }} {% endif %} -{% set updated_member_name = request.args.get("memberName") %} -{% if updated_member_name %} +{% set member_name = request.args.get("memberName") %} +{% set updated_role = request.args.get("updatedRole") %} +{% if updated_role %} {% set message -%} -

    {{ updated_member_name }}'s role was successfully updated.

    +

    {{ member_name }}'s role was successfully updated to {{ updated_role }}

    {%- endset %} {{ Alert('Workspace role updated successfully', From 40c248df16309bf9e8f38090cf85fd4662df2d39 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 5 Sep 2018 13:58:30 -0400 Subject: [PATCH 11/14] Refactor and format --- atst/domain/workspaces.py | 7 ++---- .../{update_member.py => edit_member.py} | 2 +- atst/models/user.py | 4 ---- atst/routes/workspaces.py | 23 ++++++++----------- 4 files changed, 13 insertions(+), 23 deletions(-) rename atst/forms/{update_member.py => edit_member.py} (89%) diff --git a/atst/domain/workspaces.py b/atst/domain/workspaces.py index ccdbcd3c..c1af06ea 100644 --- a/atst/domain/workspaces.py +++ b/atst/domain/workspaces.py @@ -39,10 +39,7 @@ class Workspaces(object): def get_for_update(cls, user, workspace_id): workspace = Workspaces.get(user, workspace_id) Authorization.check_workspace_permission( - user, - workspace, - Permissions.ADD_APPLICATION_IN_WORKSPACE, - "add project", + user, workspace, Permissions.ADD_APPLICATION_IN_WORKSPACE, "add project" ) return workspace @@ -92,7 +89,7 @@ class Workspaces(object): user, workspace, Permissions.ASSIGN_AND_UNASSIGN_ATAT_ROLE, - "update workspace member", + "edit workspace member", ) return WorkspaceUsers.update_role(member, workspace.id, role_name) diff --git a/atst/forms/update_member.py b/atst/forms/edit_member.py similarity index 89% rename from atst/forms/update_member.py rename to atst/forms/edit_member.py index 2d60d33d..e76df6d0 100644 --- a/atst/forms/update_member.py +++ b/atst/forms/edit_member.py @@ -6,7 +6,7 @@ from atst.forms.fields import SelectField from .data import WORKSPACE_ROLES -class UpdateMemberForm(Form): +class EditMemberForm(Form): workspace_role = SelectField( "Workspace Role", choices=WORKSPACE_ROLES, validators=[Optional()] diff --git a/atst/models/user.py b/atst/models/user.py index 0d8f0dc6..643fd3f5 100644 --- a/atst/models/user.py +++ b/atst/models/user.py @@ -21,10 +21,6 @@ class User(Base): first_name = Column(String) last_name = Column(String) - @property - def role(self): - return self.atat_role.name - @property def atat_permissions(self): return self.atat_role.permissions diff --git a/atst/routes/workspaces.py b/atst/routes/workspaces.py index 80f35741..5ded0500 100644 --- a/atst/routes/workspaces.py +++ b/atst/routes/workspaces.py @@ -13,8 +13,7 @@ from atst.domain.workspace_users import WorkspaceUsers from atst.domain.projects import Projects from atst.forms.new_project import NewProjectForm from atst.forms.new_member import NewMemberForm -from atst.forms.update_member import UpdateMemberForm -from atst.forms.forms import ValidatedForm +from atst.forms.edit_member import EditMemberForm from atst.domain.authz import Authorization from atst.models.permissions import Permissions @@ -117,7 +116,7 @@ def create_member(workspace_id): url_for( "workspaces.workspace_members", workspace_id=workspace.id, - newMemberName=new_member.user.full_name, + newMemberName=new_member.user_name, ) ) else: @@ -134,7 +133,7 @@ def view_member(workspace_id, member_id): "edit this workspace user", ) member = WorkspaceUsers.get(workspace_id, member_id) - form = UpdateMemberForm(workspace_role=member.role) + form = EditMemberForm(workspace_role=member.role) return render_template( "member_edit.html", form=form, workspace=workspace, member=member ) @@ -152,22 +151,20 @@ def update_member(workspace_id, member_id): "edit this workspace user", ) member = WorkspaceUsers.get(workspace_id, member_id) - form = UpdateMemberForm(http_request.form) + form = EditMemberForm(http_request.form) if form.validate(): - if form.data["workspace_role"]: + role = None + if form.data["workspace_role"] != member.role: role = form.data["workspace_role"] - else: - role = member.user.role - Workspaces.update_member( - g.current_user, workspace, member, role - ) + Workspaces.update_member(g.current_user, workspace, member, role) + return redirect( url_for( "workspaces.workspace_members", workspace_id=workspace.id, - memberName=member.user.full_name, - updatedRole=form.data["workspace_role"], + memberName=member.user_name, + updatedRole=role, ) ) else: From 6444325f4254b13fac3203ba0753ed652577a779 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 5 Sep 2018 14:11:12 -0400 Subject: [PATCH 12/14] Update test for updating user role --- tests/domain/test_workspaces.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/domain/test_workspaces.py b/tests/domain/test_workspaces.py index dc48ee48..296c16d9 100644 --- a/tests/domain/test_workspaces.py +++ b/tests/domain/test_workspaces.py @@ -121,7 +121,7 @@ def test_need_permission_to_create_workspace_user(): Workspaces.create_member(random_user, workspace, user_data) -def test_can_update_workspace_user_role(): +def test_update_workspace_user_role(): owner = UserFactory.create() workspace = Workspaces.create(RequestFactory.create(creator=owner)) user_data = { @@ -132,10 +132,11 @@ def test_can_update_workspace_user_role(): "dod_id": "1234567890", } member = Workspaces.create_member(owner, workspace, user_data) - role_name = "developer" + role_name = "admin" updated_member = Workspaces.update_member(owner, workspace, member, role_name) assert updated_member.workspace == workspace + assert updated_member.role == role_name def test_need_permission_to_update_workspace_user_role(): From 277b4a22e2a5ecf72b2b25388cfebed073c29020 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 5 Sep 2018 14:49:27 -0400 Subject: [PATCH 13/14] Fix selector styling --- styles/components/_selector.scss | 5 +++- styles/sections/_member_edit.scss | 42 ++++++++++++++++--------------- templates/member_edit.html | 1 - 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/styles/components/_selector.scss b/styles/components/_selector.scss index 7b95b325..030c5ea5 100644 --- a/styles/components/_selector.scss +++ b/styles/components/_selector.scss @@ -14,7 +14,7 @@ width: 100%; height: $input-height; margin: 0; - padding: $input-padding-vertical 0.7em; + padding: $input-padding-vertical ($gap*5) $input-padding-vertical 0.7em; line-height: $input-line-height; color: $color-base; font-size: $base-font-size; @@ -42,6 +42,9 @@ label { padding: 0; + dt { + font-weight: $font-bold; + } } } } diff --git a/styles/sections/_member_edit.scss b/styles/sections/_member_edit.scss index fc9415a5..902ece36 100644 --- a/styles/sections/_member_edit.scss +++ b/styles/sections/_member_edit.scss @@ -3,23 +3,6 @@ padding: $gap*2; justify-content: space-between; - dl { - margin: 0; - - > div { - margin-bottom: $gap; - } - } - - dt { - font-weight: normal; - color: $color-gray; - } - - dd { - display: inline; - } - .member-card__header { display: flex; flex-direction: column; @@ -35,11 +18,13 @@ margin: 0; display: flex; margin-top: $gap; + fieldset { + margin: 0; + } - label { + legend { flex: none; margin-top: $gap*1.5; - font-weight: $font-normal; margin-right: $gap*2; } } @@ -51,5 +36,22 @@ .icon-link { margin: 0 -$gap; } + + dl { + margin: 0; + + > div { + margin-bottom: $gap; + } + } + + dt { + font-weight: normal; + color: $color-gray; + } + + dd { + display: inline; + } } -} \ No newline at end of file +} diff --git a/templates/member_edit.html b/templates/member_edit.html index 0dc47768..f92bb313 100644 --- a/templates/member_edit.html +++ b/templates/member_edit.html @@ -14,7 +14,6 @@

    {{ member.user.full_name }}

    - {{ Selector(form.workspace_role) }}
    From 1ade48ece633c9d68b10a9b14ecbbac046b0c512 Mon Sep 17 00:00:00 2001 From: Montana Date: Wed, 5 Sep 2018 16:42:48 -0400 Subject: [PATCH 14/14] Fix styling on search bar --- templates/member_edit.html | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/templates/member_edit.html b/templates/member_edit.html index f92bb313..ea213976 100644 --- a/templates/member_edit.html +++ b/templates/member_edit.html @@ -39,12 +39,14 @@
    -
    - - - +
    +
    + + + +