-
Notifications
You must be signed in to change notification settings - Fork 218
Implement shopify app bulk status subcommand
#6667
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
Conversation
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3422 tests passing in 1390 suites. Report generated by 🧪jest coverage report action from bce54f6 |
1b17a01 to
de15a6b
Compare
c8e62d6 to
4c19852
Compare
de15a6b to
16085c8
Compare
3dc60f0 to
1220b22
Compare
1220b22 to
8e4cd3b
Compare
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
|
/snapit |
|
🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm. Test the snapshot by installing your package globally: npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/[email protected]Caution After installing, validate the version by running just |
gonzaloriestra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! The only important thing is using ensureAuthenticatedAdminAsApp.
Also, another UX suggestion: I think both execute and status should be under the same topic. For example I'd find it more clear to have:
shopify app bulk run(instead ofshopify app execute)shopify app bulk status
packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts
Outdated
Show resolved
Hide resolved
packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts
Outdated
Show resolved
Hide resolved
Yeah, this is happening, I'm just waiting for us to be out of PR stack hell before I move the existing code into the |
ac5e1e1 to
40308af
Compare
40308af to
761e22f
Compare
41d68b3 to
02b54ed
Compare
761e22f to
32308b1
Compare
32308b1 to
cc1db6c
Compare
Allows users to check the status of a bulk operation by ID.
cc1db6c to
bce54f6
Compare
Differences in type declarationsWe 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:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/common/string.d.ts@@ -120,4 +120,12 @@ export declare function pascalize(str: string): string;
* @param delimiter - Delimiter used to split the string into tokens.
* @returns String with the normalized list of tokens.
*/
-export declare function normalizeDelimitedString(delimitedString?: string, delimiter?: string): string | undefined;
\ No newline at end of file
+export declare function normalizeDelimitedString(delimitedString?: string, delimiter?: string): string | undefined;
+/**
+ * Given two dates, it returns a human-readable string representing the time elapsed between them.
+ *
+ * @param from - Start date.
+ * @param to - End date.
+ * @returns A string like "5 minutes ago" or "2 days ago".
+ */
+export declare function timeAgo(from: Date, to: Date): string;
\ No newline at end of file
|

WHY are these changes introduced?
Part 1/2 addressing https://github.com/shop/issues-api-foundations/issues/1142.
WHAT is this pull request doing?
Adds a new command that lets users check the progress of their long-running bulk operations.
How to test your changes?
Start a bulk query:
Note down the ID and pass it to the
bulk statuscommand using--id:If you pass in an ID that doesn't exist, you'll get an error: