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

Support migrating subscription link extension #4788

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

AniTumany
Copy link
Contributor

WHY are these changes introduced?

Closes: https://github.com/Shopify/subscriptions-app-remix/issues/2062
Rebased off of: #4758
Should be merged after: #4787

WHAT is this pull request doing?

Supports migrating dashboard-managed subscription link extensions to become CLI managed as part of the dev and deploy commands.

How to test your changes?

Follow the steps described in #4787 to import subscription link extensions to CLI. Once imported run shopify app dev or shopify app deploy to migrate the extension to be CLI managed.

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

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

Copy link
Contributor

github-actions bot commented Nov 1, 2024

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

  • Some seemingly private modules might be re-exported through public modules.
  • If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/themes/api.d.ts
@@ -5,7 +5,7 @@ export type AssetParams = Pick<ThemeAsset, 'key'> & Partial<Pick<ThemeAsset, 'va
 export declare function fetchTheme(id: number, session: AdminSession): Promise<Theme | undefined>;
 export declare function fetchThemes(session: AdminSession): Promise<Theme[]>;
 export declare function createTheme(params: ThemeParams, session: AdminSession): Promise<Theme | undefined>;
-export declare function fetchThemeAssets(id: number, filenames: Key[], session: AdminSession): Promise<ThemeAsset[]>;
+export declare function fetchThemeAsset(id: number, key: Key, session: AdminSession): Promise<ThemeAsset | undefined>;
 export declare function deleteThemeAsset(id: number, key: Key, session: AdminSession): Promise<boolean>;
 export declare function bulkUploadThemeAssets(id: number, assets: AssetParams[], session: AdminSession): Promise<Result[]>;
 export declare function fetchChecksums(id: number, session: AdminSession): Promise<Checksum[]>;

Copy link
Contributor

github-actions bot commented Nov 1, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
71.76% (-0.29% 🔻)
8445/11768
🟡 Branches
68.24% (-0.25% 🔻)
4093/5998
🟡 Functions
71.31% (-0.02% 🔻)
2217/3109
🟡 Lines
72.2% (-0.23% 🔻)
7985/11059
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🔴
... / clear.ts
0% 100% 0% 0%
🔴
... / generate.ts
0% 100% 0% 0%
🔴
... / list.ts
0% 100% 0% 0%
🔴
... / notifications.ts
0% 0% 0% 0%
🟢
... / global-context.ts
100% 100% 100% 100%
🟡
... / notifications-system.ts
61.29% 60.32% 84.21% 68.83%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / app.ts
85.92% (-0.22% 🔻)
70.37% (+0.24% 🔼)
91.67% (+0.18% 🔼)
87.8% (+0.3% 🔼)
🟢
... / identifiers-extensions.ts
79.49% (-5.27% 🔻)
71.43% (-3.15% 🔻)
100%
80.95% (-5.22% 🔻)
🟢
... / conf-store.ts
100%
90.48% (-2.21% 🔻)
100% 100%
🟢
... / ConcurrentOutput.tsx
98.39% (-1.61% 🔻)
90.91% (-4.55% 🔻)
100%
98.33% (-1.67% 🔻)
🟢
... / base-command.ts
84.81% (+0.39% 🔼)
80% (-1.25% 🔻)
77.27%
85.51% (+0.43% 🔼)
🔴
... / cli.ts
1.39% (-0.02% 🔻)
0% 0%
1.54% (-0.02% 🔻)

Test suite run success

1914 tests passing in 870 suites.

Report generated by 🧪jest coverage report action from 574019d

Base automatically changed from unify-extension-migration-code to main November 4, 2024 14:20
@AniTumany AniTumany marked this pull request as ready for review November 7, 2024 21:50
@AniTumany AniTumany requested a review from a team as a code owner November 7, 2024 21:50
@AniTumany AniTumany force-pushed the migrate-subscription-links-extensions branch from 8a4f30d to bbd6984 Compare November 7, 2024 21:53
Copy link
Contributor

github-actions bot commented Nov 7, 2024

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.

@AniTumany AniTumany force-pushed the migrate-subscription-links-extensions branch from bbd6984 to 574019d Compare November 8, 2024 17:10
@AniTumany AniTumany added this pull request to the merge queue Nov 8, 2024
Merged via the queue into main with commit a22a8cf Nov 8, 2024
27 checks passed
@AniTumany AniTumany deleted the migrate-subscription-links-extensions branch November 8, 2024 17:23
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