🇮🇳India @vishalkhode

Looks good to me.

vishalkhode → made their first commit to this issue’s fork.

Reviewed changes, looks good to me.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

@ankitv18: Backward compatibility is already taken care of. Please check acquia_cms_audio.module and acquia_cms_audio.install file.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

Looks good to me. Merged.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → created an issue.

Re-opening this back. As we'll complete the development and merge as part of this ticket.

vishalkhode → created an issue.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

Reviewed changes, looks good to me now. Hence, RTBC.

Reviewed changes, looks good to me now. Hence, RTBC.

Reviewed changes, looks good to me now. Hence, RTBC.

Reviewed changes, looks good to me. Hence, marking RTBC.

Looks good to me. Merged.

vishalkhode → made their first commit to this issue’s fork.

Reviewed changes, looks good to me. Hence, RTBC.

@japerry I think removing category make more sense here because as per the Drupal, the category can be defined to ensure that the field is appropriately listed under the specified category when created via the UI. However, since our field is configured not to display in the UI, so we can safely remove this option.

@Mikeedfw I was able to replicate the issue on both Acquia DAM → : 1.0.14 → and Acquia DAM → : 1.1.3 → . This appears to be a Drupal Core issue and occurs under the following conditions:

1. Go to the Text formats and editors page and click the Configure button for the text filter you are using. Ex: Full Html. Ex:
2. Scroll to the bottom of the page, and under Filter Settings => Embed media, check the value for the Default View mode option. The value could be something other than "Default" (e.g., "Full Content").
3. Next, go to the Manage Display page for the DAM Image media type (e.g., /admin/structure/media/manage/acquia_dam_image_asset/display) and check the Custom display settings section. The configured default view mode value (e.g., "Full Content") will be unchecked. Ex:

So, basically the issue occurs when the Default View mode is set to a specific value (e.g., "Full Content") in the Embed media filter settings, and that view mode is not enabled for the media type under Custom display settings.

The WSOD error is caused by the LottieFiles Field → module, not due the Acquia DAM module. The acquia_dam_asset field type has no_ui = TRUE, meaning it cannot be created through the UI. Therefore, it is unlikely for this error to occur when creating new field via the UI.

@cb_govcms: I agree that PHP 7 and Drupal 9 are EOL and no longer supported. We could merge this and release a new minor version supporting PHP 8.1 and Drupal 10.3 (and above), while using the 4.0.x branch to maintain backward compatibility with older PHP and Drupal versions.

vishalkhode → changed the visibility of the branch 3236423-4.0.x-do-not-require-unique-characters to hidden.

Update MR !70 and reverted changes from @COBadger. I think we shouldn't create a new password policy constraint for zero password validation as this should be treated normally because user can enter other single digit numeric / string values like 1 or 2 or 3 or a or b etc. The policy validator should be more of generic validations. I've added strlen condition to fix this issue and added PHPUnit tests as well.

vishalkhode → made their first commit to this issue’s fork.

It seems your configurations do not include password_policy under dependencies.enforced.module, which ensures these configurations are removed when the module is uninstalled.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

Reviewed changes, looks good to me. Hence, RTBC.

Reviewed changes, looks good to me. Hence, RTBC.

vishalkhode → changed the visibility of the branch 3519247-approach-1 to hidden.

vishalkhode → made their first commit to this issue’s fork.

Reviewed changes, looks good to me. Hence, RTBC.

Not able to reproduce issue as well. Verified that getMapping() method is called multiple times, but, due to the condition i.e if (!$this->mapping) {, it only executes once. Please add steps to reproduce and re-open, if we still see this issue. Hence, closing.

Reviewed changes, overall looks good to me. A PHUnit test covering above scenario would be good or we should handle separately ? As we can also then remove the $reset parameter from getKeyValue() and getKeyValues() methods as cache is now being updated / deleted in CRUD operations and ensuring it always returns the correct value. Hence, RTBC.

Requested some changes in the MR. One question here, should this provider function similar like the Environment Key provider, meaning should only read values from the state rather than saving, reading and deleting value as well from the state ?

Reviewed changes, looks good to me. Hence, RTBC.
Note: Core Version requirement has been updated and Drupal Core 8 support is dropped. Probably this requires a new minor release.

vishalkhode → created an issue.

In !MR 6 Looks like there's a complete new feature has been introduced i.e Provide support for external file url for key. I'm not sure if this should be supported or not. In any case, this should be implemented and discussed separately. We should limit this ticket to fix the bug only.

I've to verified and not seeing any error with 1.1.x. Looks like this is already fixed & merged as part of 🐛 TypeError on EmbedCodeUrlBuilder on new revision update for an existing asset/image on Acquia DAM Active . Hence, closing.

To fix the issue, I think we can do the following:

1. Update the method to also update the static cache. For example:
   

     $key_values = &drupal_static('getKeyValue', []);
     $key_values[$this->id()] = $key_value;
   

2. Update the method to remove the value from the static cache as well. For ex:
   

     $key_values = &drupal_static('getKeyValue');
     unset($key_values[$this->id()]);
   

3. Lastly, we should consider removing the $reset argument from the method and the related code, since the static cache should now always reflect the correct and updated value. Are there any edge cases where we might still need the flag ?
4. Add / Update the PHPUnit tests to cover all above scenarios.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

rajeshreeputra → credited vishalkhode → .

ankitv18 → credited vishalkhode → .

vishalkhode → created an issue.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

Thanks @rajeshreeputra for working on this. Changes looks good. Hence, merged.

vishalkhode → made their first commit to this issue’s fork.

Thanks @vipin.mittal18 & @Rajeshreeputra for reporting issues. The CI was failing earlier, I've updated the CI template and rebase the MR, though there are couple of allowed failure CI jobs and some of them are failing, we can create a separate ticket to fix this.

vishalkhode → made their first commit to this issue’s fork.

Reviewed changes, looks good to me. Hence, RTBC.

vishalkhode → created an issue.

vishalkhode → made their first commit to this issue’s fork.

vishalkhode → made their first commit to this issue’s fork.

hestenet → credited vishalkhode → .

rajeshreeputra → credited vishalkhode → .

rajeshreeputra → credited vishalkhode → .

rajeshreeputra → credited vishalkhode → .

ankitv18 → credited vishalkhode → .

ankitv18 → credited vishalkhode → .

rajeshreeputra → credited vishalkhode → .

rajeshreeputra → credited vishalkhode → .

rajeshreeputra → credited vishalkhode → .

rajeshreeputra → credited vishalkhode → .

vishalkhode → made their first commit to this issue’s fork.

rajeshreeputra → credited vishalkhode → .

rajeshreeputra → credited vishalkhode → .

ankitv18 → credited vishalkhode → .

ankitv18 → credited vishalkhode → .