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

Deploy fetch remote specs #3180

Merged
merged 6 commits into from
Dec 22, 2023
Merged

Deploy fetch remote specs #3180

merged 6 commits into from
Dec 22, 2023

Conversation

alvaro-shopify
Copy link
Contributor

@alvaro-shopify alvaro-shopify commented Dec 7, 2023

WHY are these changes introduced?

Follow-up: #3170
Follow-up: #3214

WHAT is this pull request doing?

  • Make specifications optional for the App loader. Set default value to an empty array so no extensions are loaded in this case.
  • deploy command loads the app again with the extensions once the remote app is selected, thus, the gated specs are applied and in case they are not allowed the extensions are not loaded
  • Temporal app access config module added. Supports the new section access
    • direct_api_offline_access true | false to enable Online access for Direct API

How to test your changes?

  • Create a new cli app and run the app config link command to link it to a new remote app.

  • With the org beta flag App Access Module disabled

    • Add the section access with whatever value
    • Run the deploy command
    • Run the link command. The added access section content will be removed as the app module wasn't uploaded in the previous deploy
  • With the org beta flag App Access Module enabled

    • Add the section access with whatever value
    • Run the deploy command
    • Modify the value inside the access section
    • Run the link command. The access section content will be restored to the content it had before running the deploy command as it was uploaded with it

    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
    • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

Copy link
Contributor

github-actions bot commented Dec 7, 2023

We detected some changes at either packages/*/src or packages/cli-kit/assets/cli-ruby/** 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.

@alvaro-shopify alvaro-shopify mentioned this pull request Dec 7, 2023
6 tasks
Copy link
Contributor

github-actions bot commented Dec 7, 2023

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
73.54% (+0.02% 🔼)
6413/8721
🟡 Branches
70.81% (-0.02% 🔻)
3120/4406
🟡 Functions 72.53% 1637/2257
🟡 Lines
74.67% (+0.03% 🔼)
6074/8135
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / app_config_app_access.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / loader.ts
92.58% (-0.3% 🔻)
85.07% (-1.08% 🔻)
95%
93.92% (+0.02% 🔼)

Test suite run success

1513 tests passing in 696 suites.

Report generated by 🧪jest coverage report action from 8cf1988

@alvaro-shopify alvaro-shopify force-pushed the deploy-fetch-remote-specs branch 2 times, most recently from 3c2a7de to 79d3de5 Compare December 8, 2023 22:22
@alvaro-shopify alvaro-shopify force-pushed the deploy-fetch-remote-specs branch 2 times, most recently from 0d1451c to a233750 Compare December 11, 2023 17:30
Copy link
Contributor

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/common/object.d.ts
@@ -45,4 +45,21 @@ export declare function deepCompare(one: object, two: object): boolean;
  * @param two - The second object to be compared.
  * @returns Two objects containing the fields that are different, each one with the values of one object.
  */
-export declare function deepDifference(one: object, two: object): [object, object];
\ No newline at end of file
+export declare function deepDifference(one: object, two: object): [object, object];
+/**
+ * Gets the value at path of object. If the resolved value is undefined, the defaultValue is returned in its place.
+ *
+ * @param object - The object to query.
+ * @param path - The path of the property to get.
+ * @returns - Returns the resolved value.
+ */
+export declare function getPathValue<T = object>(object: object, path: string): T | undefined;
+/**
+ * Sets the value at path of object. If a portion of path doesn't exist, it's create.
+ *
+ * @param object - The object to modify.
+ * @param path - The path of the property to set.
+ * @param value - The value to set.
+ * @returns - Returns object.
+ */
+export declare function setPathValue(object: object, path: string, value?: unknown): object;
\ No newline at end of file
packages/cli-kit/dist/public/common/version.d.ts
@@ -1 +1 @@
-export declare const CLI_KIT_VERSION = "3.52.0";
\ No newline at end of file
+export declare const CLI_KIT_VERSION = "3.51.0";
\ No newline at end of file

Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

Code looks good and it works as expected 👌

@alvaro-shopify alvaro-shopify force-pushed the add-remaining-app-config-specs branch 2 times, most recently from e431a1f to ce3f4f5 Compare December 20, 2023 15:53
Base automatically changed from add-remaining-app-config-specs to main December 21, 2023 12:46
@alvaro-shopify alvaro-shopify force-pushed the deploy-fetch-remote-specs branch 3 times, most recently from a4f5b44 to ba041c2 Compare December 21, 2023 17:27
@alvaro-shopify alvaro-shopify added this pull request to the merge queue Dec 22, 2023
Merged via the queue into main with commit 84f83d1 Dec 22, 2023
@alvaro-shopify alvaro-shopify deleted the deploy-fetch-remote-specs branch December 22, 2023 17:02
@pt2pham pt2pham added the #gsd:36684 Versioned app configurations project (https://vault.shopify.io/gsd/projects/36684) label Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
#gsd:36684 Versioned app configurations project (https://vault.shopify.io/gsd/projects/36684)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants