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.

Sign up for GitHub

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

Jump to bottom

Allow ContentEventJob to receive either an ActiveFedora object or a Valkyrie resource #4193

Closed
randalldfloyd opened this issue Jan 22, 2020 · 2 comments · Fixed by #4278
Closed

Allow ContentEventJob to receive either an ActiveFedora object or a Valkyrie resource #4193

randalldfloyd opened this issue Jan 22, 2020 · 2 comments · Fixed by #4278
Assignees
@jeremyf
Labels
wings

Comments

@randalldfloyd
Copy link
Contributor

Descriptive summary

Convert ContentEventJob to allow it to receive either an ActiveFedora based object or a Valkyrie::Resource equivalent object.

Rationale

Allow jobs to execute correctly with existing AF implementation or the Wings Valkyrie implementation.

Expected behavior

ContentEventJob receives a Valkyrie::Resource and performs the action.

Actual behavior

ContentEventJob raises an exception when passed a Valkyrie::Resource.

Approach for Conversion

See Pattern for Jobs Receiving Parameter that may be an AF Object or a Valkyrie Resource

Related work

These have all be modified to support AF object or Valkyrie resource using the established pattern.
@bkeese bkeese mentioned this issue Feb 18, 2020
Merged
@jeremyf jeremyf self-assigned this Mar 23, 2020
jeremyf added a commit that referenced this issue Mar 23, 2020
@jeremyf
Refactoring to unify a subsystem of Event behavior
d2e6196 
Prior to this commit, the descendants of ContentEventJob each
approached writing their `action` string differently. With this
change, I've normalized that behavior. There is still work to be done
to get a handle on the whole event ecosystem.

Relates to: #4193
@jeremyf jeremyf mentioned this issue Mar 23, 2020
Closed
jeremyf added a commit that referenced this issue Mar 23, 2020
@jeremyf
Fixing logic around deleting nested membership
c5b49b4 
> When removing a collection after adding it to a work,
> CollectionsMembershipActor still adds the collection even though the
> _destroy flag is set to true.
>
> This is happening in both new/update works, in version 2.4.1, 2.7.0,
> and most likely 3.x.
>
> Steps to reproduce the past behavior
>
> 1.    Edit an existing work
> 2.    Add a collection to the work
> 3.    Remove the collection (before saving)
> 4.    Save the work

Based on this report, someone via the UI goes to a work, then chooses
to add a collection to the work. They then change their mind and mark
"delete" for that collection, then go to save. Prior to the logic that
is put in place, this would add the collection to the work.

The fix now prioritizes checking did someone say delete? If so, go
down that logic path else go down the add logic path.

Closes #4193

And yes, I did mean to disable to elements for Rubocop. That method
is complicated, and while I could extract an inner method, I felt that
that was not worth the current effort. I wear my shame proudly.
jeremyf added a commit that referenced this issue Mar 23, 2020
@jeremyf
Fixing logic around deleting nested membership
04ed479 
> When removing a collection after adding it to a work,
> CollectionsMembershipActor still adds the collection even though the
> _destroy flag is set to true.
>
> This is happening in both new/update works, in version 2.4.1, 2.7.0,
> and most likely 3.x.
>
> Steps to reproduce the past behavior
>
> 1.    Edit an existing work
> 2.    Add a collection to the work
> 3.    Remove the collection (before saving)
> 4.    Save the work

Based on this report, someone via the UI goes to a work, then chooses
to add a collection to the work. They then change their mind and mark
"delete" for that collection, then go to save. Prior to the logic that
is put in place, this would add the collection to the work.

The fix now prioritizes checking did someone say delete? If so, go
down that logic path else go down the add logic path.

Closes #4193

And yes, I did mean to disable to elements for Rubocop. That method
is complicated, and while I could extract an inner method, I felt that
that was not worth the current effort. I wear my shame proudly.
@jeremyf jeremyf mentioned this issue Mar 23, 2020
Merged
jeremyf added a commit that referenced this issue Mar 23, 2020
@jeremyf
Fixing logic around deleting nested membership
b234454 
> When removing a collection after adding it to a work,
> CollectionsMembershipActor still adds the collection even though the
> _destroy flag is set to true.
>
> This is happening in both new/update works, in version 2.4.1, 2.7.0,
> and most likely 3.x.
>
> Steps to reproduce the past behavior
>
> 1.    Edit an existing work
> 2.    Add a collection to the work
> 3.    Remove the collection (before saving)
> 4.    Save the work

Based on this report, someone via the UI goes to a work, then chooses
to add a collection to the work. They then change their mind and mark
"delete" for that collection, then go to save. Prior to the logic that
is put in place, this would add the collection to the work.

The fix now prioritizes checking did someone say delete? If so, go
down that logic path else go down the add logic path.

Closes #4193

And yes, I did mean to disable to elements for Rubocop. That method
is complicated, and while I could extract an inner method, I felt that
that was not worth the current effort. I wear my shame proudly.
jeremyf added a commit that referenced this issue Mar 23, 2020
@jeremyf
Fixing logic around deleting nested membership
7c543c9 
> When removing a collection after adding it to a work,
> CollectionsMembershipActor still adds the collection even though the
> _destroy flag is set to true.
>
> This is happening in both new/update works, in version 2.4.1, 2.7.0,
> and most likely 3.x.
>
> Steps to reproduce the past behavior
>
> 1.    Edit an existing work
> 2.    Add a collection to the work
> 3.    Remove the collection (before saving)
> 4.    Save the work

Based on this report, someone via the UI goes to a work, then chooses
to add a collection to the work. They then change their mind and mark
"delete" for that collection, then go to save. Prior to the logic that
is put in place, this would add the collection to the work.

The fix now prioritizes checking did someone say delete? If so, go
down that logic path else go down the add logic path.

Closes #4193
jeremyf added a commit that referenced this issue Mar 24, 2020
@jeremyf
Refactoring to unify a subsystem of Event behavior
a182c85 
Prior to this commit, the descendants of ContentEventJob each
approached writing their `action` string differently. With this
change, I've normalized that behavior. There is still work to be done
to get a handle on the whole event ecosystem.

Relates to: #4193
no-reply pushed a commit that referenced this issue Mar 31, 2020
@jeremyf
Fixing logic around deleting nested membership
ca21a7c 
> When removing a collection after adding it to a work,
> CollectionsMembershipActor still adds the collection even though the
> _destroy flag is set to true.
>
> This is happening in both new/update works, in version 2.4.1, 2.7.0,
> and most likely 3.x.
>
> Steps to reproduce the past behavior
>
> 1.    Edit an existing work
> 2.    Add a collection to the work
> 3.    Remove the collection (before saving)
> 4.    Save the work

Based on this report, someone via the UI goes to a work, then chooses
to add a collection to the work. They then change their mind and mark
"delete" for that collection, then go to save. Prior to the logic that
is put in place, this would add the collection to the work.

The fix now prioritizes checking did someone say delete? If so, go
down that logic path else go down the add logic path.

Closes #4193
jeremyf added a commit that referenced this issue Mar 31, 2020
@jeremyf
Backporting fix of logic around deleting nested membership
6f28a2f 
> When removing a collection after adding it to a work,
> CollectionsMembershipActor still adds the collection even though the
> _destroy flag is set to true.
>
> This is happening in both new/update works, in version 2.4.1, 2.7.0,
> and most likely 3.x.
>
> Steps to reproduce the past behavior
>
> 1.    Edit an existing work
> 2.    Add a collection to the work
> 3.    Remove the collection (before saving)
> 4.    Save the work

Based on this report, someone via the UI goes to a work, then chooses
to add a collection to the work. They then change their mind and mark
"delete" for that collection, then go to save. Prior to the logic that
is put in place, this would add the collection to the work.

The fix now prioritizes checking did someone say delete? If so, go
down that logic path else go down the add logic path.

Closes #4193
@jeremyf jeremyf mentioned this issue Mar 31, 2020
Merged
jeremyf added a commit that referenced this issue Mar 31, 2020
@jeremyf
Expanding backport of logic around deleting nested membership
21be50b 
In looking at a backport for #4193, I found similar logic in the
Hyrax::Actors::AttachMembersActor. I believe this is something that is
likely possible in the UI for work members.

Related to #4193 and #4278
@jeremyf jeremyf reopened this Apr 2, 2020
jeremyf added a commit that referenced this issue May 7, 2020
@jeremyf
Addressing case where children are added again
d874c05 
Prior to this commit, there was no guard in place to prevent adding
children again.

The logic would check "is something marked for delete" if not, then
add it. That needed to change, because when a user would submit a
form changing keywords, then we would re-add all of the already added
members.

Closes #4301 (which relates to #4193 and #4278)
Related to #4302
@jeremyf jeremyf mentioned this issue May 7, 2020
Merged
jeremyf added a commit that referenced this issue May 7, 2020
@jeremyf
Addressing case where children are added again
848d560 
Prior to this commit, there was no guard in place to prevent adding
children again.

The logic would check "is something marked for delete" if not, then
add it. That needed to change, because when a user would submit a
form changing keywords, then we would re-add all of the already added
members.

Closes #4301 (which relates to #4193 and #4278)
Related to #4302
jeremyf added a commit that referenced this issue May 7, 2020
@jeremyf
Addressing case where children are added again
aaa73d3 
Prior to this commit, there was no guard in place to prevent adding
children again.

The logic would check "is something marked for delete" if not, then
add it. That needed to change, because when a user would submit a
form changing keywords, then we would re-add all of the already added
members.

Closes #4301 (which relates to #4193 and #4278)
Related to #4302
jeremyf added a commit that referenced this issue May 12, 2020
@jeremyf
Refactoring to unify a subsystem of Event behavior
4946f4e 
Prior to this commit, the descendants of ContentEventJob each
approached writing their `action` string differently. With this
change, I've normalized that behavior. There is still work to be done
to get a handle on the whole event ecosystem.

Relates to: #4193
@jeremyf
Copy link
Contributor

jeremyf commented May 5, 2021 •
edited
Loading

The ContentEventJob has 6 subclasses:

  • ContentUpdateEventJob (tested via fbfef1b)
  • ContentNewVersionEventJob
  • FileSetAttachedEventJob
  • ContentRestoredVersionEventJob
  • ContentDepositorChangeEventJob
  • ContentDepositEventJob

This requires checking their implementations, but I suspect they are almost entirely a "noop"

@jeremyf
Copy link
Contributor

jeremyf commented May 5, 2021

The Hyrax::WithEvents#log_events (see code) exposes the common logging method.

Below are the five places that mixin the Hyrax::WithEvents module:

  • app/models/concerns/hyrax/user.rb
  • app/models/concerns/hyrax/file_set_behavior.rb
  • app/models/hyrax/resource.rb
  • app/models/concerns/hyrax/work_behavior.rb
  • app/presenters/hyrax/file_set_presenter.rb

In other words a Hyrax::Resource (which is a Valkyrie::Resource)
and those that use Hyrax::WorkBehavior (e.g., GenericWork) will
each have the necessary log_events method. Which is the most common pattern of the event_log and it's progeny.

With that information, I am comfortable closing this issue.

@jeremyf jeremyf closed this as completed May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jeremyf jeremyf

Labels
wings
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixing logic around deleting nested membership samvera/hyrax
2 participants
@jeremyf @randalldfloyd