diff --git a/atst/forms/app_settings.py b/atst/forms/app_settings.py index 20e1aa25..19801a10 100644 --- a/atst/forms/app_settings.py +++ b/atst/forms/app_settings.py @@ -2,18 +2,18 @@ from flask_wtf import FlaskForm from wtforms.fields import FieldList, FormField, HiddenField, RadioField, StringField from .forms import BaseForm -from .data import ENV_ROLES +from .data import ENV_ROLES, ENV_ROLE_NO_ACCESS as NO_ACCESS class MemberForm(FlaskForm): user_id = HiddenField() user_name = StringField() - role_name = RadioField(choices=ENV_ROLES, default="no_access") + role_name = RadioField(choices=ENV_ROLES, default=NO_ACCESS) @property def data(self): _data = super().data - if "role_name" in _data and _data["role_name"] == "no_access": + if "role_name" in _data and _data["role_name"] == NO_ACCESS: _data["role_name"] = None return _data diff --git a/atst/forms/data.py b/atst/forms/data.py index 2ddc3124..e0d4cf7f 100644 --- a/atst/forms/data.py +++ b/atst/forms/data.py @@ -217,6 +217,7 @@ REQUIRED_DISTRIBUTIONS = [ ("other", "Other as necessary"), ] +ENV_ROLE_NO_ACCESS = "No Access" ENV_ROLES = [(role.value, role.value) for role in CSPRole] + [ - ("no_access", "No access") + (ENV_ROLE_NO_ACCESS, "No access") ] diff --git a/atst/forms/team.py b/atst/forms/team.py index 96c6a5ea..29e46874 100644 --- a/atst/forms/team.py +++ b/atst/forms/team.py @@ -3,7 +3,7 @@ from wtforms.fields import FormField, FieldList, HiddenField, RadioField, String from wtforms.validators import Required from .application_member import EnvironmentForm as BaseEnvironmentForm -from .data import ENV_ROLES +from .data import ENV_ROLES, ENV_ROLE_NO_ACCESS as NO_ACCESS from .forms import BaseForm from atst.forms.fields import SelectField from atst.domain.permission_sets import PermissionSets @@ -18,6 +18,13 @@ class EnvironmentForm(BaseEnvironmentForm): filters=[lambda x: None if x == "None" else x], ) + @property + def data(self): + _data = super().data + if "role" in _data and _data["role"] == NO_ACCESS: + _data["role"] = None + return _data + class PermissionsForm(FlaskForm): perms_team_mgmt = SelectField( diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index b3077b77..935aaaf4 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -5,6 +5,7 @@ from atst.domain.environments import Environments from atst.domain.applications import Applications from atst.forms.app_settings import AppEnvRolesForm from atst.forms.application import ApplicationForm, EditEnvironmentForm +from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS from atst.domain.authz.decorator import user_can_access_decorator as user_can from atst.models.environment_role import CSPRole from atst.domain.exceptions import NotFoundError @@ -46,10 +47,10 @@ def sort_env_users_by_role(env): users_list = [] no_access_users = env.application.users - env.users no_access_list = [ - {"user_id": str(user.id), "user_name": user.full_name, "role_name": "no_access"} + {"user_id": str(user.id), "user_name": user.full_name, "role_name": NO_ACCESS} for user in no_access_users ] - users_list.append({"role": "no_access", "members": no_access_list}) + users_list.append({"role": NO_ACCESS, "members": no_access_list}) for role in CSPRole: users_list.append( diff --git a/atst/routes/applications/team.py b/atst/routes/applications/team.py index d4fa0a22..3aa988e4 100644 --- a/atst/routes/applications/team.py +++ b/atst/routes/applications/team.py @@ -113,7 +113,7 @@ def update_team(application_id): environment_role_form.environment_id.data ) Environments.update_env_role( - environment, user, environment_role_form.role.data + environment, user, environment_role_form.data.get("role") ) flash("updated_application_team_settings", application_name=application.name) diff --git a/js/components/__tests__/edit_environment_role.test.js b/js/components/__tests__/edit_environment_role.test.js index 4421d58c..8242acb4 100644 --- a/js/components/__tests__/edit_environment_role.test.js +++ b/js/components/__tests__/edit_environment_role.test.js @@ -1,5 +1,5 @@ import { shallowMount } from '@vue/test-utils' -import EditEnvironmentRole from '../forms/edit_environment_role' +import { NO_ACCESS, EditEnvironmentRole } from '../forms/edit_environment_role' describe('EditEnvironmentRole', () => { var initialRoleCategories, wrapper @@ -7,7 +7,7 @@ describe('EditEnvironmentRole', () => { beforeEach(() => { initialRoleCategories = [ { - role: 'no_access', + role: NO_ACCESS, members: [ { role_name: null, user_id: '123' }, { role_name: null, user_id: '456' }, @@ -41,10 +41,10 @@ describe('EditEnvironmentRole', () => { it('removes null roles to no_access', () => { let roles = wrapper.vm.sanitizeValues([ - { role: 'no_access', members: [{ role_name: null }] }, + { role: NO_ACCESS, members: [{ role_name: null }] }, ]) expect(roles).toEqual([ - { role: 'no_access', members: [{ role_name: 'no_access' }] }, + { role: NO_ACCESS, members: [{ role_name: NO_ACCESS }] }, ]) }) diff --git a/js/components/forms/edit_application_roles.js b/js/components/forms/edit_application_roles.js index c4f6d490..ef5de676 100644 --- a/js/components/forms/edit_application_roles.js +++ b/js/components/forms/edit_application_roles.js @@ -1,7 +1,7 @@ import FormMixin from '../../mixins/form' import Modal from '../../mixins/modal' import toggler from '../toggler' -import EditEnvironmentRole from './edit_environment_role' +import { EditEnvironmentRole } from './edit_environment_role' export default { name: 'edit-application-roles', diff --git a/js/components/forms/edit_environment_role.js b/js/components/forms/edit_environment_role.js index cbb09457..e9081c34 100644 --- a/js/components/forms/edit_environment_role.js +++ b/js/components/forms/edit_environment_role.js @@ -5,7 +5,9 @@ import Modal from '../../mixins/modal' // https://github.com/dod-ccpo/atst/pull/799/files#r282240663 // May also want to reconsider the data structure by storing the roles and members separately -export default { +export const NO_ACCESS = 'No Access' + +export const EditEnvironmentRole = { name: 'edit-environment-role', mixins: [FormMixin], @@ -26,7 +28,7 @@ export default { roles.forEach(role => { role.members.forEach(member => { if (member.role_name === null) { - member.role_name = 'no_access' + member.role_name = NO_ACCESS } }) }) @@ -34,7 +36,7 @@ export default { }, checkNoAccess: function(role) { - return role === 'no_access' + return role === NO_ACCESS }, toggleSection: function(sectionName) { diff --git a/js/components/toggler.js b/js/components/toggler.js index 480af36e..225e144b 100644 --- a/js/components/toggler.js +++ b/js/components/toggler.js @@ -1,4 +1,4 @@ -import editEnvironmentRole from './forms/edit_environment_role' +import { EditEnvironmentRole } from './forms/edit_environment_role' import FormMixin from '../mixins/form' import optionsinput from './options_input' import textinput from './text_input' @@ -17,7 +17,7 @@ export default { }, components: { - editEnvironmentRole, + EditEnvironmentRole, optionsinput, textinput, optionsinput, diff --git a/js/index.js b/js/index.js index 68eceffb..520dea03 100644 --- a/js/index.js +++ b/js/index.js @@ -18,7 +18,7 @@ import poc from './components/forms/poc' import oversight from './components/forms/oversight' import toggler from './components/toggler' import NewApplication from './components/forms/new_application' -import EditEnvironmentRole from './components/forms/edit_environment_role' +import { EditEnvironmentRole } from './components/forms/edit_environment_role' import EditApplicationRoles from './components/forms/edit_application_roles' import MultiStepModalForm from './components/forms/multi_step_modal_form' import funding from './components/forms/funding' diff --git a/templates/fragments/applications/edit_team.html b/templates/fragments/applications/edit_team.html index 74dba163..1609b0cb 100644 --- a/templates/fragments/applications/edit_team.html +++ b/templates/fragments/applications/edit_team.html @@ -79,13 +79,15 @@ {{ "portfolios.applications.team_settings.add_to_environment" | translate }} {{ Icon("plus") }} - + {% if user_can(permissions.DELETE_APPLICATION_MEMBER) %} + + {% endif %} {% endcall %} {{ member_form.user_id() }} diff --git a/tests/routes/applications/test_settings.py b/tests/routes/applications/test_settings.py index a658107a..40f758ed 100644 --- a/tests/routes/applications/test_settings.py +++ b/tests/routes/applications/test_settings.py @@ -18,11 +18,11 @@ from atst.domain.environments import Environments from atst.domain.permission_sets import PermissionSets from atst.domain.portfolios import Portfolios from atst.domain.exceptions import NotFoundError - from atst.models.environment_role import CSPRole from atst.models.portfolio_role import Status as PortfolioRoleStatus from atst.forms.application import EditEnvironmentForm from atst.forms.app_settings import AppEnvRolesForm +from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS from tests.utils import captured_templates @@ -166,7 +166,7 @@ def test_data_for_app_env_roles_form(app, client, user_session): "env_id": env.id, "team_roles": [ { - "role": "no_access", + "role": NO_ACCESS, "members": [ { "user_id": str(app_role.user_id), @@ -309,7 +309,7 @@ def test_update_team_env_roles(client, user_session): "envs-0-team_roles-1-members-1-user_id": env_role_2.user.id, "envs-0-team_roles-1-members-1-role_name": CSPRole.BASIC_ACCESS.value, "envs-0-team_roles-1-members-2-user_id": env_role_3.user.id, - "envs-0-team_roles-1-members-2-role_name": "no_access", + "envs-0-team_roles-1-members-2-role_name": NO_ACCESS, } user_session(application.portfolio.owner) diff --git a/tests/routes/applications/test_team.py b/tests/routes/applications/test_team.py index b2d5a0e1..99c7da78 100644 --- a/tests/routes/applications/test_team.py +++ b/tests/routes/applications/test_team.py @@ -4,6 +4,7 @@ from flask import url_for from atst.domain.permission_sets import PermissionSets from atst.models import CSPRole +from atst.forms.data import ENV_ROLE_NO_ACCESS as NO_ACCESS from tests.factories import * @@ -120,8 +121,7 @@ def test_update_team_environment_roles(client, user_session): assert env_role.role == CSPRole.TECHNICAL_READ.value -@pytest.mark.skip(reason="Need to rebase against master") -def test_update_team_revoke_environment_access(client, user_session): +def test_update_team_revoke_environment_access(client, user_session, db, session): application = ApplicationFactory.create() owner = application.portfolio.owner app_role = ApplicationRoleFactory.create( @@ -141,12 +141,13 @@ def test_update_team_revoke_environment_access(client, user_session): "members-0-permission_sets-perms_env_mgmt": PermissionSets.EDIT_APPLICATION_ENVIRONMENTS, "members-0-permission_sets-perms_del_env": PermissionSets.DELETE_APPLICATION_ENVIRONMENTS, "members-0-environment_roles-0-environment_id": environment.id, - "members-0-environment_roles-0-role": "", + "members-0-environment_roles-0-role": NO_ACCESS, }, ) assert response.status_code == 302 - assert env_role.role == CSPRole.TECHNICAL_READ.value + env_role_exists = db.exists().where(EnvironmentRole.id == env_role.id) + assert not session.query(env_role_exists).scalar() def test_create_member(client, user_session):