Problem/Motivation
There's a @todo comment in /src/Client/AcquiaDamClient.php to consider making the expand parameters in the getAsset method configurable. This is a great idea! Can we do that?
try {
// @todo make this configurable, maybe?
$expand = 'asset_properties,embeds,file_properties,metadata,metadata_info,metadata_vocabulary,security,thumbnails';Proposed resolution
Add configuration to /admin/config/acquia-dam to allow users to select parameters. I'm not sure if I have the right wording, but something like:
Select assets to include in response.
Checkboxes with options: asset_properties,embeds,file_properties,metadata,metadata_info,metadata_vocabulary,security,thumbnails.
Remaining tasks
Question: Should the list of options be hardcoded to the current options? Or could/should they be dynamically generated?
User interface changes
Update Acquia Dam admin config page with checkboxes to select which parameters should be included in the getAsset response.
Issue fork acquia_dam-3392969
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
japerryBump for jira
Comment #3
baluertlThe case is that there are no or only very few benefits to making it configurable. This GET parameter (docs) controls how much detailed data the API should include in its response regarding the given asset. As the module utilizes its own HTTP Client responsible for sending
allmost of the external requests, and each API endpoint has a dedicated method to call. Nowhere else within the entire codebase of the module does thisexpandGET parameter have an effect. Therefore, I tend to suggest rather simply deleting these@todocomments and leaving thisexpandparameter the most optimized for each endpoint as possible.Comment #7
rajeshreeputraI've implemented the solution suggested by @baluertl in comment #3. After reviewing the codebase, making the expand parameters configurable would provide minimal benefit since:
getAsset()methods where the detailed data is actually needed.The changes in MR !181 include:
This keeps the expand parameters optimized for get asset data endpoint while cleaning up the outdated todo comments. The current implementation is already efficient and doesn't require user configuration.
Comment #8
rajeshreeputraRequesting review.
Comment #9
amangrover90 commentedComment #11
amangrover90 commented