Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workflow Draft - Working Space - Editing Version - Record Copy - Draft Punk #3592

Merged
merged 64 commits into from
Jun 19, 2019

Conversation

Delawen
Copy link
Contributor

@Delawen Delawen commented Feb 15, 2019

Related to #2577 and #1527

What it is

Different people use different naming for this feature (see title) so first I will explain what it is:

This is a new feature to be able to edit published and approved metadata without modifying the public version of that metadata. Only works if workflow is enabled. If workflow is not enabled, behaviour is same as usual.

Definition: "Records with workflow enabled", from now on, will be just "records".

What it does

When an approved record is edited, a copy/draft of that record is created to allow editors and reviewers to work on that record without modifying the approved version. All users that can see the record but has no editing/reviewing privileges over it, will only see the approved version, for them the draft/copy will not exist.

This way a user with enough privileges can view and edit the published record without modifying what the rest of the users (anonymous or non-editors of that record). This is specially useful for long editings (that last more than one session) or edits that need some kind of review before getting published.

This is very transparent to the user. The user will only see the draft copy when opening the record with the editor or on the metadata view, when clicking on the draft/copy link.

From now on we call this record "copy" as "draft".

How does it relate to publishing records

All published records will get the "approved" status automatically.

Developers hint

Some services have added an "approved" parameter (optional, true by default) which allows privileged users to do things with the draft instead of the record.

Storage

This draft/copy is saved on both the database and the index marked with the draft flag.

Drafts will be saved on a different table on the database, so we can keep a unique identifier by uuid on the metadata table. The lucene index will be the same, just adding the "draft" flag, which allows us to hide the draft records conveniently.

How does draft and approved version relate

Linkage between approved and draft records are mostly based on UUID. So if the UUID is changed (corner case), the draft may "disconnect" from the approved record. This is mandatory because we use UUID in too many places as the identifier of the metadata. As UUID is not an editable field on GeoNetwork, this shouldn't pose a problem except on some corner cases.

In case of "disconnection", once the draft is appoved, it will create a different new record with the new UUID, leaving the other one disconnected. But in a vanilla GN this shouldn't happen.

How to disable draft mode

Draft mode is enabled by default.

If you want to work with the previous version of the code, you can edit web/src/main/resources/config-spring-geonetwork.xml and comment the following section. Remember to restart the catalog after that change.

  <!-- Draft workspace/workflow -->
    <bean primary="true" class="org.fao.geonet.kernel.datamanager.draft.DraftMetadataIndexer"/>
    <bean primary="true" class="org.fao.geonet.kernel.datamanager.draft.DraftMetadataManager"/>
    <bean primary="true" class="org.fao.geonet.kernel.datamanager.draft.DraftMetadataUtils"/>

Remember that this only works with workflow enabled records. If workflow is not used, this feature is not active anyway.

Lucene index

The flag on Lucene is associated to both approved and draft records, with the following meaning:

  • draft:y = this is a draft (and therefore there is an approved record)
  • draft:n = this is not a draft and there is no draft associated to it
  • draft:e = this is not a draft and there is a draft associated to it

This means, whenever a draft is created or removed, the indexed version of the approved record gets updated.

@Delawen Delawen added this to the 3.8.0 milestone Feb 15, 2019
@Delawen Delawen changed the title [WIP] Workflow Draft - Working Space - Editing Version - Record Copy [WIP] Workflow Draft - Working Space - Editing Version - Record Copy - Draft Punk Feb 15, 2019
@Delawen
Copy link
Contributor Author

Delawen commented Feb 15, 2019

Working on tests and minor bugs. Once I get this finished, rebase and review :)

@Delawen
Copy link
Contributor Author

Delawen commented Feb 25, 2019

Almost ready to review, doing more intense testing to make sure...

@PascalLike
Copy link
Contributor

TEST CASE:
I have two users an editor and a reviewer (both in Sample group)

As editor:

  • I create a test with editor and I submit it.

As reviewer:

  • I approve and publish it.
  • I try to edit the record -> A draft is born.
  • I delete the record.

As admin:

  • I try to delete the users: I can delete the reviewer but not the editor because of:
Error when deleting user
IllegalArgumentException

Cannot delete a user that is also a metadata owner

The error can be reproduced in different ways, the bug is that there is some zombie draft associated with the user who creates the original record. On the contrary, if I delete the draft before deleting the original metadata record the delete user is allowed.

Expected behavior:
The draft is automatically removed with the original record OR the draft is somehow accessible after I removed the original record

@Delawen Delawen changed the title [WIP] Workflow Draft - Working Space - Editing Version - Record Copy - Draft Punk Workflow Draft - Working Space - Editing Version - Record Copy - Draft Punk Feb 26, 2019
@Delawen
Copy link
Contributor Author

Delawen commented Feb 26, 2019

Expected behavior:
The draft is automatically removed with the original record OR the draft is somehow accessible after I removed the original record

I strongly suspect you are trying to remove a metadata when you don't have privileges to do so as reviewer. Working on it.

@Delawen
Copy link
Contributor Author

Delawen commented Feb 27, 2019

I think this comes from another bug on GN related to having edit, delete,... buttons and services based on group privilege and not on privilege over the record and in the end an InvalidDataAccessApiUsageException is thrown. Which in current master is transparent, but lead to other issues here. Trying to work on this on a separated PR.

@Delawen
Copy link
Contributor Author

Delawen commented Feb 27, 2019

For example, this record is owned by a user:

imagen

And although it doesn't have privileges on the group for editing, a reviewer can "try to" edit it:
imagen

@PascalLike
Copy link
Contributor

True, I noticed the same and I tried to propose a solution here #3610 (but now requires to be added to different points of the GUI).

But the issue I reported happens even with this test case:

I have two users an editor and a reviewer (both in Sample group)

As editor:

  • I create a test with editor and I submit it.

As reviewer:

  • I approve and publish it.
  • I try to edit the record -> Draft is created.

As editor:

  • I delete the record.

As admin:

  • I try to delete the users: I can delete the reviewer but not the editor because of:
Error when deleting user
IllegalArgumentException

Cannot delete a user that is also a metadata owner

The editor can delete his metadata, right?

@Delawen
Copy link
Contributor Author

Delawen commented Feb 27, 2019

But when editing with the reviewer, you are creating a draft... associated to the reviewer, not to the editor. All my tests were the editor editing.

I am hunting this to make it more robust so there is no draft orphan no matter what.

@Delawen Delawen force-pushed the draft2018 branch 2 times, most recently from 071cf3d to ceea8bd Compare February 28, 2019 11:37
@CMath04
Copy link
Collaborator

CMath04 commented Mar 5, 2019

By doing some tests, I've found 2 more bugs:

Record not removed from the index when deleted

Bug description
When deleting a record that have a Draft copy attach to it, both the original and the draft copy are removed from the database but not from the index.
As long as the index is not rebuild, the record will still be shown in the search and contribution panel.

To reproduce

  1. Create a record
  2. Enable the workflow on it and set its status to “Approved”
  3. Open the editor with this record (to create a Draft copy)
  4. Save & close
  5. Delete the record either from the contribution or the view panel.

Content of the original metadata under metadraf view

Bug description
In the Draft view (after clicking “This record has a draft version. Click here to see it.”), if we update the record status, the content of the view will be replaced with the content of the original metadata without changing the URL (/catalog.search#/metadraf/). Refreshing the web page will show the content of the Draft copy again.

To reproduce
As an admin or a reviewer:

  1. Go to the record view of a metadata that have a draft attached to it (…/catalog.search#/metadata/)
  2. Switch to the Draft copy view by clicking “This record has a draft version. Click here to see it.”. (…/catalog.search#/metadraf/)
  3. Change the workflow status from “Draft” to anything else (Manage record -> update record status)

@Delawen Delawen added the critical Issue is critical and must be fixed in the next release label Mar 12, 2019
@Delawen
Copy link
Contributor Author

Delawen commented Mar 12, 2019

Thanks @ChaussierMathieu !

@Delawen Delawen force-pushed the draft2018 branch 3 times, most recently from cbe5f85 to 7e77e67 Compare March 13, 2019 15:44
@josegar74
Copy link
Member

josegar74 commented Mar 14, 2019

Some comments:

  • Reviewer can't access the draft version, steps done:

a) Users: editor, reviewer in sample group

b) Editor user creates a metadata, enable workflow and sets status to submitted

c) Reviewer user updates the status to Approved and publishes the metadata --> Metadata becomes public

d) Editor user edits the metadata again, updating the title and can access publish/draft versions.

e) Editor user sets status of draft version to Submitted

f) Reviewer can access the published version, but the link to view the draft version displays again the published version

  • Metadata detail page displays a link to view the draft version, but requires some styling, currently is not very clear.

draft-link

  • In the search results (for users that can access the draft version), there's nothing that indicates the user that the metadata has a draft version

search-results

  • In the editor dashboard it's displayed the published version, I think can be relevant to display the draft version there if the user has access to it.

  • I disabled the Spring beans related to this feature, to don't use the draft editing copy, but there's some side effect. Test case:

a) Editor creates a metadata and changes status to submitted.

b) Reviewer approves the metadata and publishes it --> Metadata becomes public

c) Reviewer (or Editor) edit and save the metadata.

Expected: the change is available publicly
Result: the metadata is unpublished and requires publication again

@Delawen Delawen force-pushed the draft2018 branch 2 times, most recently from 0d7914c to de7ed9d Compare March 21, 2019 14:00
fxprunayre and others added 5 commits May 9, 2019 13:19
…atasets (#3784)

* Associated resources / Clarify links added when linking service and
datasets

* Associated resources / Clarify links added when linking service and
    datasets / Cleanup url once linking.
An extra setting is added for the background colour on hover and active colour
Copy link
Contributor

@PascalLike PascalLike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit list is pretty long... what could be a good strategy to merge this?

josegar74 and others added 7 commits June 14, 2019 10:02
… an editing copy until removed the editing copy
…t. Rename button to 'Save draft' so user can make a clear distinction between a record and a draft.
… of the groups of the owner in 'hasEditPermission' method: code was using user groups of the metadata owner instead that could be not related to the metadata and not checking the current logged user. Original code seem was already fine
…rking copy' to make clear that the published version is not removed when removing the working copy (draft) version
@fxprunayre fxprunayre mentioned this pull request Jun 18, 2019
7 tasks
@josegar74
Copy link
Member

Merging the branch, additional work to review cases described in https://docs.google.com/spreadsheets/d/1TTocIhI94X5OoEURBVhQsqRnRjzGnPWW-WoMy5Yxr3Y/edit#gid=0 during Bolsena codesprint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
critical Issue is critical and must be fixed in the next release enhancement help wanted new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants