Skip to content

test behavior of deleting orphaned map annotations#4907

Merged
jburel merged 1 commit intoome:developfrom
mtbc:deleting-maps
Dec 10, 2016
Merged

test behavior of deleting orphaned map annotations#4907
jburel merged 1 commit intoome:developfrom
mtbc:deleting-maps

Conversation

@mtbc
Copy link
Copy Markdown
Member

@mtbc mtbc commented Oct 26, 2016

What this PR does

Adds integration testing to verify that in deletion of objects, associated map annotations are deleted if thus orphaned.

Testing this PR

https://ci.openmicroscopy.org/job/OMERO-DEV-merge-integration-java/lastCompletedBuild/testngreports/integration.delete/HierarchyDeleteTest/ should pass.

@pwalczysko
Copy link
Copy Markdown
Member

The two concrete tests added here https://ci.openmicroscopy.org/job/OMERO-DEV-merge-integration-java/400/testngreports/integration.delete/HierarchyDeleteTest/testDeleteOrphanedMapAnnotationViaFolders and the one right below it are passing. So is the whole build.
Read the tests and understood.
Ready to merge fmpov.

@joshmoore
Copy link
Copy Markdown
Member

I think this leaves me raising the same question @sbesson did elsewhere about whether or not map annotations should in fact be non-shareable. cc: @eleanorwilliams @manics @aleksandra-tarkowska regarding mapr usage.

@mtbc
Copy link
Copy Markdown
Member Author

mtbc commented Nov 4, 2016

The deletion machinery can also cope with comments that annotate multiple images given that everything in our model allows them: it does the same kind of orphaning check, though with the difference that it is more eager to delete others' orphaned comments even outside read-write groups. Even if the graphical clients were to encourage treatment of map annotations as non-sharable then if the server doesn't enforce that then it has to cope somehow with their being shared. (Its willingness to let others' map annotations be orphaned in read-annotate groups is separate from this PR's tests.)

@mtbc
Copy link
Copy Markdown
Member Author

mtbc commented Nov 4, 2016

(Currently the graph rules treat map annotations similarly to XML annotations.)

@joshmoore
Copy link
Copy Markdown
Member

Even if the graphical clients encourage treatment of map annotations as non-sharable then if the server doesn't enforce that then it has to cope somehow with their being shared.

And I think I'm proposing that they be orphaned and not deleted.

(Its willingness to let others' map annotations be orphaned in read-annotate groups is separate from this PR's tests.)

How do you mean?

@mtbc
Copy link
Copy Markdown
Member Author

mtbc commented Nov 4, 2016

And I think I'm proposing that they be orphaned and not deleted.

Ah, okay; I had read "non-sharable" as "more dispensable". Can we merge this as is so it at least helps document current behavior and open a card (whether 5.3.0 or IDR) for changing behavior? Probably worth a wider discussion of other annotations like XML ones. Also has larger implications for, e.g., what to leave behind of others' annotations if moving data to another group or downgrading a group to private.

How do you mean?

This PR doesn't test anything about others' map annotations.

@mtbc
Copy link
Copy Markdown
Member Author

mtbc commented Nov 4, 2016

(Note that in the meantime clients can have the map annotation orphans never deleted by setting excludeType=['MapAnnotation']; this can also select by namespace.)

@mtbc
Copy link
Copy Markdown
Member Author

mtbc commented Dec 7, 2016

Created https://trello.com/c/Gnoi9mTM/141-never-delete-orphaned-map-annotations so that this PR may now be merged.

@jburel
Copy link
Copy Markdown
Member

jburel commented Dec 10, 2016

Merging now that the discussion has been captured.

@jburel jburel merged commit 9afd163 into ome:develop Dec 10, 2016
@mtbc mtbc deleted the deleting-maps branch December 12, 2016 18:01
@jburel jburel added this to the 5.3.0 milestone Mar 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants