Add ability to change existing roles

Author: di

tests/unit/manage/test_views.py

@@ -215,6 +215,152 @@ def test_post_duplicate_role(self, db_request):
         }
 
 
+class TestChangeProjectRoles:
+
+    def test_change_role(self, db_request):
+        project = ProjectFactory.create(name="foobar")
+        user = UserFactory.create(username="testuser")
+        role = RoleFactory.create(
+            user=user, project=project, role_name="Owner"
+        )
+        new_role_name = "Maintainer"
+
+        db_request.method = "POST"
+        db_request.user = pretend.stub()
+        db_request.POST = MultiDict({
+            "role_id": role.id,
+            "role_name": new_role_name,
+        })
+        db_request.session = pretend.stub(
+            flash=pretend.call_recorder(lambda *a, **kw: None),
+        )
+        db_request.route_path = pretend.call_recorder(
+            lambda *a, **kw: "/the-redirect"
+        )
+
+        result = views.change_project_role(project, db_request)
+
+        assert role.role_name == new_role_name
+        assert db_request.route_path.calls == [
+            pretend.call('manage.project.roles', name=project.name),
+        ]
+        assert db_request.session.flash.calls == [
+            pretend.call("Successfully changed role", queue="success"),
+        ]
+        assert isinstance(result, HTTPSeeOther)
+        assert result.headers["Location"] == "/the-redirect"
+
+    def test_change_role_invalid_role_name(self, pyramid_request):
+        project = pretend.stub(name="foobar")
+
+        pyramid_request.method = "POST"
+        pyramid_request.POST = MultiDict({
+            "role_id": str(uuid.uuid4()),
+            "role_name": "Invalid Role Name",
+        })
+        pyramid_request.route_path = pretend.call_recorder(
+            lambda *a, **kw: "/the-redirect"
+        )
+
+        result = views.change_project_role(project, pyramid_request)
+
+        assert pyramid_request.route_path.calls == [
+            pretend.call('manage.project.roles', name=project.name),
+        ]
+        assert isinstance(result, HTTPSeeOther)
+        assert result.headers["Location"] == "/the-redirect"
+
+    def test_change_role_when_multiple(self, db_request):
+        project = ProjectFactory.create(name="foobar")
+        user = UserFactory.create(username="testuser")
+        owner_role = RoleFactory.create(
+            user=user, project=project, role_name="Owner"
+        )
+        maintainer_role = RoleFactory.create(
+            user=user, project=project, role_name="Maintainer"
+        )
+        new_role_name = "Maintainer"
+
+        db_request.method = "POST"
+        db_request.user = pretend.stub()
+        db_request.POST = MultiDict([
+            ("role_id", owner_role.id),
+            ("role_id", maintainer_role.id),
+            ("role_name", new_role_name),
+        ])
+        db_request.session = pretend.stub(
+            flash=pretend.call_recorder(lambda *a, **kw: None),
+        )
+        db_request.route_path = pretend.call_recorder(
+            lambda *a, **kw: "/the-redirect"
+        )
+
+        result = views.change_project_role(project, db_request)
+
+        assert db_request.db.query(Role).all() == [maintainer_role]
+        assert db_request.route_path.calls == [
+            pretend.call('manage.project.roles', name=project.name),
+        ]
+        assert db_request.session.flash.calls == [
+            pretend.call("Successfully changed role", queue="success"),
+        ]
+        assert isinstance(result, HTTPSeeOther)
+        assert result.headers["Location"] == "/the-redirect"
+
+    def test_change_missing_role(self, db_request):
+        project = ProjectFactory.create(name="foobar")
+        missing_role_id = str(uuid.uuid4())
+
+        db_request.method = "POST"
+        db_request.user = pretend.stub()
+        db_request.POST = MultiDict({
+            "role_id": missing_role_id,
+            "role_name": 'Owner',
+        })
+        db_request.session = pretend.stub(
+            flash=pretend.call_recorder(lambda *a, **kw: None),
+        )
+        db_request.route_path = pretend.call_recorder(
+            lambda *a, **kw: "/the-redirect"
+        )
+
+        result = views.change_project_role(project, db_request)
+
+        assert db_request.session.flash.calls == [
+            pretend.call("Could not find role", queue="error"),
+        ]
+        assert isinstance(result, HTTPSeeOther)
+        assert result.headers["Location"] == "/the-redirect"
+
+    def test_change_own_owner_role(self, db_request):
+        project = ProjectFactory.create(name="foobar")
+        user = UserFactory.create(username="testuser")
+        role = RoleFactory.create(
+            user=user, project=project, role_name="Owner"
+        )
+
+        db_request.method = "POST"
+        db_request.user = user
+        db_request.POST = MultiDict({
+            "role_id": role.id,
+            "role_name": "Maintainer",
+        })
+        db_request.session = pretend.stub(
+            flash=pretend.call_recorder(lambda *a, **kw: None),
+        )
+        db_request.route_path = pretend.call_recorder(
+            lambda *a, **kw: "/the-redirect"
+        )
+
+        result = views.change_project_role(project, db_request)
+
+        assert db_request.session.flash.calls == [
+            pretend.call("Cannot remove yourself as Owner", queue="error"),
+        ]
+        assert isinstance(result, HTTPSeeOther)
+        assert result.headers["Location"] == "/the-redirect"
+
+
 class TestDeleteProjectRoles:
 
     def test_delete_role(self, db_request):

tests/unit/test_routes.py

@@ -139,6 +139,13 @@ def add_policy(name, filename):
             traverse="/{name}",
             domain=warehouse,
         ),
+        pretend.call(
+            "manage.project.change_role",
+            "/project/{name}/collaboration/change/",
+            factory="warehouse.packaging.models:ProjectFactory",
+            traverse="/{name}",
+            domain=warehouse,
+        ),
         pretend.call(
             "manage.project.delete_role",
             "/project/{name}/collaboration/delete/",

warehouse/manage/forms.py

@@ -51,3 +51,7 @@ class CreateRoleForm(RoleNameMixin, UsernameMixin, forms.Form):
     def __init__(self, *args, user_service, **kwargs):
         super().__init__(*args, **kwargs)
         self.user_service = user_service
+
+
+class ChangeRoleForm(RoleNameMixin, forms.Form):
+    pass

warehouse/manage/views.py

@@ -15,10 +15,11 @@
 from pyramid.httpexceptions import HTTPSeeOther
 from pyramid.security import Authenticated
 from pyramid.view import view_config
+from sqlalchemy.orm.exc import NoResultFound
 
 from warehouse.accounts.interfaces import IUserService
 from warehouse.accounts.models import User
-from warehouse.manage.forms import CreateRoleForm
+from warehouse.manage.forms import CreateRoleForm, ChangeRoleForm
 from warehouse.packaging.models import Role
 
 
@@ -111,6 +112,66 @@ def manage_project_roles(project, request, _form_class=CreateRoleForm):
     }
 
 
+@view_config(
+    route_name="manage.project.change_role",
+    uses_session=True,
+    require_methods=["POST"],
+    permission="manage",
+)
+def change_project_role(project, request, _form_class=ChangeRoleForm):
+    # This view was modified to handle deleting multiple roles for a single
+    # user and should be updated when fixing GH-2745
+
+    form = _form_class(request.POST)
+
+    if form.validate():
+        role_ids = request.POST.getall('role_id')
+
+        if len(role_ids) > 1:
+            # This user has more than one role, so just delete all the ones
+            # that aren't what we want.
+            #
+            # This should be removed when fixing GH-2745.
+            (
+                request.db.query(Role)
+                .filter(
+                    Role.id.in_(role_ids),
+                    Role.project == project,
+                    Role.role_name != form.role_name.data
+                )
+                .delete(synchronize_session="fetch")
+            )
+            request.session.flash(
+                'Successfully changed role', queue="success"
+            )
+        else:
+            # This user only has one role, so get it and change the type.
+            try:
+                role = (
+                    request.db.query(Role)
+                    .filter(
+                        Role.id == request.POST.get('role_id'),
+                        Role.project == project,
+                    )
+                    .one()
+                )
+                if role.role_name == "Owner" and role.user == request.user:
+                    request.session.flash(
+                        "Cannot remove yourself as Owner", queue="error"
+                    )
+                else:
+                    role.role_name = form.role_name.data
+                    request.session.flash(
+                        'Successfully changed role', queue="success"
+                    )
+            except NoResultFound:
+                request.session.flash("Could not find role", queue="error")
+
+    return HTTPSeeOther(
+        request.route_path('manage.project.roles', name=project.name)
+    )
+
+
 @view_config(
     route_name="manage.project.delete_role",
     uses_session=True,

warehouse/routes.py

@@ -105,6 +105,13 @@ def includeme(config):
         traverse="/{name}",
         domain=warehouse,
     )
+    config.add_route(
+        "manage.project.change_role",
+        "/project/{name}/collaboration/change/",
+        factory="warehouse.packaging.models:ProjectFactory",
+        traverse="/{name}",
+        domain=warehouse,
+    )
     config.add_route(
         "manage.project.delete_role",
         "/project/{name}/collaboration/delete/",

warehouse/static/js/warehouse/index.js

@@ -143,3 +143,22 @@ docReady(() => {
     });
   }
 });
+
+docReady(() => {
+  let changeRoleForms = document.querySelectorAll("form.changeRole");
+
+  if (changeRoleForms) {
+    for (let form of changeRoleForms) {
+      let changeButton = form.querySelector("button.change-button");
+      let changeSelect = form.querySelector("select.change-field");
+
+      changeSelect.addEventListener("change", function (event) {
+        if (event.target.value === changeSelect.dataset.original) {
+          changeButton.style.visibility = "hidden";
+        } else {
+          changeButton.style.visibility = "visible";
+        }
+      });
+    }
+  }
+});

warehouse/static/sass/blocks/_button.scss

@@ -155,3 +155,8 @@
     }
   }
 }
+
+
+.change-button {
+  visibility: hidden;
+}

warehouse/templates/manage/roles.html

@@ -60,11 +60,26 @@ <h2>Collaborators</h2>
         {% if role.user == request.user %}
           {{ role.role_name }}
         {% else %}
-          <form>
-            <select name="" id="">
-              <option value="">Owner</option>
-              <option value="">Maintainer</option>
+          <form class="changeRole" method="POST" action="{{ request.route_path('manage.project.change_role', name=project.name) }}">
+            <input name="csrf_token" type="hidden" value="{{ request.session.get_csrf_token() }}">
+            {% for role in roles %}
+              <input type="hidden" name="role_id" value="{{ role.id }}">
+            {% endfor %}
+
+            <select class="change-field" name="role_name" data-original="{{ role.role_name }}">
+            {% for role_name in ['Owner', 'Maintainer'] %}
+            <option value="{{ role_name }}" {{ 'selected' if role_name == role.role_name else '' }}>
+              {{ role_name }}
+            </option>
+            {% endfor %}
             </select>
+            <button
+              class="button change-button"
+              type="submit"
+              title="Save this new role"
+            >
+              Save
+            </button>
           </form>
         {% endif %}
         </td>