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

manually implementing postsave #815

Merged
merged 2 commits into from
Dec 10, 2020
Merged

manually implementing postsave #815

merged 2 commits into from
Dec 10, 2020

Conversation

qadan
Copy link

@qadan qadan commented Dec 8, 2020

Related issue: Islandora/documentation#1731

What does this Pull Request do?

Removes hook_post_action as a dependency, and manually implements the functionality it was providing.

The hook_post_action module likely won't be ported to Drupal 9. The functionality it provides is also being implemented in Drupal 9 at a later date. The intention of this pull request is to provide a stopgap between the loss of support for hook_post_action in Drupal 9 and our eventual ability to recreate this functionality when it's added into core in Drupal 9.1 or later.

What's new?

After an insert operation is taken for a Media, a registered shutdown function fires off file derivative actions for that Media.

How should this be tested?

We only need to ensure that there is no regression from #756, so the test case there will suffice.

Additional Notes:

The recommendation in this PR is to wait for entity events to be implemented in D9, but also, if we did the work to implement actions as mentioned in this comment, said future work would likely be enough to delay us having to migrate completely to entity events. But I think those entity events should be considered the ideal endgame.

Interested parties

@dannylamb likely as part of the ongoing D9 sprint https://github.com/orgs/Islandora/projects/3

Copy link

@dannylamb dannylamb left a comment

Choose a reason for hiding this comment

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

I'm setting up the test case to put this through the paces. I'm also wondering if it will move this along in the Github project.

Copy link

@dannylamb dannylamb left a comment

Choose a reason for hiding this comment

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

Just ran through a basic multifile media test. Works as expected. I was able to attach FITS files to a field_fits_file field on a new media type.

I had to do some refactoring in MediaSourceService and MediaSourceController, but that's totally unrelated to this and it can come in now. I'll make a subsequent PR for what I had to do.

@dannylamb
Copy link

Oh.. Travis is complaining about a doc comment @qadan . Fix that and I'll merge.

@dannylamb dannylamb merged commit e57b9e7 into Islandora:8.x-1.x Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants