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

Command icon 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

capysara created an issue. See original summary.

japerry’s picture

Bump for jira

baluertl’s picture

Title: Make expand parameters in getAsset configurable so users can select what information is requested » Make expand parameters configurable in the Client class

The 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 all most of the external requests, and each API endpoint has a dedicated method to call. Nowhere else within the entire codebase of the module does this expand GET parameter have an effect. Therefore, I tend to suggest rather simply deleting these @todo comments and leaving this expand parameter the most optimized for each endpoint as possible.

rajeshreeputra made their first commit to this issue’s fork.

rajeshreeputra changed the visibility of the branch 3392969-asset-type-display to hidden.

rajeshreeputra’s picture

Version: 1.0.x-dev » 1.1.x-dev
Status: Active » Needs review

I've implemented the solution suggested by @baluertl in comment #3. After reviewing the codebase, making the expand parameters configurable would provide minimal benefit since:

  1. The expand parameter is only used in getAsset() methods where the detailed data is actually needed.
  2. Each API endpoint has its own dedicated method with optimized or no parameters.
  3. Making it configurable would add unnecessary complexity without significant value.

The changes in MR !181 include:

  • Removed the @todo comment from the $expand property documentation, as keeping it hardcoded is the preferred approach.
  • Removed unnecessary expand parameter from the getAssetVersions() method call, as version endpoints don't require the same expanded data as asset detail endpoints.

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.

rajeshreeputra’s picture

Requesting review.

amangrover90’s picture

Status: Needs review » Reviewed & tested by the community

amangrover90’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.