-
Notifications
You must be signed in to change notification settings - Fork 158
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
refactor app management client to prepare for new BP mutation #5529
Draft
gordonhirsch
wants to merge
1
commit into
main
Choose a base branch
from
refactor-app-management-client
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+14
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 16, 2025
Coverage report
Test suite run success2169 tests passing in 950 suites. Report generated by 🧪jest coverage report action from 6f7f603 |
d684699
to
a642ed7
Compare
isaacroldan
approved these changes
Mar 18, 2025
a642ed7
to
0df7e14
Compare
0df7e14
to
6f7f603
Compare
5 tasks
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/node/api/business-platform.d.ts@@ -40,6 +40,6 @@ export declare function businessPlatformOrganizationsRequest<T>(query: string, t
* @param variables - GraphQL variables to pass to the query.
* @returns The response of the query of generic type <T>.
*/
-export declare function businessPlatformOrganizationsRequestDoc<TResult>(query: TypedDocumentNode<TResult, GraphQLVariables> | TypedDocumentNode<TResult, Exact<{
+export declare function businessPlatformOrganizationsRequestDoc<TResult, TVariables extends Variables>(query: TypedDocumentNode<TResult, TVariables> | TypedDocumentNode<TResult, Exact<{
[key: string]: never;
-}>>, token: string, organizationId: string, variables?: GraphQLVariables): Promise<TResult>;
\ No newline at end of file
+}>>, token: string, organizationId: string, variables?: TVariables): Promise<TResult>;
\ No newline at end of file
|
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WHY are these changes introduced?
This Refactor/cleanup of the business platform interface allows the addition of new BP calls in the future and makes the existing code clearer.
WHAT is this pull request doing?
This PR renames the
encodedGidFromId
function toencodedGidFromOrganizationId
to make its purpose more explicit and to make room for encoding other id types in the future.Additionally, it updates the type signature for
businessPlatformOrganizationsRequestDoc
to properly handle generic variables, improving type safety. This makes it consistent with other*RequestDoc
functions and seems to allow proper usage with mutations.How to test your changes?
Run the existing test suite to ensure all tests pass with the renamed function. It was not necessary to add new test.
Measuring impact
How do we know this change was effective? Please choose one:
Checklist