Skip to content

Conversation

@javier-godoy
Copy link
Member

@javier-godoy javier-godoy commented Oct 28, 2025

Close #121

Summary by CodeRabbit

  • New Features

    • Added Java 21 profile and Vaadin 25 prerelease support.
    • Enhanced JSON migration capabilities for frontend integration.
  • Chores

    • Bumped project version to 4.4.0 and updated Jetty to 11.0.26.
    • Added json-migration-helper and snapshot repository configuration.
    • Expanded .gitignore for frontend-generated artifacts.
    • Updated copyright year and added a minimal AppShell configurator.

@javier-godoy
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

Walkthrough

Updates prepare the project for Vaadin 25: version bump to 4.4.0-SNAPSHOT, Jetty upgrade, new json-migration-helper dependency and repositories, added Maven profile for Java 21/Vaadin 25, .gitignore additions, small Java source edits to use JsonMigration helpers, and a new test AppShellConfiguratorImpl class.

Changes

Cohort / File(s) Summary
Build config & ignores
pom.xml, .gitignore
Project version → 4.4.0-SNAPSHOT; Jetty 11.0.1211.0.26; added flowing-snapshots repository and Vaadin prerelease repos; added json-migration-helper dependency 0.0.1-SNAPSHOT; renamed Jetty config webAppConfigwebApp; added .gitignore entries for /src/main/frontend/generated and /src/main/frontend/index.html.
Vaadin JSON migration edits
src/main/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewer.java, src/main/java/com/flowingcode/vaadin/addons/demo/MultiSourceCodeViewer.java
Copyright year updated to 2025; added JsonMigration import and replaced direct JSON property/event handling with JsonMigration helper calls for property/event data access to accommodate Vaadin 25.
Test AppShell configurator
src/test/java/com/flowingcode/vaadin/addons/demo/AppShellConfiguratorImpl.java
New public class AppShellConfiguratorImpl implementing AppShellConfigurator, annotated with @Theme, minimal empty body.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review pom.xml for duplicated profile/pluginManagement entries and correctness of added repositories/dependency and the license plugin exclusion.
  • Verify JsonMigration usage in the two Java files correctly maps event/property JSON for Vaadin 25 and retains compatibility with existing flows.
  • Confirm AppShellConfiguratorImpl placement under src/test is intentional and aligns with the license plugin exclusion referenced in pom.xml.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The PR includes several changes not explicitly mentioned in issue #121. The .gitignore additions for frontend artifacts, the Jetty version upgrade from 11.0.12 to 11.0.26, the new AppShellConfiguratorImpl test class, the flowing-snapshots repository addition, and multiple pom.xml reconfigurations (webAppConfig renaming, license plugin exclusions, profile duplications) all go beyond the specific scope of fixing the NoSuchMethodError in the two identified files. While some may be necessary supporting infrastructure for Vaadin 25 compatibility, they are not explained or justified in the linked issue. Consider clarifying which of these additional changes are required for Vaadin 25 compatibility and which are ancillary improvements. If they are supporting infrastructure, document this in the PR description or issue. Additionally, review the pom.xml for the noted duplicated profile additions to ensure there are no structural issues. Consider whether each major dependency upgrade (like Jetty) and new classes (like AppShellConfiguratorImpl) directly support the core objective stated in issue #121.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: add compatibility with Vaadin 25" clearly and concisely describes the primary objective of the pull request. It directly relates to the main change—adding support for Vaadin 25—and provides sufficient context for someone scanning commit history to understand the purpose. The title is specific enough to distinguish this PR from other maintenance or feature updates without being overly verbose.
Linked Issues Check ✅ Passed The PR directly addresses the requirements in issue #121 by updating both specified files. SourceCodeViewer.java now uses JsonMigration.setPropertyJson(codeViewer, "env", env) instead of the deprecated direct method call, and MultiSourceCodeViewer.java now uses JsonMigration.getEventData(ev) instead of the deprecated approach. These changes resolve the NoSuchMethodError by providing a compatibility layer that handles Vaadin 25's transition from JSON Elemental to Jackson-based JSON handling. The new json-migration-helper dependency enables this migration path.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch vaadin25-wip

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 845fabf and f7b601f.

📒 Files selected for processing (1)
  • src/main/java/com/flowingcode/vaadin/addons/demo/MultiSourceCodeViewer.java (3 hunks)
🔇 Additional comments (3)
src/main/java/com/flowingcode/vaadin/addons/demo/MultiSourceCodeViewer.java (3)

5-5: LGTM: Copyright year updated.

Standard maintenance to update the copyright year to 2025.


23-23: LGTM: Import added for JsonMigration helper.

The import is necessary for the Vaadin 25 compatibility layer used at line 50.


49-55: LGTM: Correctly implements Vaadin 25 compatibility.

The migration from ev.getEventData() to JsonMigration.getEventData(ev) properly addresses the Vaadin 25 JSON handling change from Elemental JSON to Jackson. The existing null handling and error propagation logic remains intact.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
.gitignore (1)

22-23: Potential duplication with existing ignore patterns.

Lines 16-17 already contain /frontend/generated and /frontend/index.html, which may overlap with these new entries. If the project only uses one directory structure, consider consolidating to avoid redundancy.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77d981e and 845fabf.

📒 Files selected for processing (5)
  • .gitignore (1 hunks)
  • pom.xml (7 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/demo/MultiSourceCodeViewer.java (3 hunks)
  • src/main/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewer.java (3 hunks)
  • src/test/java/com/flowingcode/vaadin/addons/demo/AppShellConfiguratorImpl.java (1 hunks)
🔇 Additional comments (6)
src/main/java/com/flowingcode/vaadin/addons/demo/SourceCodeViewer.java (1)

23-23: Migration correctly addresses Vaadin 25 compatibility.

The change from codeViewer.setPropertyJson("env", env) to JsonMigration.setPropertyJson(codeViewer, "env", env) properly wraps the Vaadin Element API call to support both Vaadin 24 (JSON Elemental) and Vaadin 25 (Jackson). This directly resolves the NoSuchMethodError reported in issue #121.

Also applies to: 89-89

src/test/java/com/flowingcode/vaadin/addons/demo/AppShellConfiguratorImpl.java (1)

1-9: LGTM! Minimal AppShell configurator is appropriate.

This minimal implementation provides the necessary AppShell configuration for the test/demo environment. The @theme annotation with default settings is sufficient for the intended purpose.

src/main/java/com/flowingcode/vaadin/addons/demo/MultiSourceCodeViewer.java (1)

23-23: Migration correctly handles event data for Vaadin 25.

The wrapping of event data with JsonMigration.convertToJsonValue() ensures compatibility with Vaadin 25's Jackson-based JSON handling while maintaining support for Vaadin 24. This change properly addresses the event data format differences between versions.

Also applies to: 49-56

pom.xml (3)

27-27: Verify Jetty upgrade compatibility.

The Jetty version jumped from 11.0.12 to 11.0.26 (14 patch versions). While patch versions typically maintain backward compatibility, this is a significant jump that may include changes affecting the development server. Ensure the upgrade has been tested with the demo application.


58-64: Verify SNAPSHOT dependency is intentional.

The json-migration-helper dependency uses version 0.0.1-SNAPSHOT, which requires the flowing-snapshots repository. SNAPSHOT versions are typically avoided in released code as they can change over time, potentially causing build instability.

Confirm whether:

  1. This SNAPSHOT dependency is temporary until a stable release is available
  2. The project plans to release with a stable version of json-migration-helper before going to production

Also applies to: 106-110


319-339: LGTM! v25 profile correctly enables Vaadin 25 testing.

The new Maven profile properly configures:

  • Java 21 (required for Vaadin 25)
  • Vaadin 25.0.0-beta2 version
  • Prerelease repositories for dependencies and plugins

This allows the project to be tested with Vaadin 25 without affecting the default build, which is the recommended approach for dual-version support.

@sonarqubecloud
Copy link

@paodb paodb merged commit 7ce0bcc into master Nov 3, 2025
3 checks passed
@paodb paodb deleted the vaadin25-wip branch November 3, 2025 12:30
@github-project-automation github-project-automation bot moved this from To Do to Pending release in Flowing Code Addons Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending release

Development

Successfully merging this pull request may close these issues.

Vaadin 25: NoSuchMethodError Element.setPropertyJson(String, JsonValue)

3 participants