From 1a15624e13bfe0a3bdddb1f0c5bf8940c9f04a04 Mon Sep 17 00:00:00 2001 From: Bryan Newbold Date: Fri, 21 Dec 2018 14:10:56 -0800 Subject: more edit edgecases; editgroup status check --- python/tests/api_entity_editing.py | 53 +++++++++++++++++++++++++++++++++++++- python/tests/api_entity_state.py | 18 +++++++++++++ python/tests/api_misc.py | 38 +++++++++++++++++++++++++++ rust/src/api_helpers.rs | 17 ++++++++++++ rust/src/api_server.rs | 1 + rust/src/api_wrappers.rs | 5 +++- 6 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 python/tests/api_misc.py diff --git a/python/tests/api_entity_editing.py b/python/tests/api_entity_editing.py index 3c0baa8e..d14b949b 100644 --- a/python/tests/api_entity_editing.py +++ b/python/tests/api_entity_editing.py @@ -40,6 +40,31 @@ def test_multiple_edits_same_group(api): api.accept_editgroup(eg.id) +def test_edit_after_accept(api): + + c1 = CreatorEntity(display_name="test updates") + + # create + eg = quick_eg(api) + c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) + api.accept_editgroup(eg.id) + + # should be unable to create an edit on an old editgroup + c2 = CreatorEntity(display_name="left") + try: + api.create_creator(c2, editgroup=eg.id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + # TODO: need better message + #assert "accepted" in e.body + + # cleanup + eg = quick_eg(api) + api.delete_creator(c1.ident) + api.accept_editgroup(eg.id) + + def test_edit_deletion(api): c1 = CreatorEntity(display_name="test edit updates") @@ -88,6 +113,32 @@ def test_delete_accepted_edit(api): api.accept_editgroup(eg.id) # try to delete - with pytest.raises(fatcat_client.rest.ApiException): + try: api.delete_creator_edit(edit.edit_id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + assert "accepted" in e.body + + +def test_wip_revision(api): + + c1 = CreatorEntity(display_name="test edit nugget") + + # fetch revision before accepting + eg = quick_eg(api) + c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) + rev = api.get_creator_revision(c1.revision) + assert "nugget" in rev.display_name + assert rev.state is None + assert rev.ident is None + assert rev.revision == c1.revision + + # fetch revision after accepting + api.accept_editgroup(eg.id) + rev = api.get_creator_revision(c1.revision) + assert "nugget" in rev.display_name + assert rev.state is None + assert rev.ident is None + assert rev.revision == c1.revision diff --git a/python/tests/api_entity_state.py b/python/tests/api_entity_state.py index 35a8527a..43827dff 100644 --- a/python/tests/api_entity_state.py +++ b/python/tests/api_entity_state.py @@ -426,3 +426,21 @@ def test_required_entity_fields(api): assert 400 <= e.status < 500 assert "title" in e.body +def test_revert_current_status(api): + + c1 = CreatorEntity(display_name="test updates") + + # create + eg = quick_eg(api) + c1 = api.get_creator(api.create_creator(c1, editgroup=eg.id).ident) + api.accept_editgroup(eg.id) + + # try to "revert" to current revision + eg = quick_eg(api) + c1_revert = CreatorEntity(revision=c1.revision) + try: + api.update_creator(c1.ident, c1_revert, editgroup=eg.id) + assert False + except fatcat_client.rest.ApiException as e: + assert 400 <= e.status < 500 + assert "current" in e.body diff --git a/python/tests/api_misc.py b/python/tests/api_misc.py new file mode 100644 index 00000000..3510ea82 --- /dev/null +++ b/python/tests/api_misc.py @@ -0,0 +1,38 @@ + +import json +import pytest +from copy import copy + +from fatcat_client import * +from fatcat_client.rest import ApiException +from fixtures import * + + +def test_lookups(api): + + api.lookup_creator(orcid='0000-0003-3118-6859') + api.lookup_container(issnl='1549-1277') + api.lookup_file(sha256='ffc1005680cb620eec4c913437dfabbf311b535cfe16cbaeb2faec1f92afc362') + api.lookup_release(pmid='54321') + api.lookup_release(isbn13='978-3-16-148410-0') + +def test_lookup_hide_extend(api): + + r = api.lookup_release(doi='10.1371/journal.pmed.0020124') + assert len(r.refs) >= 2 + assert r.files is None + assert r.container is None + assert len(r.container_id) > 10 + assert r.abstracts == [] + + r = api.lookup_release(doi='10.1371/journal.pmed.0020124', expand='files', hide='refs,abstracts') + assert r.refs is None + assert len(r.files[0].sha1) == 40 + assert r.container is None + assert r.abstracts is None + + r = api.lookup_release(doi='10.1371/journal.pmed.0020124', expand='container,abstracts') + assert len(r.refs) >= 2 + assert r.files is None + assert r.container.issnl + assert r.abstracts == [] diff --git a/rust/src/api_helpers.rs b/rust/src/api_helpers.rs index 77377531..b6525546 100644 --- a/rust/src/api_helpers.rs +++ b/rust/src/api_helpers.rs @@ -21,6 +21,22 @@ pub struct EditContext { pub autoaccept: bool, } +impl EditContext { + + /// This function should always be run within a transaction + pub fn check(&self, conn: &DbConn) -> Result<()> { + let count: i64 = changelog::table + .filter(changelog::editgroup_id.eq(&self.editgroup_id.to_uuid())) + .count() + .get_result(conn)?; + if count > 0 { + return Err(ErrorKind::EditgroupAlreadyAccepted(self.editgroup_id.to_string()).into()); + } + return Ok(()); + } +} + + #[derive(Clone, Copy, PartialEq)] pub struct ExpandFlags { pub files: bool, @@ -202,6 +218,7 @@ pub fn get_or_create_editgroup(editor_id: Uuid, conn: &DbConn) -> Result { pub fn accept_editgroup(editgroup_id: FatCatId, conn: &DbConn) -> Result { // check that we haven't accepted already (in changelog) // NB: could leave this to a UNIQUE constraint + // TODO: redundant with check_edit_context let count: i64 = changelog::table .filter(changelog::editgroup_id.eq(editgroup_id.to_uuid())) .count() diff --git a/rust/src/api_server.rs b/rust/src/api_server.rs index 0961194b..d03fce07 100644 --- a/rust/src/api_server.rs +++ b/rust/src/api_server.rs @@ -24,6 +24,7 @@ macro_rules! entity_batch_handler { ) -> Result> { let edit_context = make_edit_context(conn, editgroup_id, autoaccept)?; + edit_context.check(&conn)?; let model_list: Vec<&models::$model> = entity_list.iter().map(|e| e).collect(); let edits = $model::db_create_batch(conn, &edit_context, model_list.as_slice())?; diff --git a/rust/src/api_wrappers.rs b/rust/src/api_wrappers.rs index aa7f9ec3..fe9cd793 100644 --- a/rust/src/api_wrappers.rs +++ b/rust/src/api_wrappers.rs @@ -36,7 +36,7 @@ macro_rules! wrap_entity_handlers { _context: &Context, ) -> Box + Send> { let conn = self.db_pool.get().expect("db_pool error"); - // No transaction for GET? + // No transaction for GET let ret = match (|| { let entity_id = FatCatId::from_str(&id)?; let hide_flags = match hide { @@ -88,6 +88,7 @@ macro_rules! wrap_entity_handlers { Some(FatCatId::from_str(&s)?) } else { None }; let edit_context = make_edit_context(&conn, editgroup_id, false)?; + edit_context.check(&conn)?; entity.db_create(&conn, &edit_context)?.into_model() }) { Ok(edit) => @@ -168,6 +169,7 @@ macro_rules! wrap_entity_handlers { Some(FatCatId::from_str(&s)?) } else { None }; let edit_context = make_edit_context(&conn, editgroup_id, false)?; + edit_context.check(&conn)?; entity.db_update(&conn, &edit_context, entity_id)?.into_model() }) { Ok(edit) => @@ -213,6 +215,7 @@ macro_rules! wrap_entity_handlers { None => None, }; let edit_context = make_edit_context(&conn, editgroup_id, false)?; + edit_context.check(&conn)?; $model::db_delete(&conn, &edit_context, entity_id)?.into_model() }) { Ok(edit) => -- cgit v1.2.3