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

♻️ Extract Action Logic for improved composibility #6644

Merged
merged 3 commits into from Jan 30, 2024

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Jan 26, 2024

Prior to this commit, there was a precarity regarding the extensibility of the create action for valkyrie objects.

The create method had several interacting objects, which made downstream composibility harder to implement (without completely overriding the action's behavior). Downstream could:

  1. Provide a different container for transactions
  2. Inject additional steps into the transaction container, and those steps could require step args.
  3. Remove/replace one of the transactions that has a step arg (which would then raise an error).

In all of these cases, the size of the original method makes for harder downstream extensibility.

With this commit, I'm extracting an action object, putting forward a minimum amount of work so we can discuss the approach.

This should be considered a noop change as I'm moving where the logic happens.

If we deem that this is a good approach, I'd envision extracting some of the work controller behavior specs into the given action. I would also anticipate extracting the update and delete actions.

Prior to this commit, there was a precarity regarding the extensibility
of the create action for valkyrie objects.

The create method had several interacting objects, which made downstream
composibility harder to implement (without completely overriding the
action's behavior).  Downstream could:

1. Provide a different container for transactions
2. Inject additional steps into the transaction container, and those
   steps could require step args.
3. Remove/replace one of the transactions that has a step arg (which
   would then raise an error).

In all of these cases, the size of the original method makes for harder
downstream extensibility.

With this commit, I'm extracting an action object, putting forward a
minimum amount of work so we can discuss the approach.

This should be considered a noop change as I'm moving where the logic
happens.

If we deem that this is a good approach, I'd envision extracting some of
the work controller behavior specs into the given action.  I would also
anticipate extracting the update and delete actions.
@jeremyf jeremyf force-pushed the create-refactor-for-valkyrie-create-work-action branch from b2382b1 to da6a221 Compare January 26, 2024 19:01
Copy link
Contributor

@dlpierce dlpierce left a comment

Choose a reason for hiding this comment

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

I like this! It simplifies the controller and makes modification of the work creation process more manageable. Please, continue.

@dlpierce dlpierce merged commit 206cb5f into main Jan 30, 2024
6 checks passed
@dlpierce dlpierce deleted the create-refactor-for-valkyrie-create-work-action branch January 30, 2024 19:59
jeremyf added a commit that referenced this pull request Jan 30, 2024
* main:
  ♻️ Extract Action Logic for improved composibility (#6644)
  Prevent Wings from loading when disabled
  use rspec filter to exclude :active_fedora tags
  configure Solr for koppie
  ci: add koppie build and test
@dlpierce dlpierce added the notes-minor Release Notes: Non-breaking features label Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-minor Release Notes: Non-breaking features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants