CEXT-6318: Create a separate package for Admin UI SDK#489
Conversation
🦋 Changeset detectedLatest commit: 6cc7292 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
obarcelonap
left a comment
There was a problem hiding this comment.
Overall LGTM, please review the publication config in package.json and the snake case in registering extension payload.
| `adminuisdk/extension/${workspaceName}/${extensionName}`, | ||
| fetchOptions, | ||
| ) | ||
| .then((_res) => { |
There was a problem hiding this comment.
You can just await the deletion without this then block
await httpClient.delete(
`adminuisdk/extension/${workspaceName}/${extensionName}`,
fetchOptions,
);
There was a problem hiding this comment.
@obarcelonap It depends, actually. For example, we did this with the event subscriptions endpoint, and for some reason, it always returned [] (not sure if it was a Commerce API thing specific to that endpoint). That's why I suggested to him to use .then without a return value to force void and ensure it even if the API changes.
There was a problem hiding this comment.
I proposed to await without returning so you still get Promise<Void> as return type. In any case not a blocker.
There was a problem hiding this comment.
I see, I didn't understand. Works both ways, I don't have a strong opinion.
- remove SDK from AdminUiSDK
- code review
So I just remembered that @capape explained a related issue to me just yesterday, and asked him what the reason might be. Turns out Commerce converts everything to @capape Correct me if I understood wrong 🙈 |
Description
This PR is a part of #426. As ACL will need a rework based on the new structure, the new package for the admin ui can be delivered separately.
https://jira.corp.adobe.com/browse/CEXT-6318
Motivation and Context
To align with other packages for eventing/webhook, the admin ui API should also be extracted to a separate package.
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: