Problem/Motivation

Currently, the DAM configuration form combines both authentication settings and image style configurations in a single form. As a result, any time a user updates image style-related configurations, the authentication settings are also resaved. This coupling can potentially lead to accidental changes or unnecessary authentication updates when only image styles are being modified.

Proposed Resolution

We should refactor the configuration interface to clearly separate DAM authentication settings from image style configurations. By creating dedicated forms for each, we can:

  • Reduce the risk of unintentional changes to authentication details,
  • Improve the user experience by making configuration sections more focused,
  • Make maintenance, troubleshooting easier.

Steps to Reproduce

  1. Go to the current DAM configuration form.
  2. Notice that both authentication and image style configurations are present together.
  3. Update any image style-related setting and save.
  4. Observe that authentication settings are also rescanned and saved.

Acceptance Criteria

  • Authentication and image style configurations have separate forms/pages.
  • Updating image style settings does not trigger a resave of authentication settings, and vice versa.
  • Documentation/update notes reflect this segregation for end users.

This change will ensure clearer separation of concerns, reduce risk, and improve the overall configuration workflow.

CommentFileSizeAuthor
#5 dam-config-form.png262.66 KBrajeshreeputra
#5 dam-current-config-form-duplicate-buttons.png338 KBrajeshreeputra
#5 dam-proposed-config-form.png349.03 KBrajeshreeputra
#5 dam-updated-config-form-with-disabled-field.png276.54 KBrajeshreeputra

Issue fork acquia_dam-3549173

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:

About issue forks

Comments

rajeshreeputra created an issue. See original summary.

rajeshreeputra opened merge request !201

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Status: Active » Needs review

Requesting review.

japerry’s picture

japerry
Location KOMK
commented
Title: Separate DAM Authentication Settings from Image Styles in Configuration Form » Improvements to the DAM configuration page
Status: Needs review » Needs work

I changed the title to reflect a few more UI changes that should occur here:

1) The image styles menu should be dependent on being connected to the DAM. If you're not authenticated, don't show the image styles tab.
2) The 'authenticate site' and 'save dam configuration' buttons seem redundant. With the image styles moved, the main page should be for authentication details only, with 'Authenticate Site' and 'Disconnect Site' being the two buttons options (depending on the state of the site)
3) While authentication type is greyed out, domain is not. We have some logic that disconnects the site.. but instead we should just grey all of this out and require someone to 'disconnect' before changing any of the credential details.

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
StatusFileSize
new dam-updated-config-form-with-disabled-field.png276.54 KB
new dam-proposed-config-form.png349.03 KB
new dam-current-config-form-duplicate-buttons.png338 KB
new dam-config-form.png262.66 KB

Updated MR with following:

  1. Added requirement check in routing for point 1
    Image style tab not visible when no DAM configured
  2. Updated config form to incorporate feedback for point 2 and 3.
    dam authentication form button label changes

After disconnecting a site, the authentication button appears twice. I believe it would be more effective not to display the Site Authentication button in first fieldset.
current:
duplicate site authentication buttons
Proposed:
single site authentication button

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Status: Needs work » Needs review

Updated MR with proposed solution, requesting review.

japerry’s picture

japerry
Location KOMK
commented
Status: Needs review » Reviewed & tested by the community

Exactly what I was looking for! thanks @rajeshreeputra! Will rebase and run tests, assuming they're good, I'll get it merged.

japerry’s picture

japerry
Location KOMK
commented
Status: Reviewed & tested by the community » Needs work

There are some more tests that need updating, moving to NW.

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Status: Needs work » Needs review

Updated tests and it's green now, Hence moving ahead!

japerry’s picture

japerry
Location KOMK
commented
Status: Needs review » Fixed

Looks great! Merged

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.

japerry’s picture

japerry
Location KOMK
commented
Status: Fixed » Needs work

While not specifically caused by this issue, it is easier to see in this issue -- if you put a url schema for the domain in the form, it says the domain passes validation, but redirects improperly. Rolling a fix momentarily to fix that.

japerry opened merge request !213

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Status: Needs work » Needs review

Updated service to remove protocol or trailing slashes from domains while using key module, requesting review.

  • japerry committed 0cf61ac7 on 1.1.x 
    Followup #3549173 "Fix domain protocol sanitization on configuration...
japerry’s picture

japerry
Location KOMK
commented
Status: Needs review » Fixed

Yah, thats probably a good check. Merged!

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.