-
Notifications
You must be signed in to change notification settings - Fork 3
fix(android): include restBase and restNamespace in GBKit post payload #432
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
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
b7930f0
fix(android): include restBase and restNamespace in GBKit post payload
dcalhoun 9f94bef
test(android): assert restBase and restNamespace in GBKitGlobal payload
dcalhoun dbf4d12
fix(api-fetch): apply post fallbacks in filterEndpointsMiddleware
dcalhoun 944fbfc
docs(api-fetch): note follow-up for filterEndpointsMiddleware
dcalhoun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 file contains hidden or 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 file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ import { getQueryArg } from '@wordpress/url'; | |
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { getGBKit } from './bridge'; | ||
| import { getGBKit, POST_FALLBACKS } from './bridge'; | ||
|
|
||
| /** | ||
| * @typedef {import('@wordpress/api-fetch').APIFetchMiddleware} APIFetchMiddleware | ||
|
|
@@ -111,16 +111,31 @@ function tokenAuthMiddleware( options, next ) { | |
| * Middleware to filter out requests to specific endpoints. | ||
| * | ||
| * @type {APIFetchMiddleware} | ||
| * | ||
| * @todo Properly seed the post entity and remove this middleware. | ||
| * | ||
| * This was added to prevent re-fetching entity content provided by the native | ||
| * host app, which can lead to content loss. However, we can likely avoid the | ||
| * need for this middleware by ensuring we properly seed the entity content into | ||
| * the store on initialization. | ||
| * | ||
| * This requires hoisting the relevant logic from `useEditorSetup` to occur | ||
| * before we render the editor, and invoking `finishResolution`. | ||
| * | ||
| * See: https://github.com/wordpress-mobile/GutenbergKit/commit/c9b4fc9978a3760ba97f3f5d4359c2bc2155bb80 | ||
|
Comment on lines
+115
to
+125
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the future, we can improve entity seeding and negate the need this middleware that manually disables the entity fetch requests. See c9b4fc9 as a proof-of-concept. |
||
| */ | ||
| function filterEndpointsMiddleware( options, next ) { | ||
| const { post } = getGBKit(); | ||
| const { id, restNamespace, restBase } = post ?? {}; | ||
|
|
||
| if ( id === undefined || ! restNamespace || ! restBase ) { | ||
| if ( ! post || post.id === undefined ) { | ||
| return next( options ); | ||
| } | ||
|
|
||
| const disabledPath = `/${ restNamespace }/${ restBase }/${ id }`; | ||
| // Apply the same fallback contract as `getPost()` so the filter still | ||
| // engages on hosts whose payload omits restBase/restNamespace. | ||
| const restNamespace = post.restNamespace || POST_FALLBACKS.restNamespace; | ||
| const restBase = post.restBase || POST_FALLBACKS.restBase; | ||
| const disabledPath = `/${ restNamespace }/${ restBase }/${ post.id }`; | ||
|
|
||
| if ( | ||
| options.path === disabledPath || | ||
|
|
||
This file contains hidden or 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 file contains hidden or 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
Oops, something went wrong.
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.
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.
This will be removed and replaced with fetching of
PostTypeDetailsin a future PR focused on editing existing posts (and align with iOS). In the interim, this ensures Android correctly sets therestBasevalue for posts and pages.