Add an "input args" parameter when defining derivative actions.

[jasonhildebrand]

• Other Relevant Links:

Related PRs:

• Derivative connector: accept an inputargs property and pass it throug… Alpaca#91
• Extend houdini to accept a new header X-Islandora-Input-Args which ca… Crayfish#172

What does this Pull Request do?

This PR adds an "input args" parameter in derivative actions, to be used to pass additional arguments when generating image derivatives. This fields is only exposed in the UI for image derivatives (not for other derivative types), as images are the only use case which needs this additional field.

Background/Motivation

I wanted to use the "density" parameter in imagemagick, but this parameter only works if applied BEFORE the input stream. According to the man page (https://manpages.ubuntu.com/manpages/trusty/man1/convert.im6.1.html) there are several options which are only valid as so-called "input options". The addition of this field makes it possible to use these parameters to adjust the conversion. In particular, the density parameter makes it possible to generate higher-resolution JPGs from a PDF source file.

What's new?

One field is added to the "Edit action" page for image derivatives.

The field should not show for other derivative types (I think I got them all - I followed existing code to remove the field from their configuration forms).

Until the related Alpaca PR is merged, the addition of this field will cause alpaca to spew errors (about the new unknown field).

I think this means some kind of version constraint will need to be applied, but I don't know exactly what/how.

How should this be tested?

It is easier to test these PRs together. See in-depth test plan in Islandora/Alpaca#91

Documentation Status

• I have added some inline documentation into the Action configuration page (links to imagemagick documentation).

Interested parties

Tag (@ mention) interested parties or, if unsure, @Islandora/committers

[adam-vessey]

Instead of introducing this at the AbstractGenerateDerivative level, seemingly only to remove it from all but one extending class, would it make more sense to only implement in the one class? The AbstractGenerateDerivative class could be extended by other modules in the wild, which would then start showing this new option in contexts where it is not desirable.

Also, the addition of new plugin config should be accompanied by an update to the config schema; If this was just hitting the generate_image_derivative bit, then: https://github.com/Islandora/islandora/blob/2.x/modules/islandora_image/config/schema/islandora_image.schema.yml

[jasonhildebrand]

That's a good suggestion, I will make that change. I'll update the config schema, too, thanks for letting me know. peace, Jason

…

________________________________ From: Adam ***@***.***> Sent: Friday, October 21, 2022 8:20:13 AM To: Islandora/islandora ***@***.***> Cc: Jason Hildebrand ***@***.***>; Author ***@***.***> Subject: Re: [Islandora/islandora] Add an "input args" parameter when defining derivative actions. (PR #904) Instead of introducing this at the AbstractGenerateDerivative level, seemingly only to remove it from all but one extending class, would it make more sense to only implement in the one class? The AbstractGenerateDerivative class could be extended by other modules in the wild, which would then start showing this new option in contexts where it is not desirable. Also, the addition of new plugin config should be accompanied by an update to the config schema; If this was just hitting the generate_image_derivative bit, then: https://github.com/Islandora/islandora/blob/2.x/modules/islandora_image/config/schema/islandora_image.schema.yml — Reply to this email directly, view it on GitHub<#904 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABYME3ULOJ3PXIEUIJ74VTDWEKKA3ANCNFSM6AAAAAARKS3CNU>. You are receiving this because you authored the thread.Message ID: ***@***.***>

[jasonhildebrand]

I have made the suggested changes, including updating the config schema.

[rosiel]

This will be a breaking change as it changes Islandora's "API" (not really defined, but here used to mean "the way it communicates with other systems"). I'd like to include this in the first islandora 3.x release.

[jasonhildebrand]

I resolved the merge conflicts.

[jasonhildebrand]

I installed phpcs locally, but when I ran it, it produced 600+ errors on IslandoraUtils.php (an order of magnitude more than the CI job showed). Not sure what to do.

[adam-vessey]

RE: coding standards, our CI runs: https://github.com/Islandora/islandora_ci/blob/859a92805dd8cb51646ce30cfc12f30452dbff5c/travis_scripts.sh#L16

Specifically, with the Drupal coding standards.

Additional details about installing the Drupal coding standards might be gleaned from that repo, or it might be more useful to reference Drupal's docs directly: https://www.drupal.org/docs/extending-drupal/contributed-modules/contributed-module-documentation/coder/installing-coder

[jasonhildebrand]

RE: coding standards, our CI runs: https://github.com/Islandora/islandora_ci/blob/859a92805dd8cb51646ce30cfc12f30452dbff5c/travis_scripts.sh#L16

Thanks, this is useful.

[jasonhildebrand]

I've pushed a partial de-lint to run these and see if two of the files are now clean - more coming.

I've completed the de-linting, all clean now.