Skip to content

EditorLauncher Scaffold Implementation #22111

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 15 commits into from
Aug 12, 2025
Merged

Conversation

oguzkocer
Copy link
Contributor

Summary

Adds a centralized EditorLauncher system to replace direct Intent creation for editor activities, providing type-safe parameters and consistent analytics tracking.

Motivation

This scaffolding enables the future introduction of a dedicated GutenbergKitActivity by centralizing editor launch logic. The plan is to:

  1. Duplicate EditPostActivity as GutenbergKitActivity
  2. Simplify GutenbergKitActivity to only work with GutenbergKit editor, removing legacy code paths
  3. Use EditorLauncher routing to direct users to the appropriate activity based on feature flags
  4. Keep EditPostActivity as the legacy activity for existing functionality

This approach allows iterating on a clean, modern editor experience while maintaining backward compatibility through the existing activity.

Key Components

EditorLauncher

  • Purpose: Centralized helper for launching editor activities with proper routing logic
  • Features:
    • Determines which editor activity to launch based on feature flags
    • Provides analytics tracking for editor launches
    • Handles all Intent extra mapping from type-safe parameters
  • Analytics: Tracks EDITOR_LAUNCHER events with will_use_gutenberg_kit property and adds EXTRA_LAUNCHED_VIA_EDITOR_LAUNCHER intent extra

EditorLauncherParams

  • Purpose: Type-safe parameters for launching editor activities
  • Design: Kotlin data class with Java Builder pattern for compatibility
  • Builder: Requires non-null SiteModel in constructor for type safety
  • Coverage: Supports all existing editor launch scenarios (posts, pages, reblog, etc.)

Integration

  • ActivityLauncher.java: Updated all editor launch methods to use EditorLauncher
  • Analytics: Centralized EXTRA_CREATION_SOURCE_DETAIL handling eliminates code duplication
  • Dependency Injection: EditorLauncher registered in AppComponent with singleton scope

Technical Details

Type Safety Improvements

  • Builder pattern requires SiteModel parameter upfront (no runtime null checks)
  • All editor parameters validated at compile time through structured data class
  • Intent extras automatically mapped from parameters

Analytics Centralization

Previously, every ActivityLauncher method manually added analytics extras:

Intent intent = EditorLauncher.getInstance().createEditorIntent(activity, params);
intent.putExtra(AnalyticsUtils.EXTRA_CREATION_SOURCE_DETAIL, source);

Now handled automatically in EditorLauncher when params.source is provided.

Launch Method Tracking

EditorLauncher adds EXTRA_LAUNCHED_VIA_EDITOR_LAUNCHER intent extra to distinguish between:

  • Editor launches via EditorLauncher (tracked as EDITOR_LAUNCHED_VIA_EDITOR_LAUNCHER)
  • Direct Intent creation (legacy path)

This enables measuring EditorLauncher adoption vs legacy launch methods.

Completeness Validation

  • EditorLauncherTest: Unit test validates all EditorLauncherParams fields are handled
  • Documentation: Cross-references between classes and test for field-to-method mapping
  • Static Enforcement: Test fails if new fields aren't properly handled

Backward Compatibility

  • All existing editor launch paths continue to work unchanged
  • Legacy direct Intent creation still supported where needed
  • No changes to editor activity implementations

Implement hybrid approach for editor activity routing that will support
both legacy EditPostActivity and future EditPostGutenbergKitActivity.

Changes:
- Add EditorLauncher singleton with feature-flag-based routing logic
- Migrate all ActivityLauncher methods to use EditorLauncher helper
- Add EditorLauncher to dependency injection (AppComponent)
- Include analytics tracking and source identification for launches
- Route to EditPostActivity for now (scaffold implementation)

This provides infrastructure for clean editor activity separation while
maintaining backward compatibility. Future changes will:
- Add EditorLauncherParams data class with builder pattern
- Route to EditPostGutenbergKitActivity when implemented
- Add redirection fallback with monitoring

Covers ActivityLauncher methods:
- openEditorForSiteInNewStack()
- openEditorForPostInNewStack()
- openEditorForReblog()
- addNewPostForResult() variants
- editPostOrPageForResult() variants
- editPageForResult() variants
- addNewPageForResult() variants
Replace Bundle-based Intent extras with type-safe parameter objects
to improve maintainability and reduce errors.

Changes:
- Add EditorLauncherParams.kt with Kotlin data class and Java Builder pattern
- Update EditorLauncher.kt to accept EditorLauncherParams instead of Intent bundles
- Refactor ActivityLauncher.java methods to use EditorLauncherParams.Builder
- Add backward compatibility with deprecated Intent-based method
- Centralize all Intent extra handling in EditorLauncher.addEditorExtras()
- Maintain analytics tracking with type-safe parameter source detection

This provides compile-time safety while maintaining Java interoperability
through the builder pattern.
Complete migration to EditorLauncherParams for all ActivityLauncher methods.

Changes:
- Remove deprecated createEditorIntent(context, Intent) method from EditorLauncher
- Remove unused getIntentSource() legacy method
- Update remaining ActivityLauncher methods to use EditorLauncherParams.Builder:
  - addNewPostWithContentFromAIForResult()
  - addNewPageForResult() (both Activity and Fragment variants)
- All editor launches now use type-safe parameters instead of Bundle operations

This completes the EditorLauncherParams migration for ActivityLauncher,
providing compile-time safety and better maintainability.
Address all code quality issues identified by Detekt static analysis.

Changes:
- Remove unused imports (Bundle) and unused property (analyticsTracker)
- Replace TODO comments with regular comments to avoid ForbiddenComment violations
- Break down complex addEditorExtras() method into smaller helper methods:
  - addBasicExtras() - site, page, promo flags
  - addPostExtras() - post IDs, autosave, quickpress, landing editor flags
  - addReblogExtras() - reblog title, quote, image, citation, action
  - addPageExtras() - page title, content, template
  - addMiscExtras() - voice content, media, prompt ID, entry point
- Fix MaxLineLength violations by splitting long builder method calls
- Add missing newline at end of EditorLauncherParams.kt

This reduces cyclomatic complexity and improves code maintainability
while maintaining all existing functionality.
Centralizes the EXTRA_CREATION_SOURCE_DETAIL Intent extra creation in
EditorLauncher.addMiscExtras instead of requiring manual addition in
each ActivityLauncher method. This eliminates code duplication and
ensures analytics tracking is consistently applied across all editor launches.

Changes:
- Add AnalyticsUtils.EXTRA_CREATION_SOURCE_DETAIL to EditorLauncher.addMiscExtras
- Remove redundant analytics field additions from ActivityLauncher methods
- Add AnalyticsUtils import to EditorLauncher
Makes the Java Builder pattern more type-safe by requiring SiteModel in the constructor
rather than checking for null at build time.

Changes:
- EditorLauncherParams.Builder now takes @nonnull SiteModel in constructor
- Remove site() setter method from Builder
- Remove runtime requireNotNull check in build()
- Update all call sites to pass site to constructor
- Add missing @nonnull annotations in ActivityLauncher
Tracks editor launches with should_use_gutenberg_kit property to understand
feature flag usage patterns.
Ensures all EditorLauncherParams fields are handled in EditorLauncher.addEditorExtras()
through documentation and a unit test that validates completeness.

Changes:
- Add cross-reference documentation in EditorLauncherParams and EditorLauncher
- Add EditorLauncherTest with detailed field-to-method mapping documentation
- Test validates that all fields are accounted for in addEditorExtras methods
- Filter out Kotlin synthetic fields in test validation
Adds a way to distinguish editor launches that went through EditorLauncher
vs direct Intent creation for analytics purposes.

Changes:
- EditorLauncher: Add EXTRA_LAUNCHED_VIA_EDITOR_LAUNCHER intent extra
- EditPostActivity: Track EDITOR_LAUNCHED_VIA_EDITOR_LAUNCHER when extra is present (only on initial creation, not configuration changes)
- AnalyticsTracker: Add EDITOR_LAUNCHED_VIA_EDITOR_LAUNCHER stat
@oguzkocer oguzkocer added this to the 26.1 milestone Aug 8, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Aug 8, 2025

2 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ Class Builder is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 8, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr22111-4198b77
Commit4198b77
Direct Downloadjetpack-prototype-build-pr22111-4198b77.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Aug 8, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr22111-4198b77
Commit4198b77
Direct Downloadwordpress-prototype-build-pr22111-4198b77.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented Aug 8, 2025

Codecov Report

❌ Patch coverage is 0% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.43%. Comparing base (8296f75) to head (4198b77).
⚠️ Report is 8 commits behind head on trunk.

Files with missing lines Patch % Lines
...ava/org/wordpress/android/ui/ActivityLauncher.java 0.00% 38 Missing ⚠️
.../ui/notifications/NotificationsListFragmentPage.kt 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #22111   +/-   ##
=======================================
  Coverage   39.43%   39.43%           
=======================================
  Files        2149     2149           
  Lines      101872   101872           
  Branches    15604    15604           
=======================================
  Hits        40177    40177           
  Misses      58126    58126           
  Partials     3569     3569           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@oguzkocer oguzkocer marked this pull request as ready for review August 8, 2025 01:37
@oguzkocer oguzkocer requested a review from nbradbury August 8, 2025 01:38
@nbradbury
Copy link
Contributor

I asked Claude this:

Is EditPostActivity ever launched from somewhere other than EditorLauncher?

And got this response:

Yes, EditPostActivity is still launched directly from 4 other places that weren't migrated 
to use EditorLauncher:

  1. ShareIntentReceiverFragment.java:214 - Enum definition for share actions
  2. AddQuickPressShortcutActivity.java:143 - Creating QuickPress shortcuts
  3. UploadUtils.java:613 - Launching editor after media upload
  4. PostUploadNotifier.java:444 - Notification action to edit post

  These should also be migrated to use EditorLauncher for consistency 

I don't think the first one is an issue, but the others appear to be launching EditPostActivity. Should this be addressed in this PR?

@nbradbury nbradbury self-assigned this Aug 8, 2025
@nbradbury
Copy link
Contributor

When I tap a post in the post list, it shows an empty post in the editor. Same for pages.

empty-post.mp4

Logcat shows we're passing 0 as the post ID:

Unexpected response code 400 for https://public-api.wordpress.com/rest/v1.1/sites/235002728/post/0/diffs/?locale=en_US

@nbradbury
Copy link
Contributor

Sharing a photo to the app also fails with the same HTTP 400 issue.

Unexpected response code 400 for https://public-api.wordpress.com/rest/v1.1/sites/235002728/post/0/diffs/?locale=en_US
share.mp4

Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

When I tap a post in the post list, it shows an empty post in the editor. Same for pages.

I experience the same for each of the three editors—GutenbergKit, Gutenberg Mobile, and Aztec. Surprisingly, it does not occur when creating a new page from a template; that flow successfully populates the editor.

Sharing a photo to the app also fails with the same HTTP 400 issue.

Sharing media to GutenbergKit is currently unsupported, see CMM-190. That said, sharing media to Gutenberg Mobile appears broken in the trunk branch, so likely unrelated to these changes. 😞


I performed quick smoke testing of fundamental features in each of GutenbergKit, Gutenberg Mobile, and Aztec—text editing, media uploads, drafting, publishing, analytics, network connection changes, OS appearance changes, etc. I did not encounter regressions aside from those noted above.

Prevents crashes when PostModel is null (e.g., when mPostStore.getPostByLocalPostId
returns null for non-existent posts) by showing a user-friendly error message instead.

Changes:
- Add null check and error toast in both editPostOrPageForResult variants
- Prevents crashes from calling post.getId() or post.isPage() on null objects
The trunk editPostOrPageForResult methods never set boolean extras like
EXTRA_IS_PAGE, EXTRA_IS_PROMO, or EXTRA_LOAD_AUTO_SAVE_REVISION unless
explicitly required. Making all boolean parameters nullable ensures they're
only set when explicitly provided, matching trunk behavior exactly.

Changes:
- Make all boolean fields nullable in EditorLauncherParams and Builder
- Update EditorLauncher to only set extras when parameters are non-null
- Remove unnecessary loadAutoSaveRevision(false) from first editPostOrPageForResult
- Preserves explicit boolean settings in other methods (pages, promos, etc.)
@oguzkocer oguzkocer force-pushed the feature/editor-launcher-scaffold branch from 0f02cd3 to 2c1d8d5 Compare August 8, 2025 19:16
@oguzkocer
Copy link
Contributor Author

I don't think the first one is an issue, but the others appear to be launching EditPostActivity. Should this be addressed in this PR?

@nbradbury Let me take a look at that in a separate PR.

@oguzkocer
Copy link
Contributor Author

When I tap a post in the post list, it shows an empty post in the editor. Same for pages.

This was an unfortunate oversight by me, and it was happening because some of the EditorLauncherParams properties were using false as the default value when they should have been null. This should be addressed in 2c1d8d5.

Really sorry about the faulty PR 🙇‍♂️

.reblogPostImage(post.getFeaturedImage())
.reblogPostCitation(post.getUrl())
.reblogAction(EditPostActivityConstants.ACTION_REBLOG)
.source(reblogSource)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original implementation didn't set this here and set it in addNewPostForResult instead. This wouldn't have been a problem, but I've removed it in 4198b77 to match the previous implementation.

Copy link

sonarqubecloud bot commented Aug 8, 2025

@oguzkocer oguzkocer modified the milestones: 26.1, 26.2 Aug 8, 2025
Copy link
Contributor

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

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

@oguzkocer The problem with opening posts/pages appears to be resolved so merge when ready :shipit:

Copy link
Member

@dcalhoun dcalhoun left a comment

Choose a reason for hiding this comment

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

The latest iteration tested well for me in GutenbergKit, Gutenberg Mobile, and Aztec. I believe we are good to merge.

@oguzkocer oguzkocer merged commit 37a5943 into trunk Aug 12, 2025
27 of 28 checks passed
@oguzkocer oguzkocer deleted the feature/editor-launcher-scaffold branch August 12, 2025 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants