Skip to content

fix: coerce post ID 0 to nil on both platforms and JS bridge #420

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.

Sign up for GitHub

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
dcalhoun merged 2 commits into from
Apr 3, 2026
Merged

fix: coerce post ID 0 to nil on both platforms and JS bridge #420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
736ef9e
fix: coerce post ID 0 to nil on both platforms and JS bridge
jkmassel Apr 3, 2026
a71652f
test: add JS unit tests for post ID 0 coercion
dcalhoun Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions android/Gutenberg/src/main/java/org/wordpress/gutenberg/model/EditorConfiguration.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ data class EditorConfiguration(

fun setTitle(title: String) = apply { this.title = title }
fun setContent(content: String) = apply { this.content = content }
fun setPostId(postId: UInt?) = apply { this.postId = postId }
fun setPostId(postId: UInt?) = apply { this.postId = postId?.takeIf { it != 0u } }
fun setPostType(postType: String) = apply { this.postType = postType }
fun setPostStatus(postStatus: String) = apply { this.postStatus = postStatus }
fun setThemeStyles(themeStyles: Boolean) = apply { this.themeStyles = themeStyles }
Expand All @@ -99,7 +99,7 @@ data class EditorConfiguration(
fun build(): EditorConfiguration = EditorConfiguration(
title = title,
content = content,
postId = postId,
postId = postId?.takeIf { it != 0u },
postType = postType,
postStatus = postStatus,
themeStyles = themeStyles,
Expand Down
9 changes: 9 additions & 0 deletions android/Gutenberg/src/test/java/org/wordpress/gutenberg/model/EditorConfigurationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ class EditorConfigurationBuilderTest {
assertEquals(123u, config.postId)
}

@Test
fun `setPostId with zero results in null`() {
val config = builder()
.setPostId(0u)
.build()

assertNull(config.postId)
}

@Test
fun `setPostId with null clears postId`() {
val config = builder()
Expand Down
2 changes: 1 addition & 1 deletion ios/Sources/GutenbergKit/Sources/Model/EditorConfiguration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public struct EditorConfiguration: Sendable, Hashable, Equatable {
) {
self.title = title
self.content = content
self.postID = postID
self.postID = postID == 0 ? nil : postID
self.postType = postType
self.postStatus = postStatus
self.shouldUseThemeStyles = shouldUseThemeStyles
Expand Down
9 changes: 9 additions & 0 deletions ios/Tests/GutenbergKitTests/Model/EditorConfigurationTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@ struct EditorConfigurationBuilderTests: MakesTestFixtures {
#expect(config.postID == 123)
}

@Test("setPostID with zero results in nil")
func setPostIDWithZeroResultsInNil() {
let config = makeConfigurationBuilder()
.setPostID(0)
.build()

#expect(config.postID == nil)
}

@Test("setPostID with nil clears postID")
func setPostIDWithNilClearsPostID() {
let config = makeConfigurationBuilder()
Expand Down
4 changes: 2 additions & 2 deletions src/utils/bridge.js
View file
Open in desktop
Copy link
Copy Markdown
Member

@dcalhoun dcalhoun Apr 3, 2026

Choose a reason for hiding this comment

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

We could add unit tests for the changes in this file, similar to those added for iOS and Android. The relevant file would be src/utils/bridge.test.js. Below is a diff for the tests.

JS unit test diff 
diff --git a/src/utils/bridge.test.js b/src/utils/bridge.test.js
index 2616efff..91fa98df 100644
--- a/src/utils/bridge.test.js
+++ b/src/utils/bridge.test.js
@@ -282,6 +282,33 @@ describe( 'getPost', () => {
 			expect( result.title.raw ).toBe( 'Updated Title' );
 			expect( result.content.raw ).toBe( 'Updated Content' );
 		} );
+
+		it( 'should coerce post ID 0 to -1 with host content', async () => {
+			const hostContent = {
+				title: 'Title',
+				content: 'Content',
+			};
+			window.webkit = {
+				messageHandlers: {
+					requestLatestContent: {
+						postMessage: vi.fn().mockResolvedValue( hostContent ),
+					},
+				},
+			};
+			window.GBKit = {
+				post: {
+					id: 0,
+					type: 'post',
+					status: 'draft',
+					title: 'Title',
+					content: 'Content',
+				},
+			};
+
+			const result = await getPost();
+
+			expect( result.id ).toBe( -1 );
+		} );
 	} );
 
 	describe( 'fallback to GBKit', () => {
@@ -340,6 +367,22 @@ describe( 'getPost', () => {
 				restNamespace: 'wp/v2',
 			} );
 		} );
+
+		it( 'should coerce post ID 0 to -1 in GBKit fallback', async () => {
+			window.GBKit = {
+				post: {
+					id: 0,
+					type: 'post',
+					status: 'draft',
+					title: 'Title',
+					content: 'Content',
+				},
+			};
+
+			const result = await getPost();
+
+			expect( result.id ).toBe( -1 );
+		} );
 	} );
 
 	describe( 'default empty post', () => {

Copy link
Copy Markdown
Member

@dcalhoun dcalhoun Apr 3, 2026

Choose a reason for hiding this comment

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

I added these tests in a71652f. Feel free to revert if you see fit.

Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ export async function getPost() {
if ( hostContent ) {
debug( 'Using content from native host' );
return {
id: post?.id ?? -1,
id: post?.id || -1,
type: post?.type || 'post',
restBase: post?.restBase || 'posts',
restNamespace: post?.restNamespace || 'wp/v2',
Expand All @@ -313,7 +313,7 @@ export async function getPost() {
if ( post ) {
debug( 'Native bridge unavailable, using GBKit initial content' );
return {
id: post.id,
id: post.id || -1,
type: post.type || 'post',
restBase: post.restBase || 'posts',
restNamespace: post.restNamespace || 'wp/v2',
Expand Down
43 changes: 43 additions & 0 deletions src/utils/bridge.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,33 @@ describe( 'getPost', () => {
expect( result.title.raw ).toBe( 'Updated Title' );
expect( result.content.raw ).toBe( 'Updated Content' );
} );

it( 'should coerce post ID 0 to -1 with host content', async () => {
const hostContent = {
title: 'Title',
content: 'Content',
};
window.webkit = {
messageHandlers: {
requestLatestContent: {
postMessage: vi.fn().mockResolvedValue( hostContent ),
},
},
};
window.GBKit = {
post: {
id: 0,
type: 'post',
status: 'draft',
title: 'Title',
content: 'Content',
},
};

const result = await getPost();

expect( result.id ).toBe( -1 );
} );
} );

describe( 'fallback to GBKit', () => {
Expand Down Expand Up @@ -340,6 +367,22 @@ describe( 'getPost', () => {
restNamespace: 'wp/v2',
} );
} );

it( 'should coerce post ID 0 to -1 in GBKit fallback', async () => {
window.GBKit = {
post: {
id: 0,
type: 'post',
status: 'draft',
title: 'Title',
content: 'Content',
},
};

const result = await getPost();

expect( result.id ).toBe( -1 );
} );
} );

describe( 'default empty post', () => {
Expand Down
Loading