samvera / hyrax

Hyrax is a Ruby on Rails Engine built by the Samvera community. Hyrax provides a foundation for creating many different digital repository applications.
http://hyrax.samvera.org/
Apache License 2.0
183 stars 123 forks source link

When an object is edited and is in a workflow with mediation steps, the object state should be set to Inactive and should re-enter mediation #172

Open mjgiarlo opened 7 years ago

mjgiarlo commented 7 years ago

Issue by mjgiarlo Friday Sep 02, 2016 at 21:17 GMT Originally opened as https://github.com/projecthydra/sufia/issues/2629


mjgiarlo commented 7 years ago

Comment by hortongn Wednesday Sep 14, 2016 at 20:09 GMT


@vantuyls @mjgiarlo I have a few hypothetical situations that I need to understand:

1) If a work is published and can be discovered via the GUI, and someone edits that work, the work should become inactive again. That means the work suddenly disappears from public view after re-entering mediation. Maybe I'm thinking too far ahead here and we shouldn't worry about this yet, but it seems like you wouldn't want published content to go missing from the user's perspective.

I would think that editing a work should create a new "inactive" copy of that work. The inactive copy would go through mediation while the original active copy would remain discoverable to users. Once the inactive work is approved, it becomes active and replaces the original active copy of the work. Or do we just deal with this problem in a later issue?

2) Lets say we have a work with several files attached and someone edits just one of the the files (file_sets). What should happen? Generic_works currently have a "state" that can be set to inactive, but file_sets do not. So should one of the tasks for this issue be to add a state to file_sets? That would require us to include CurationConcerns::Publishable in CC's file_set_behavior.rb, correct?

mjgiarlo commented 7 years ago

Comment by mjgiarlo Thursday Sep 15, 2016 at 00:45 GMT


@hortongn Great questions!

1) We touched upon this in #2452 but failed to link the relevant requirements here. Here's the blurb that addresses your first question:

One potential complication we discussed is how far we’d want to take the notion of “draft” or inactive objects. For instance, should we aim to support “unpublishing” an object, moving it from published back to a draft state where it may be edited further? Some folks in the room had experience implementing functionality like this in the past and shared that it gets gnarly very quickly, and the task can evolve into re-inventing version control with the potentiality for multiple users to “check out” an object, work on it, and then try to re-publish it. Our sense was that it’d be sufficient to use the draft/unpublished/inactive state as a starting point, and once something is published, it’s published (until such time as someone recommends a technical design for "mediated editing" and contributes resources towards that feature).

So, I'm thinking we should deprioritize this ticket and punt it to a later sprint. @vantuyls @scholarworks @hannahfrost: Cool to punt on mediated editing for now and come back to it?

2) Ha, I was just thinking about this while looking at @blancoj's PR and some discussion on Slack, and I added a "note" here: https://github.com/projecthydra/sufia/projects/1#card-31446. (See the #community-standup channel for my proposal that we give the GitHub project feature a spin.) Let's verify that FileSets are discoverable/visible independent of Works before prioritizing this work (we can turn that note into an issue automagically). Ultimately I agree with you that we ought to mix Publishable into the FileSetBehavior in CC, yes, and also to have tooling that automatically updates the publication_state of all FileSets within a work when the Work's publication_state is changed. Thanks!

mjgiarlo commented 7 years ago

Comment by hortongn Thursday Sep 15, 2016 at 15:51 GMT


Thanks @mjgiarlo. I unassigned myself from this issue and put in back in the ready column.

mjgiarlo commented 7 years ago

Comment by mjgiarlo Friday Oct 28, 2016 at 22:42 GMT


Defer working on this issue until we have an implementation of a working 1+-step workflow in Sufia.

mjgiarlo commented 7 years ago

Comment by mjgiarlo Friday Oct 28, 2016 at 22:42 GMT


Same as #2794, in which it is concluded that the entire workflow should be restarted as a first cut.

mjgiarlo commented 7 years ago

Comment by vantuyls Friday Oct 28, 2016 at 22:42 GMT


also cross reference with #2794

mjgiarlo commented 7 years ago

Comment by vantuyls Friday Oct 28, 2016 at 22:42 GMT


curse you, @mjgiarlo

vantuyls commented 7 years ago

We'd like to try to get some resolution on our goals for this issue ASAP. Looks like we've had a bit of back and forth about how to approach a workflow item being edited: does it re-enter the workflow? can one edit an item after it has gone through the workflow without further review? Any thoughts, clarity?

@hannahfrost @ggeisler @mjgiarlo @randalldfloyd @jeremyf @scholarworks @jcoyne and anyone else?

mjgiarlo commented 7 years ago

Thanks, @vantuyls. Clarifying questions for @hannahfrost @ggeisler @jeremyf @scholarworks @jcoyne:

  1. Does this mean that when a work in the one-step workflow is edited after it is deposited, it automatically moves from the deposited state to the pending_review state?
  2. If so, can we use the existing request_review action (https://github.com/projecthydra-labs/hyrax/blob/master/lib/generators/hyrax/templates/mediated_deposit_workflow.json.erb#L51-L61) for this, assuming we also add a call to DeactivateObject to its methods?
  3. Should we also warn the user in the UI when they go to edit a work that will re-enter workflow?
  4. Do we want to mediate all or just some edit actions, Including:
    1. Metadata edits?
    2. File deletions/additions?
    3. Changes to visibility?
    4. Changes to relationships (collection membership, admin set membership, adding parent/child works)?
    5. Sharing with other users or groups?
  5. Do we also want to mediate deletes?
  6. Should we also mediate File Manager actions such as reordering and relabeling contained FileSets, and choosing a thumbnail/representative media?
randalldfloyd commented 7 years ago

@vantuyls @mjgiarlo So this is probably an uninformed question, but what support is there in AF/Sufia for versioning works (not files on works, as I understand there is some support for that already), and also how (or does) that align to Fedora 4 versioning for resources? Seems relevant here (ducking rocks made entirely of jagged edges.)

mjgiarlo commented 7 years ago

@randalldfloyd Support for this is already in AF, yes, and it aligns with Fedora 4 versioning. See: https://github.com/projecthydra/active_fedora/blob/master/spec/integration/versionable_spec.rb

What we don't have, and what would grow the scope of this ticket, is 1. awareness in Hyrax (or Sufia) that resources can be versioned, and 2. UI around resource versioning (which I imagine would benefit from more discussion and designer cycles).

🍅 😉

hannahfrost commented 7 years ago

I feel given the current implementation, specifically the limited number of roles, we should lean toward re-mediating a work that is undergoing changes. When we have more roles defined and implemented, we can revisit. Specific thoughts to questions from @mjgiarlo below.

Automatically moving a mediated work to pending review state if it is modified after original deposit: +1 Warn user that it is re-entering workflow: +1 Mediate all actions that involve changes to metadata, files, visibility, relationships: +1 Mediate sharing with other users or groups: -1 Mediate deletes: +1 Mediate file manager changes: +1

vantuyls commented 7 years ago

+1 to @hannahfrost and her plus/minus list.

As for versioning - i agree with @mjgiarlo - though this is definitely something i'd like to see in Hyrax at some point. I talked to a few folks and HC this year and there was some general interest in file versioning as a feature.

mjgiarlo commented 7 years ago

@vantuyls Basic file versioning is already supported in Hyrax, but I've heard the same rumblings and we need similar functionality for Stanford's Hyku-based IR, including expanded file versioning support (TBD) and "work" versioning support. So, suffice to say, a lot of us want to see this happen!

vantuyls commented 7 years ago

uhhhh, by file versioning do you mean file replacement?

mjgiarlo commented 7 years ago

@vantuyls Nope! It does versioning, but you might not believe me because the UI only shows non-editors the latest version. Though, caveat, I have not personally QAed this in a long while.

randalldfloyd commented 7 years ago

@vantuyls @mjgiarlo So I sensed some consensus on returning a work to pending review when modified as a simple first pass, regardless of all the discussion that raises. Is that enough consensus to act on?

mjgiarlo commented 7 years ago

@randalldfloyd :shipit:

randalldfloyd commented 7 years ago

@hannahfrost @ggeisler @jeremyf @scholarworks @jcoyne:

I've been a little stuck here trying to figure out how to return a work to the initial starting point of the workflow for an existing Sipity entity for a work without being opinionated about behavior and making brittle code bound to the current implementation (i.e. assuming what state it should go back to based on how it was first started, coding too much Sipity behavior into the update for an actor, etc.) Then it occurred to me that it's potentially easier than that. Could we not instead just define a restart_workflow or edit_work step in configuration (with a method to handle it) that defines what should happen? Then the update override on the actor would just need to know how to invoke the named step to initiate a restart. Or maybe that's how everyone thought it should work and I just now got on the same page :)

randalldfloyd commented 7 years ago

@vantuyls @mjgiarlo

I'm starting to think this story needs some design/implementation discussion around it. My initial take on the story and how to implement it suddenly changed after a brief conversation over Slack with Justin.

I had originally thought what we needed was a mechanism to make sure an object in a workflow goes back to a step for that workflow as the object is being saved. The way I was going about it, the regular "edit" functionality would be invoked from the UI in the way we are all used to in Sufia/CC/Hyrax and then the update method (i.e. in the actor stack) would invoke a named step who's behavior could be configured in the workflow json. Justin's take on it was that there would be different UI interactions that the user would use to edit something deposited via a workflow. In this way, the transition would come first, not afterwards as it would on update, and it makes it a lot more like the other mechanisms for workflow transitions. So that got me thinking that what we're really talking about is rewiring what the edit controls fundamentally do (or hiding the default ones?), vs. just letting a user get into a normal edit/save cycle and hope for the best.

Anyhow, I don't think I should be interpreting that on my own or going in and unhooking/rewiring how edit controls work. Thoughts?

vantuyls commented 7 years ago

@jcoyne @mjgiarlo @scholarworks @hannahfrost @randalldfloyd

Sounds like we need a little discussion on this before randall can move forward. Any thoughts from this group as to how putting an object back through a workflow should be wired up?

vantuyls commented 7 years ago

Just got off a call with @mjgiarlo @scholarworks about this issue. What we've settled on (pending discussion here from @jcoyne @jeremyf @randalldfloyd and others) is that:

If i've missed something from our conversation, feel free to comment here.

jeremyf commented 7 years ago

@vantuyls I don't have an opinion about this

randalldfloyd commented 7 years ago

@mjgiarlo @scholarworks @jcoyne @jeremyf @vantuyls

As a user the behavior described makes the most sense to me. From an implementation point of view, it will be clear what's going on because managing the transition via a UI element will (I think) be consistent with how the current workflow action form works. In contrast, trying to manage the transition from within the actor stack on update will be hidden and easily broken, as I've already experienced while trying to make it work that way.

jcoyne commented 7 years ago

That sounds like a good plan to me.

hannahfrost commented 7 years ago

That sounds like a good plan to me too.

tpendragon commented 7 years ago

Hi, is this only for workflows where it's defined as some sort of special "mediated" workflow? Because ours isn't a mediated process - each step isn't a checkmark that says "okay, I looked at it, we're good, on to the next set of people to checkmark it." It's more like each step in the workflow is "okay, next group, you have work to do."

So if we start with metadata, that's marked, it goes on to the next step for checking pages order/etc, someone fixes that, and it reset the workflow then it'd be adding work and notifications to people who have already done their job for that item. Further, many of our items are distributed amongst a number of our public sites via IIIF manifests - taking items down because an item is repaired is inexcusable, especially since the parties involved in the whole process would take a while to get it all back up again.

If this has to go in one way or another, we need some way to mark a workflow as for "mediated deposit" or a way to turn off all the functionality in the checkboxes above.

jcoyne commented 7 years ago

Given @tpendragon's use case, we need to differentiate between those users who can?(:edit, obj) and those who could_reset_to_edit_state?(obj). If they can?(:edit, obj) in it's present state (@tpendragon's users), then don't reset it. Sound good?

tpendragon commented 7 years ago

That works for me, but I don't see how it fulfills the original use case of "when I edit the resource it should reset the workflow"

jcoyne commented 7 years ago

@tpendragon The use case is for people who don't presently have edit access (which was revoked after they submitted their work for approval). For them, pressing the "Edit" (or "Update") button would reset the workflow to that initial state.

tpendragon commented 7 years ago

@jcoyne Oh. I mean, I have some concerns that people without edit access in ability.rb can force a record out of public view, but it doesn't really effect me. So, :+1: to the proposal. Works for my use case.

jcoyne commented 7 years ago

@tpendragon this would be restricted to users who have a depositor role, and you'd have to encode it in your workflow.json, so it wouldn't be a concern.

randalldfloyd commented 7 years ago

@jcoyne So just so I'm clear, it's still possible that someone can?(:edit, obj), in which case edit will continue to work as it always has with no affect on either visibility or it's current state (i.e. deposited) if it came from a workflow. Referring to your comment of "the use case is for people who don't presently have edit access (which was revoked after they submitted their work for approval)", that's the first time I've heard it put that way. In reading the story and all it's comments at face value I always assumed this was for anytime the item is edited by anyone after deposit. Not voicing a dissenting opinion, just trying to get the use case right.

jcoyne commented 7 years ago

@randalldfloyd Trey's use case brought to light that some users need to edit the work without forcing it to restart the workflow. My assumption that the difference between those users is that they have edit access to the work, where the user in this story does not (because it's been revoked). We need to do something to satisfy this story, without impinging on Trey's use case.

randalldfloyd commented 7 years ago

@jcoyne Got it (I think.) Edit ability vs. depositor in a workflow is the only criteria we can check in order to satisfy both use cases, so I'll just assume can edit? trumps workflow entanglements. So how are regular abilities and workflow roles handled if a user ends up with overlaps? They are separate, right, so that's possible? I'm assuming a user's existing edit ability isn't revoked just because they happen to also be in a depositor group for the work in question. Revoked means they never really had ability except to facilitate the workflow role(?)

mjgiarlo commented 7 years ago

@randalldfloyd @vantuyls Should I leave this issue in the In Progress column on the waffles?

jcoyne commented 7 years ago

@mjgiarlo this issue doesn't seem like it is actionable, can we remove it from the ready column?

mjgiarlo commented 7 years ago

@jcoyne Done.

mjgiarlo commented 7 years ago

This ticket has a subticket: https://github.com/samvera/hyrax/issues/556