Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add import admin link extensions to CLI and migration on deployment #4738

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alfonso-noriega
Copy link
Contributor

@alfonso-noriega alfonso-noriega commented Oct 24, 2024

NOTE: Do not merge until BE work lands

WHY are these changes introduced?

With the removal of the Partners Dashboard it is needed to migrate all dashboard managed extensions to be CLI managed. This PR adds support to import existing admin links (app_link and `bulk_action) and the call to the appModules migration endpoint on their deployment to update them to a CLI managed specification.

How to test your changes?

  • create a new app or reuse an existing one
  • create a new app link extension on partners for the app
  • in the CLI repository checkout this branch and run pnpm shopify app import-extensions --path path/to/your/app
  • select admin links and check that in the extensions folder of your app there is a new extension with the toml file matching this structure and fields:
[[extensions]]
type = "admin_link"
name = "bulk action name 1"
handle = "bulk-action-name-1"

  [[extensions.targeting]]
  text = "bulk action label"
  url = "http://example.org"
  target = "admin.order.selection.action"

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@alfonso-noriega alfonso-noriega marked this pull request as ready for review October 24, 2024 13:38
@alfonso-noriega alfonso-noriega requested a review from a team as a code owner October 24, 2024 13:38
Copy link
Contributor

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run "pnpm changeset add" to track your changes and include them in the next release CHANGELOG.

Copy link
Contributor

github-actions bot commented Oct 24, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
71.88% (+0.1% 🔼)
8483/11802
🟡 Branches
68.32% (+0.1% 🔼)
4119/6029
🟡 Functions
71.44% (+0.13% 🔼)
2224/3113
🟡 Lines
72.34% (+0.1% 🔼)
8022/11090
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / extension-to-toml.ts
77.78% 75% 100% 100%
🟢
... / utils.ts
84.21% 66.67% 100% 83.33%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟡
... / identifiers-extensions.ts
76.61% (-2.87% 🔻)
68.66% (-2.77% 🔻)
100%
78.38% (-2.57% 🔻)
🟡
... / app-event-watcher.ts
69.7% (-0.45% 🔻)
45.83%
73.68% (-1.32% 🔻)
75.86% (-0.41% 🔻)
🔴
... / dev-session.ts
4.17% (-0.33% 🔻)
0%
6.67% (+1.4% 🔼)
4.62% (-0.51% 🔻)

Test suite run success

1921 tests passing in 874 suites.

Report generated by 🧪jest coverage report action from aa75f67

Comment on lines 53 to 58
{
label: 'Admin Link extensions',
value: 'link extension',
extensionTypes: ['app_link', 'bulk_action'],
buildTomlObject: buildAdminLinkTomlObject,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only for shopifolk? is that intended?

@@ -45,6 +46,11 @@ export async function ensureExtensionsIds(
dashboardOnlyExtensions,
validIdentifiers,
)
const adminLinkExtensionsToMigrate = getAdminLinkExtensionsToMigrate(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that all get**ExtensionsToMigrate functions do mostly the same, do you want to unify that in a generic function? :)
We can leave it for a future PR if not

@alfonso-noriega alfonso-noriega force-pushed the 1680-cli-import-link-extension branch 4 times, most recently from a9d372c to 9848bad Compare October 25, 2024 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants