task: address PR #339 review feedback by dcalhoun · Pull Request #400 · wordpress-mobile/GutenbergKit

dcalhoun · 2026-03-23T21:24:41Z

What?

Addresses the review feedback on #339, fixing merge conflict regressions, cleaning up unused code, and adding documentation.

Why?

Several trunk changes were lost during merge conflict resolution in #339, and reviewers identified cleanup opportunities.

How?

Restore .gitignore patterns for translation files (from 3079431) and wp-env mu-plugin (from aa8eaf1)
Restore RESTAPIRepository.kt editor settings guard and its test assertions (from ba7c9c2)
Restore iOS Network Fallback picker and offline handling (from f5c91cc), adapted for Account type
Simplify editorAssets config spread in E2E helper
Remove false-positive toBeHidden E2E assertion
Delete unused basicFetch module (src/utils/fetch.js)
Add WordPress.com OAuth setup documentation (docs/code/wpcom-oauth.md)
Update wordpress-rs to trunk revision

Testing Instructions

Verify Android unit tests pass: make test-android
Verify iOS unit tests pass: make test-swift-package
Verify .gitignore matches trunk for translation files and wp-env mu-plugin entries
Verify iOS demo app builds with the restored Network Fallback picker

…n Android Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Restore the gitignore pattern from 3079431 that was lost during merge conflict resolution. This ensures the translations directory always exists (via .gitkeep) to avoid Node.js module load errors. Ref: 3079431 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Restore the gitignore exception from aa8eaf1 that was lost during merge conflict resolution. This ensures the custom Jetpack blocks MU plugin is tracked while other MU plugins are excluded. Ref: aa8eaf1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Restore the guard from ba7c9c2 that was lost during merge conflict resolution. The guard should check only `themeStyles`, not both `plugins` and `themeStyles`, to avoid 404s on sites that support plugins but not the editor settings endpoint. Ref: ba7c9c2 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Restore the Network Fallback picker UI, networkFallbackMode property, offline error handling, and buildOfflineConfiguration helper that were lost during merge conflict resolution. Adapted from ConfiguredEditor to Account type to match the current branch's data model. Ref: f5c91cc Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Align editorAssets with editorSettings by using a plain property instead of a conditional spread. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The toBeHidden() assertion passes vacuously because the plugin load failure notice never renders when plugins are enabled without editorAssets — loadEditorAssets() returns {} without error, so pluginLoadFailed is false. The toBeVisible() check for the editor already confirms successful load. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The basicFetch export in src/utils/fetch.js has no remaining importers after the bridge was updated to use native communication instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add documentation for creating a WordPress.com application for OAuth and configuring the demo apps with the required credentials, including the required Redirect URL value. Ref: https://developer.wordpress.com/docs/api/oauth2/ Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Update from branch revision (1190-c2b404d) to trunk revision (dc86c7a) as required before merging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The outdated structure led to build errors after updating the wordpress-rs revision.

dcalhoun · 2026-03-24T14:06:30Z

android/app/build.gradle.kts

+    applicationVariants.configureEach {
+        mergeAssetsProvider.configure {
+            dependsOn(copyOAuthCredentials)
+        }
+    }


In my local testing, while the file was present in the project root, Android Studio builds failed as the OAuth credentials file was perpetually missing without this addition.

dcalhoun · 2026-03-24T14:07:00Z

android/Gutenberg/src/main/java/org/wordpress/gutenberg/RESTAPIRepository.kt

     */
    suspend fun fetchEditorSettings(): EditorSettings {
-        if (!configuration.plugins && !configuration.themeStyles) {
+        if (!configuration.themeStyles) {


Reinstate the fix from ba7c9c2.

dcalhoun · 2026-03-24T14:08:09Z

e2e/editor-error.spec.js

-		await expect(
-			page.getByText(
-				'Loading plugins failed, using default editor configuration.'
-			)
-		).toBeHidden();


Remove a false-positive. A notice is no longer displayed at all as a request failure does not occur, as we no longer use a network request to retrieve the editor assets.

dcalhoun · 2026-03-24T14:09:02Z

ios/Demo-iOS/Sources/Views/SitePreparationView.swift

                Toggle("Enable Native Inserter", isOn: $viewModel.enableNativeInserter)
                Toggle("Enable Network Logging", isOn: $viewModel.enableNetworkLogging)

+                Picker("Network Fallback", selection: $viewModel.networkFallbackMode) {


Reinstate offline editor experience improvements from f5c91cc.

dcalhoun · 2026-03-24T14:09:12Z

src/utils/fetch.js

Remove unused module.

dcalhoun · 2026-03-24T14:09:35Z

.gitignore

+src/translations/*
+!src/translations/.gitkeep


Reinstate build improvements from 3079431.

dcalhoun · 2026-03-24T14:10:03Z

.gitignore

 .wp-env.credentials.json
 wp-env/mu-plugins/*
 !wp-env/mu-plugins/gutenbergkit-cors.php
+!wp-env/mu-plugins/gutenbergkit-jetpack-blocks.php


Reinstate tracking local wp-env configuration from aa8eaf1.

…339) * feat: add WordPress.com OAuth support to Android demo app Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: add WordPress.com OAuth support to iOS demo app Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: inject editor assets via native config and fix API path handling Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve ambiguous Paragraph button match in E2E tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * task: address PR #339 review feedback (#400) * chore: ensure OAuth credentials are copied before assets are merged on Android Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: restore translation files .gitignore pattern Restore the gitignore pattern from 3079431 that was lost during merge conflict resolution. This ensures the translations directory always exists (via .gitkeep) to avoid Node.js module load errors. Ref: 3079431 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: restore wp-env mu-plugin .gitignore exception Restore the gitignore exception from aa8eaf1 that was lost during merge conflict resolution. This ensures the custom Jetpack blocks MU plugin is tracked while other MU plugins are excluded. Ref: aa8eaf1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: restore editor settings guard condition Restore the guard from ba7c9c2 that was lost during merge conflict resolution. The guard should check only `themeStyles`, not both `plugins` and `themeStyles`, to avoid 404s on sites that support plugins but not the editor settings endpoint. Ref: ba7c9c2 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: restore network fallback picker and offline handling Restore the Network Fallback picker UI, networkFallbackMode property, offline error handling, and buildOfflineConfiguration helper that were lost during merge conflict resolution. Adapted from ConfiguredEditor to Account type to match the current branch's data model. Ref: f5c91cc Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: simplify editorAssets config in E2E helper Align editorAssets with editorSettings by using a plain property instead of a conditional spread. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: remove false-positive toBeHidden assertion in E2E test The toBeHidden() assertion passes vacuously because the plugin load failure notice never renders when plugins are enabled without editorAssets — loadEditorAssets() returns {} without error, so pluginLoadFailed is false. The toBeVisible() check for the editor already confirms successful load. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove unused basicFetch module The basicFetch export in src/utils/fetch.js has no remaining importers after the bridge was updated to use native communication instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add WordPress.com OAuth setup documentation Add documentation for creating a WordPress.com application for OAuth and configuring the demo apps with the required credentials, including the required Redirect URL value. Ref: https://developer.wordpress.com/docs/api/oauth2/ Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * build: update wordpress-rs to trunk revision Update from branch revision (1190-c2b404d) to trunk revision (dc86c7a) as required before merging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * task: Use latest WpApiError type The outdated structure led to build errors after updating the wordpress-rs revision. --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: update Android demo app for wordpress-rs API changes Adapt to breaking changes in wordpress-rs (dc86c7a6): - WpLoginClient and WpComApiClient now require NetworkAvailabilityProvider - KeystorePasswordTransformer now requires applicationName parameter Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: add OkHttp as direct dependency for Android demo app Resolves Kotlin compiler warnings about accessing the transitive Interceptor class from wordpress-rs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: regenerate Detekt baseline after trunk merge The OAuth changes in AuthenticationManager.kt and MainActivity.kt altered function signatures that no longer matched the baseline entries introduced by the Detekt PR in trunk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: consolidate NetworkAvailabilityProvider Move the single NetworkAvailabilityProvider instance to GutenbergKitApplication, matching the existing pattern used for AccountRepository. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: skip OAuth asset registration when credentials file is missing Only register the copyOAuthCredentials task and asset source directory when wp_com_oauth_credentials.json exists. This avoids a Gradle task dependency validation error in CI where the file is absent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: David Calhoun <github@davidcalhoun.me>

dcalhoun and others added 10 commits March 23, 2026 17:20

chore: ensure OAuth credentials are copied before assets are merged o…

91cfe03

…n Android Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor: simplify editorAssets config in E2E helper

2815ec5

Align editorAssets with editorSettings by using a plain property instead of a conditional spread. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

chore: remove unused basicFetch module

38acdac

The basicFetch export in src/utils/fetch.js has no remaining importers after the bridge was updated to use native communication instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

build: update wordpress-rs to trunk revision

3253e32

Update from branch revision (1190-c2b404d) to trunk revision (dc86c7a) as required before merging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

dcalhoun added the [Type] Code Quality Issues or PRs that relate to code quality label Mar 23, 2026

github-actions bot added the [Type] Bug An existing feature does not function as intended label Mar 23, 2026

dcalhoun changed the title ~~fix: address PR #339 review feedback~~ task: address PR #339 review feedback Mar 23, 2026

dcalhoun removed the [Type] Bug An existing feature does not function as intended label Mar 23, 2026

task: Use latest WpApiError type

dc418ef

The outdated structure led to build errors after updating the wordpress-rs revision.

dcalhoun marked this pull request as ready for review March 24, 2026 14:16

dcalhoun commented Mar 24, 2026

View reviewed changes

dcalhoun merged commit e2135be into jkmassel/wordpress-com-support Mar 24, 2026
10 checks passed

dcalhoun deleted the task/address-wpcom-auth-shortcomings branch March 24, 2026 14:35

dcalhoun mentioned this pull request Mar 24, 2026

chore: add WordPress.com OAuth support to iOS and Android demo apps #339

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

task: address PR #339 review feedback#400

task: address PR #339 review feedback#400
dcalhoun merged 11 commits intojkmassel/wordpress-com-supportfrom
task/address-wpcom-auth-shortcomings

dcalhoun commented Mar 23, 2026

Uh oh!

dcalhoun Mar 24, 2026

Uh oh!

dcalhoun Mar 24, 2026

Uh oh!

dcalhoun Mar 24, 2026

Uh oh!

dcalhoun Mar 24, 2026

Uh oh!

dcalhoun Mar 24, 2026

Uh oh!

dcalhoun Mar 24, 2026

Uh oh!

dcalhoun Mar 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		src/translations/*
		!src/translations/.gitkeep

Conversation

dcalhoun commented Mar 23, 2026

What?

Why?

How?

Testing Instructions

Uh oh!

dcalhoun Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant