media_acquiadam: Client->getAssetsByCategory() should use exact search

MR test failure is unrelated to the MR: "Drupal core development requires Composer 2.3.6, but Composer 2.2.7 is installed. Please run 'composer self-update'."

This would be a breaking change and needs to be guarded behind config

Good call with grandfathering existing sites into the existing behavior. Test coverage is good (and very efficient).

There are a couple of issues I'd like to raise, though:

Terminology

Widen refers to "phrase search" and "exact search". See https://community.widen.com/collective/s/article/How-do-I-search-for-ass...

Phrase search
Phrase searches match search results to a portion of text within a field as long as all words of the entire phrase entered occur in sequence. If search terms are enclosed in double quotations, only assets that include the exact phrase will be returned. For example, a search for “phone cord” will yield results with “the phone cord” but not “phone with a cord.”

Exact search
If a search is surrounded by brackets ({ }), the entire contents of a field must match the search query. When search terms are entered between the brackets, the exact contents of the search will be returned. For example, searching for {quick_fox.eps} will not return an asset titled quick fox.eps, but it will return an asset titled quick_fox.eps.

There isn't a concept of "strict search". Can we adopt the Widen terminology for consistency?

API-friendly Implementation

The method we're altering here, ->getAssetsByCategory(), is a public method on a service. Anyone can call it. If we're going to support both phrase search and exact search, shouldn't the search behavior be determined at the method level?

  /**
   * Get a list of Assets given a Category ID.
   *
   * @param string $category_name
   *   Category name.
   * @param array $params
   *   Additional query parameters for the request.
+  * @param string $search_type
+  *   Search type: either 'phrase' or 'exact'.
   *
   * @return array
   *   Contains the following keys:
   *     - total_count: The total number of assets in the result set across all
   *       pages.
   *     - assets: an array of Asset objects.
   *
   * @throws \GuzzleHttp\Exception\GuzzleException
   */
-  public function getAssetsByCategory(string $category_name, array $params = []): array {
+  public function getAssetsByCategory(string $category_name, array $params = [], $search_type = 'phrase'): array {
    if ($category_name) {
      $params['query'] = 'category:' . $category_name;
-      if ($this->config->get('strict_category_search')) {
+      if ($search_type == 'exact') {
        $params['query'] = 'category:({' . $category_name . '})';
      }
-      else {
-      elseif ($search_type == 'phrase') {
        $params['query'] = 'category:' . $category_name;
      }
    }

\Drupal\media_acquiadam\Plugin\EntityBrowser\Widget\Acquiadam (the only place where this module calls the method in question) below:

  /**
   * {@inheritdoc}
   */
  public function getForm(array &$original_form, FormStateInterface $form_state, array $additional_widget_parameters) {

  ....

      if ($offset > $total_category) {
        $params['limit'] = $num_per_page;
      }
      if (count($current_category->parts) > 0) {
        $category_name = implode('/', $current_category->parts);
      }
+     $search_type = $this->config->get('category_search_type');
+     $category_assets = $this->acquiadam->getAssetsByCategory($category_name, $params, $search_type);
-     $category_assets = $this->acquiadam->getAssetsByCategory($category_name, $params);
      if ($total_category == 0 || $total_category <= $offset || $total_category < $num_per_page) {
        $items = $category_assets['assets'] ?? [];
      }
      // Total asset conatins both asset and subcategory(if any).
      $total_asset += $category_assets['total_count'] ?? 0;
    }

The change contemplated above would also require a hook_update_N() function be added to the .install file since the 'category_search_type' config would be a string and not a boolean.

Sure, we can make it a parameter that is called and the caller checks the boolean.

The change contemplated above would also require a hook_update_N() function be added to the .install file since the 'category_search_type' config would be a string and not a boolean.

These never work with config as folks upgrade and never re-export config. Regardless of adding a hook update, we'd have to assume null values for any existing installation. We could use a string value instead of boolean, but it is currently a boolean op: phrase or exact.

Fixed wording, move to parameter and the caller in the Entity Browser Widget checks config.

Merged! Thanks for feedback @Chris Burge

mglaman → closed merge request !12

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