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

Filter orgs to only those with develop_apps permission #5036

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

amcaplan
Copy link
Contributor

@amcaplan amcaplan commented Dec 5, 2024

WHY are these changes introduced?

Depends on https://github.com/Shopify/business-platform/pull/23112

Needed for https://github.com/Shopify/develop-app-inner-loop/issues/2343

Fixes https://github.com/Shopify/develop-app-inner-loop/issues/2283

Dependency of #5004

To prevent users from seeing a long list of orgs they can't actually develop with.

WHAT is this pull request doing?

Filters organization list (Which organization is this work for?) to only show organizations where the user has the develop_apps permissions.

How to test your changes?

  1. Run app config link with a user that has access to BP orgs with develop_apps (e.g. Dev User). It should work as usual, you will be prompted to select an org.
  2. Run app config link the CLI with a user that has access to BP orgs without the develop_apps permission (e.g. Alice). You should get an error because no orgs are available.
    • Note that if you run the CLI with this user on main, you will be prompted to select one of these orgs, but you'll hit downstream problems because you lack the necessary permissions to develop apps using those orgs.

Tophatting won't work properly until some caching behavior is cleaned up here.

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

Checklist

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

Copy link
Contributor Author

amcaplan commented Dec 5, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Contributor

github-actions bot commented Dec 5, 2024

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements 75.12% 8879/11820
🟡 Branches 70.27% 4338/6173
🟡 Functions 75.04% 2324/3097
🟡 Lines
75.68% (+0.01% 🔼)
8395/11093
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / app-event-watcher.ts
93.83% (-1.23% 🔻)
86.49% (-2.7% 🔻)
90.48% 98.61%

Test suite run success

2005 tests passing in 905 suites.

Report generated by 🧪jest coverage report action from 6c74a9e

@amcaplan amcaplan force-pushed the filter-orgs-by-permission branch from bfdf841 to c555a81 Compare January 13, 2025 19:18
@amcaplan amcaplan force-pushed the filter-orgs-by-permission branch from c555a81 to 6c74a9e Compare January 14, 2025 14:09
@amcaplan amcaplan marked this pull request as ready for review January 14, 2025 14:13
@amcaplan amcaplan requested a review from a team as a code owner January 14, 2025 14:13
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.

@amcaplan amcaplan added this pull request to the merge queue Jan 15, 2025
Merged via the queue into main with commit 31ee2c2 Jan 15, 2025
29 checks passed
@amcaplan amcaplan deleted the filter-orgs-by-permission branch January 15, 2025 21:26
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.

3 participants