Merge branch 'release/0.7.1'

README.rst

@@ -182,6 +182,18 @@ DB Models
 Changelog
 =========
 
+v. 0.7.1
+--------
+
+* Revised API endpoints for registering/editing permissions; now allowing cataloging admins to
+  register new and edit existing permissions on their collections
+  (`#126 <https://github.com/libris/xl_auth/issues/126>`_)
+* UX enhancements (`#129 <https://github.com/libris/xl_auth/issues/129>`_,
+  `#134 <https://github.com/libris/xl_auth/issues/134>`_,
+  `#131 <https://github.com/libris/xl_auth/issues/131>`_,
+  `#130 <https://github.com/libris/xl_auth/issues/130>`_)
+
+
 v. 0.7.0
 --------
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "xl_auth",
-  "version": "0.7.0",
+  "version": "0.7.1",
   "author": "National Library of Sweden",
   "license": "Apache-2.0",
   "description": "Authorization and OAuth2 provider for LibrisXL",

tests/end2end/test_collection_registering.py

@@ -28,7 +28,7 @@ def test_superuser_can_register_new_collection(superuser, testapp):
     # Clicks Collections button
     res = res.click(href=url_for('collection.home'), index=0)
     # Clicks Register New Collection button
-    res = res.click(_('New Collection'))
+    res = res.click(_('New Collection'), index=0)
     # Fills out the form
     form = res.forms['registerCollectionForm']
     form['code'] = 'SfX'

tests/end2end/test_permission_deleting.py

@@ -65,16 +65,18 @@ def test_cataloging_admin_can_delete_existing_permission(user, permission, super
     # We see no Permissions button
     assert res.lxml.xpath("//a[contains(@text,'{0}')]".format(_('Permissions'))) == []
 
-    # Try to go there directly
+    # Try to go there directly and fail
     testapp.get('/permissions/', status=403)
 
-    # Try to delete a specific permission directly
-    res = testapp.get(url_for('permission.delete', permission_id=permission.id))
+    # Go to profile instead and click through delete flow
+    res = testapp.get(url_for('user.profile'))
+    res = res.click(href=url_for('collection.view', collection_code=permission_collection_code))
+    res = res.click(href=url_for('permission.delete', permission_id=permission.id))
     form = res.forms['deletePermissionForm']
     form['acknowledged'] = 'y'
     res = form.submit()
     assert res.status_code == 302
-    assert url_for('public.home') in res.location
+    assert url_for('collection.view', collection_code=permission_collection_code) in res.location
     res = res.follow()
 
     # Permission was deleted, so number of permissions are 1 less than initial state
@@ -83,9 +85,10 @@ def test_cataloging_admin_can_delete_existing_permission(user, permission, super
     assert len(Permission.query.all()) == old_count - 1
 
 
-def test_user_cannot_delete_permission(user, permission, testapp):
-    """Attempt to delete a permission as non-cataloging admin user."""
+def test_user_cannot_delete_permission(user, permission, superuser, testapp):
+    """Attempt to delete a permission as non-'cataloging admin' user."""
     assert user.is_cataloging_admin_for(permission.collection) is False
+    assert user.is_cataloging_admin is False
     old_count = len(Permission.query.all())
     # Goes to homepage
     res = testapp.get('/')
@@ -102,12 +105,17 @@ def test_user_cannot_delete_permission(user, permission, testapp):
     # Try to go there directly
     testapp.get('/permissions/', status=403)
 
-    # Try to delete a specific permission directly
+    # Try to delete a specific permission with direct URL
+    testapp.get(url_for('permission.delete', permission_id=permission.id), status=403)
+
+    # Accidentally be cataloging admin for unrelated collection and try again
+    temp_permission = PermissionFactory(user=user, cataloging_admin=True).save_as(superuser)
     res = testapp.get(url_for('permission.delete', permission_id=permission.id))
     form = res.forms['deletePermissionForm']
     form['acknowledged'] = 'y'
     res = form.submit()
     assert _('You do not have sufficient privileges for this operation.') in res
+    temp_permission.delete()
 
     # Nothing was deleted
     assert len(Permission.query.all()) == old_count

tests/end2end/test_permission_editing.py

@@ -9,12 +9,11 @@
 
 from xl_auth.permission.models import Permission
 
-from ..factories import CollectionFactory, PermissionFactory
+from ..factories import CollectionFactory, PermissionFactory, UserFactory
 
 
-# noinspection PyUnusedLocal
 def test_superuser_can_edit_existing_permission(superuser, permission, testapp):
-    """Edit existing permission."""
+    """Edit existing permission as superuser."""
     old_count = len(Permission.query.all())
     other_collection = CollectionFactory()
     # Goes to homepage
@@ -34,7 +33,10 @@ def test_superuser_can_edit_existing_permission(superuser, permission, testapp):
     # Defaults are kept -- setting ``form['user_id'] = permission.user.id`` is redundant
     form['collection_id'] = other_collection.id
     # Submits
-    res = form.submit().follow()
+    res = form.submit()
+    assert res.status_code == 302
+    assert res.location.endswith(url_for('permission.home'))
+    res = res.follow()
     assert res.status_code == 200
     # The permission was updated, and number of permissions are the same as initial state
     assert _('Updated permissions for "%(username)s" on collection "%(code)s".',
@@ -45,10 +47,87 @@ def test_superuser_can_edit_existing_permission(superuser, permission, testapp):
     assert len(res.lxml.xpath("//td[contains(., '{0}')]".format(other_collection.code))) == 1
 
 
-# noinspection PyUnusedLocal
-def test_superuser_sees_error_message_if_permission_is_already_registered(superuser,
-                                                                          permission,
-                                                                          testapp):
+def test_cataloging_admin_can_edit_permission_from_collection_view(user, permission, superuser,
+                                                                   testapp):
+    """Edit existing permission from collection view as cataloging admin."""
+    # Make 'user' cataloging admin for 'permission.collection' and 2nd one
+    PermissionFactory(user=user, collection=permission.collection,
+                      cataloging_admin=True).save_as(superuser)
+    initial_collection_code = permission.collection.code
+    other_permission = PermissionFactory(user=user, cataloging_admin=True).save_as(superuser)
+    old_permission_count = len(Permission.query.all())
+    # Goes to homepage
+    res = testapp.get('/')
+    # Fills out login form
+    form = res.forms['loginForm']
+    form['username'] = user.email
+    form['password'] = 'myPrecious'
+    # Submits
+    res = form.submit().follow()
+    # Clicks to View Collection from profile
+    res = res.click(href=url_for('collection.view', collection_code=permission.collection.code))
+    # Clicks Edit on a permission
+    res = res.click(href=url_for('permission.edit', permission_id=permission.id))
+    # Fills out the form, by changing to another collection
+    form = res.forms['editPermissionForm']
+    # Defaults are kept -- setting ``form['user_id'] = permission.user.id`` is redundant
+    form['collection_id'] = other_permission.collection.id
+    # Submits
+    res = form.submit()
+    assert res.status_code == 302
+    assert url_for('collection.view', collection_code=initial_collection_code) in res.location
+    res = res.follow()
+    assert res.status_code == 200
+    # The permission was updated, and number of permissions are the same as initial state
+    assert _('Updated permissions for "%(username)s" on collection "%(code)s".',
+             username=permission.user.email, code=other_permission.collection.code) in res
+    assert len(Permission.query.all()) == old_permission_count
+    # The edited permission is NOT listed on page for 'initial_collection_code'.
+    assert len(res.lxml.xpath("//td[contains(., '{0}')]".format(permission.user.email))) == 0
+
+
+def test_cataloging_admin_can_edit_permission_from_user_view(user, permission, superuser, testapp):
+    """Edit existing permission from user view as cataloging admin."""
+    # Make 'user' cataloging admin for 'permission.collection' and 2nd one
+    PermissionFactory(user=user, collection=permission.collection,
+                      cataloging_admin=True).save_as(superuser)
+    initial_permission_user_id = permission.user.id
+    other_user = UserFactory().save_as(superuser)
+    old_permission_count = len(Permission.query.all())
+    # Goes to homepage
+    res = testapp.get('/')
+    # Fills out login form
+    form = res.forms['loginForm']
+    form['username'] = user.email
+    form['password'] = 'myPrecious'
+    # Submits
+    res = form.submit().follow()
+    # Clicks to View Collection from profile
+    res = res.click(href=url_for('collection.view', collection_code=permission.collection.code))
+    # Clicks to View User on collection permissions view
+    res = res.click(href=url_for('user.view', user_id=permission.user_id))
+    # Clicks Edit on a permission on the user view
+    res = res.click(href=url_for('permission.edit', permission_id=permission.id))
+    # Fills out the form, by changing to another user
+    form = res.forms['editPermissionForm']
+    # Defaults are kept, setting ``form['collection_id'] = permission.collection.id`` is redundant
+    form['user_id'] = other_user.id
+    # Submits
+    res = form.submit()
+    assert res.status_code == 302
+    assert url_for('user.view', user_id=initial_permission_user_id) in res.location
+    res = res.follow()
+    assert res.status_code == 200
+    # The permission was updated, and number of permissions are the same as initial state
+    assert _('Updated permissions for "%(username)s" on collection "%(code)s".',
+             username=other_user.email, code=permission.collection.code) in res
+    assert len(Permission.query.all()) == old_permission_count
+    # The edited permission is NOT listed on the view for permissions of initial user.
+    assert len(res.lxml.xpath("//td/a[@href='{0}']".format(
+        url_for('permission.edit', permission_id=permission.id)))) == 0
+
+
+def test_superuser_sees_error_if_permission_is_already_registered(superuser, permission, testapp):
     """Show error if permission is edited to (user_id, collection_id) pair that already exists."""
     other_permission = PermissionFactory()
     # Goes to homepage
@@ -75,8 +154,10 @@ def test_superuser_sees_error_message_if_permission_is_already_registered(superu
           username=other_permission.user.email, code=other_permission.collection.code)) in res
 
 
-def test_user_cannot_edit_permission(user, permission, testapp):
-    """Attempt to edit a permission."""
+def test_user_cannot_edit_permission(user, permission, superuser, testapp):
+    """Attempt to edit a permission as non-'cataloging admin' user."""
+    assert user.is_cataloging_admin_for(permission.collection) is False
+    assert user.is_cataloging_admin is False
     # Goes to homepage
     res = testapp.get('/')
     # Fills out login form
@@ -92,5 +173,15 @@ def test_user_cannot_edit_permission(user, permission, testapp):
     # Try to go there directly
     testapp.get('/permissions/', status=403)
 
-    # Try to go directly to edit
-    testapp.get('/permissions/edit/1', status=403)
+    # Try to edit a specific permission directly
+    testapp.get(url_for('permission.edit', permission_id=permission.id), status=403)
+
+    # Accidentally be cataloging admin for unrelated collection and try again
+    PermissionFactory(user=user, cataloging_admin=True).save_as(superuser)
+    res = testapp.get(url_for('permission.edit', permission_id=permission.id))
+    form = res.forms['editPermissionForm']
+    form['registrant'].checked = not permission.registrant
+    res = form.submit()
+    # FIXME: The intended assertion below doesn't work.. :(
+    # assert _('You do not have sufficient privileges for this operation.') in res
+    assert res.status_code == 200  # Fallback, means we didn't redirect due to validation error.