Conversation
The bulk Publish action in the browse view's action bar had no
awareness of the editor.hidePublish org/site config, unlike the
edit view's da-title.js. Also switches editor.hidePublish/editor.path
reads to look up the config sheet by name ("data") first, falling
back to positional lookup, since getFirstSheet silently picks the
wrong sheet when the data rows aren't the first key in a multi-sheet
config.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tionbar Co-Authored-By: Claude <noreply@anthropic.com>
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch.
|
shsteimer
changed the title
| const configs = await Promise.all(fetchDaConfigs({ org, site })); | ||
| const rows = configs.filter(Boolean).reverse().flatMap((c) => getFirstSheet(c) || []); | ||
| const rows = configs.filter(Boolean).reverse() | ||
| .flatMap((c) => getSheetByName(c, 'data') ?? getFirstSheet(c) ?? []); |
There was a problem hiding this comment.
looks for data sheet first, falls back to first sheet if it doesn't exist. This was added because when testing against /aem-sandbox/block-collection there were already many config sheets, but no data sheet to add this to. so had to create the sheet and couldn't re-order.
Open to feedback if any issues anticipated with this change.
There was a problem hiding this comment.
data It's pretty generic, but it's also there usually as the first one or the only one. For other configs flags we started using flags sheet. Maybe we check for this.
There was a problem hiding this comment.
yea, mixed feelings on this. I needed it to test the feature because on http://localhost:3000/config#/aem-sandbox/block-collection/ there was no data sheet yet, but there were other sheets, and sheet editor doesn't allow re-ordering. The first sheet was actually the library sheet. Perhaps we revert this change, update sheets editor to allow sheet re-ordering, and (medium term) consider which configs should migrate to flags?
There was a problem hiding this comment.
data sheet becomes the new /etc/...
There was a problem hiding this comment.
I decided to just revert this change for now. did some magic to re-order those sheet tabs so I could verify.
I know this was previously approved, but needs a new approve so I can merge with new changes (both here and da-nx pr, adobe/da-nx#666)
| const configs = await Promise.all(fetchDaConfigs({ org, site })); | ||
| const configTab = configs.flatMap((config) => getFirstSheet(config) || []); | ||
| const configTab = configs.filter(Boolean) | ||
| .flatMap((config) => getSheetByName(config, 'data') ?? getFirstSheet(config) ?? []); |
There was a problem hiding this comment.
same here as above in terms of sheet resolution
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # blocks/browse/da-actionbar/da-actionbar.js
mockSiteConfig returned a headers-less plain object for every request, including the HLX_ADMIN ping isHlx6() makes before the config fetch; its headers.get() call then threw. mockConfig already special-cased this ping URL — apply the same fix to mockSiteConfig. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
# Conflicts: # blocks/edit/da-title/da-title.js
…Publish Reverts the getSheetByName fallback in da-browse.js/da-title.js back to plain getFirstSheet; better multi-sheet config lookup will be tackled separately. The editor.hidePublish feature itself is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
createConnection opened a real y-websocket connection to admin.da.live, letting the real socket's own close race the test's synthetic connection-close emit and double-count refreshToken calls — flaky on CI. Stub WebSocket like the rapid-reconnect describe block already does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4 participants
Description
editor.hidePublish should work everywhere.
Related Issue
adobe/da-nx#664
should merge with adobe/da-nx#666
Motivation and Context
working with a customer who wants to globally disable publish and force everything through request publish. If we are gonna have the setting, should work everywhere. Eventually, in hlx6, we should probably deprecate the setting and rely on permissions to hide or disable the button.
How Has This Been Tested?
ran locally with aem sandbox.
With the related da-nx pr it hides for edit, canvas, and browse.
Screenshots (if appropriate):
Types of changes
Checklist: