Skip to content

Commit

Permalink
Allow update/delete of records in published collections
Browse files Browse the repository at this point in the history
Previously only record:admin could update/delete records in published collections, the idea being that non-curator users with write access shouldn't be able to make changes that automatically propagate to a catalog without curator oversight. But in practice only curators have record:write permissions (with senior curators having record:admin).

The nature of permissions for non-curator users or systems that require write access to records can be addressed in future, as needed. That may in any case become moot if record submission is handled separately (via an archive API) from record management.
  • Loading branch information
marksparkza committed Oct 25, 2023
1 parent 945372c commit 12ded16
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 38 deletions.
43 changes: 11 additions & 32 deletions odp/api/routers/record.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,36 +432,15 @@ def _set_record(
if not create and auth.collection_ids != '*' and record.collection_id not in auth.collection_ids:
raise HTTPException(HTTP_403_FORBIDDEN)

if not ignore_collection_tags:
is_frozen = Session.execute(
select(CollectionTag).
where(CollectionTag.collection_id == record_in.collection_id).
where(CollectionTag.tag_id == ODPCollectionTag.FROZEN)
).first() is not None

is_published = Session.execute(
select(CollectionTag).
where(CollectionTag.collection_id == record_in.collection_id).
where(CollectionTag.tag_id == ODPCollectionTag.PUBLISHED)
).first() is not None

is_harvested = Session.execute(
select(CollectionTag).
where(CollectionTag.collection_id == record_in.collection_id).
where(CollectionTag.tag_id == ODPCollectionTag.HARVESTED)
).first() is not None

if is_frozen:
raise HTTPException(
HTTP_422_UNPROCESSABLE_ENTITY,
'Cannot update a record belonging to a frozen collection',
)

if is_published and not is_harvested:
raise HTTPException(
HTTP_422_UNPROCESSABLE_ENTITY,
'Cannot update a record belonging to a published collection',
)
if not ignore_collection_tags and Session.execute(
select(CollectionTag).
where(CollectionTag.collection_id == record_in.collection_id).
where(CollectionTag.tag_id == ODPCollectionTag.FROZEN)
).first() is not None:
raise HTTPException(
HTTP_422_UNPROCESSABLE_ENTITY,
'Cannot update a record belonging to a frozen collection',
)

if record_in.doi and Session.execute(
select(Record).
Expand Down Expand Up @@ -548,11 +527,11 @@ def _delete_record(
if not ignore_collection_tags and Session.execute(
select(CollectionTag).
where(CollectionTag.collection_id == record.collection_id).
where(CollectionTag.tag_id.in_((ODPCollectionTag.FROZEN, ODPCollectionTag.PUBLISHED)))
where(CollectionTag.tag_id == ODPCollectionTag.FROZEN)
).first() is not None:
raise HTTPException(
HTTP_422_UNPROCESSABLE_ENTITY,
'Cannot delete a record belonging to a published or frozen collection',
'Cannot delete a record belonging to a frozen collection',
)

if Session.get(PublishedRecord, record_id):
Expand Down
8 changes: 2 additions & 6 deletions test/api/test_record.py
Original file line number Diff line number Diff line change
Expand Up @@ -628,10 +628,6 @@ def test_update_record(api, record_batch, admin_route, scopes, collection_tags,
assert_unprocessable(r, 'Cannot update a record belonging to a frozen collection')
assert_db_state(record_batch)
assert_no_audit_log()
elif not admin_route and ODPCollectionTag.PUBLISHED in collection_tags and ODPCollectionTag.HARVESTED not in collection_tags:
assert_unprocessable(r, 'Cannot update a record belonging to a published collection')
assert_db_state(record_batch)
assert_no_audit_log()
else:
if record.doi and parent_doi:
record.parent = record_batch[0]
Expand Down Expand Up @@ -855,8 +851,8 @@ def test_delete_record(api, record_batch_with_ids, admin_route, scopes, collecti
r = api(scopes, api_client_collections).delete(f'{route}{(record_id := record_batch_with_ids[2].id)}')

if authorized:
if not admin_route and set(collection_tags) & {ODPCollectionTag.FROZEN, ODPCollectionTag.PUBLISHED}:
assert_unprocessable(r, 'Cannot delete a record belonging to a published or frozen collection')
if not admin_route and ODPCollectionTag.FROZEN in collection_tags:
assert_unprocessable(r, 'Cannot delete a record belonging to a frozen collection')
assert_db_state(record_batch_with_ids)
assert_no_audit_log()
elif is_published_record:
Expand Down

0 comments on commit 12ded16

Please sign in to comment.