Repository navigation
chore: platform integration javascript for templates - #1191
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The HTML body unhide logic uses an invalid document.body.style=null assignment (risking a permanently hidden UI), and the platform postMessage response target origin handling should be corrected for allowed cross-origin embedding.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Refactors the visualization UI “platform vs standalone” setup by removing the old query-param–based environment.js and introducing a new platform-integration.js that initializes platform context via postMessage, plus adds helper scripts for automated UI smoke-running and platform deployment.
Changes:
- Move default
SETUPinitialization intoquickstart-page.jsand delay client initialization untilSETUP.readyresolves. - Add
shared/platform-integration.js(and per-template copies) to populateSETUPvia a message handshake when embedded on the Timefold Platform. - Add
scripts/DeployToPlatform.javaandscripts/run-all-quarkus.javajbang utilities.
File summaries
| File | Description |
|---|---|
| visualizations/shared/quickstart-page.js | Introduces default SETUP and gates initialization behind SETUP.ready. |
| visualizations/shared/platform-integration.js | Adds platform iframe message handshake + resize reporting. |
| visualizations/shared/index.template.html | Updates script includes; hides body until initialization; disables solve/analyze initially. |
| visualizations/shared/environment.js | Removes old URL-query-param platform detection. |
| use-cases/meeting-scheduling/src/main/resources/META-INF/resources/shared/quickstart-page.js | Same SETUP.ready-gated initialization as shared. |
| use-cases/meeting-scheduling/src/main/resources/META-INF/resources/shared/platform-integration.js | Template copy of platform integration logic. |
| use-cases/meeting-scheduling/src/main/resources/META-INF/resources/shared/environment.js | Removes old environment logic. |
| use-cases/meeting-scheduling/src/main/resources/META-INF/resources/index.html | Switches to platform integration + hides body until init. |
| use-cases/maintenance-scheduling/src/main/resources/META-INF/resources/shared/quickstart-page.js | Same SETUP.ready-gated initialization as shared. |
| use-cases/maintenance-scheduling/src/main/resources/META-INF/resources/shared/platform-integration.js | Template copy of platform integration logic. |
| use-cases/maintenance-scheduling/src/main/resources/META-INF/resources/shared/environment.js | Removes old environment logic. |
| use-cases/maintenance-scheduling/src/main/resources/META-INF/resources/index.html | Switches to platform integration + hides body until init. |
| use-cases/flight-crew-scheduling/src/main/resources/META-INF/resources/shared/quickstart-page.js | Same SETUP.ready-gated initialization as shared. |
| use-cases/flight-crew-scheduling/src/main/resources/META-INF/resources/shared/platform-integration.js | Template copy of platform integration logic. |
| use-cases/flight-crew-scheduling/src/main/resources/META-INF/resources/shared/environment.js | Removes old environment logic. |
| use-cases/flight-crew-scheduling/src/main/resources/META-INF/resources/index.html | Switches to platform integration + hides body until init. |
| use-cases/conference-scheduling/src/main/resources/META-INF/resources/shared/quickstart-page.js | Same SETUP.ready-gated initialization as shared. |
| use-cases/conference-scheduling/src/main/resources/META-INF/resources/shared/platform-integration.js | Template copy of platform integration logic. |
| use-cases/conference-scheduling/src/main/resources/META-INF/resources/shared/environment.js | Removes old environment logic. |
| use-cases/conference-scheduling/src/main/resources/META-INF/resources/index.html | Switches to platform integration + hides body until init. |
| use-cases/bed-allocation/src/main/resources/META-INF/resources/shared/quickstart-page.js | Same SETUP.ready-gated initialization as shared. |
| use-cases/bed-allocation/src/main/resources/META-INF/resources/shared/platform-integration.js | Template copy of platform integration logic. |
| use-cases/bed-allocation/src/main/resources/META-INF/resources/shared/environment.js | Removes old environment logic. |
| use-cases/bed-allocation/src/main/resources/META-INF/resources/index.html | Switches to platform integration + hides body until init. |
| scripts/run-all-quarkus.java | Adds a Playwright-based script to run each Quarkus project and take screenshots. |
| scripts/DeployToPlatform.java | Adds a script to inject deploy plugin config into pom.xml, deploy, then restore. |
Review details
- Files reviewed: 25/26 changed files
- Comments generated: 13
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
The new scripts have concrete correctness/usability issues (video handling in Playwright runner and incorrect DeployToPlatform usage paths / SNAPSHOT blocking) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
.github/scripts/DeployToPlatform.java:33
- The Javadoc example path should point to ".github/scripts/DeployToPlatform.java" (currently missing the leading dot).
* Example:
* jbang ./github/scripts/DeployToPlatform.java --module use-cases/maintenance-scheduling \
* --url https://sandbox.timefold.dev \
.github/scripts/DeployToPlatform.java:211
- The CLI usage example printed by usage() should use the correct ".github/scripts/DeployToPlatform.java" path so copy/paste works.
Example:
jbang scripts/DeployToPlatform.java --module use-cases/maintenance-scheduling \\
--url https://sandbox.timefold.dev \\
.github/scripts/DeployToPlatform.java:198
- The CLI usage text printed by usage() points to "scripts/DeployToPlatform.java", but the file lives at ".github/scripts/DeployToPlatform.java"; this makes the help output misleading.
System.err.println("""
Usage: jbang scripts/DeployToPlatform.java --url <platformUrl> --key <key> --tenants <t1,t2,...> [options]
- Files reviewed: 25/26 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The new platform init handshake can currently hang indefinitely without a timeout, and the deploy script help text contains incorrect paths that will mislead users.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
visualizations/shared/platform-integration.js:55
- SETUP.ready currently waits indefinitely for an "init" postMessage. If the parent never sends it (or it gets blocked), QuickstartPage never creates SolverClient / loads data and the UI stays disabled forever; add a timeout so the page fails fast with a visible error.
.github/scripts/DeployToPlatform.java:32
- The Javadoc example path is missing the leading dot, so copy/paste will fail from the repo root ("./github/..." vs the actual ".github/").
* jbang ./github/scripts/DeployToPlatform.java --module use-cases/maintenance-scheduling \
.github/scripts/DeployToPlatform.java:197
- The usage text printed on --help/errors points to "scripts/DeployToPlatform.java", but the file lives under ".github/scripts"; this makes the help output misleading.
Usage: jbang scripts/DeployToPlatform.java --url <platformUrl> --key <key> --tenants <t1,t2,...> [options]
.github/scripts/DeployToPlatform.java:210
- The help output example command uses "scripts/DeployToPlatform.java" instead of the actual ".github/scripts" path, so it won't work when copy/pasted from the repo root.
jbang scripts/DeployToPlatform.java --module use-cases/maintenance-scheduling \\
- Files reviewed: 25/26 changed files
- Comments generated: 0 new
- Review effort level: Lite
dd90920 to
9f49ea7
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The new platform handshake can leave the UI stuck indefinitely if the init message is missing/blocked, and the deploy script’s printed usage/examples contain incorrect paths.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
visualizations/shared/platform-integration.js:36
- When
onPlatformis present but the parent never sends a validinitmessage (or it’s filtered out byisTrustedOrigin()),SETUP.readyneither resolves nor rejects, leaving the UI stuck with solve/analyze disabled and no error toast. Consider adding a timeout that rejects the promise so QuickstartPage can surface a clear initialization error.
.github/scripts/DeployToPlatform.java:35
- The Javadoc example uses
jbang ./github/scripts/DeployToPlatform.java, but the file actually lives under./.github/scripts/DeployToPlatform.java(as also shown earlier in the same Javadoc). This makes the example command fail if copied verbatim.
* Example:
* jbang ./github/scripts/DeployToPlatform.java --module use-cases/maintenance-scheduling \
* --url https://sandbox.timefold.dev \
* --key maintenance-scheduling-template \
* --tenants ae226da1-5aea-4aba-93fc-8f911f37aa23
.github/scripts/DeployToPlatform.java:200
- The
usage()output referencesscripts/DeployToPlatform.java, but the script is stored in./.github/scripts/DeployToPlatform.java. Updating the printed usage/example paths will prevent copy/paste errors.
System.err.println("""
Usage: jbang scripts/DeployToPlatform.java --url <platformUrl> --key <key> --tenants <t1,t2,...> [options]
Required:
--url Timefold platform URL, e.g. https://sandbox.timefold.dev
- Files reviewed: 25/26 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/robustness issues in the new helper scripts and platform handshake flow that should be addressed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
.github/scripts/DeployToPlatform.java:32
- The Javadoc example path uses "./github/scripts/DeployToPlatform.java" but the script lives under ".github/scripts" (leading dot). As written, copy/pasting the example will fail.
* jbang ./github/scripts/DeployToPlatform.java --module use-cases/maintenance-scheduling \
.github/scripts/DeployToPlatform.java:197
- The usage() help text still points to "scripts/DeployToPlatform.java", but the actual path is ".github/scripts/DeployToPlatform.java". This makes the CLI help misleading for anyone running the script from the repo root.
Usage: jbang scripts/DeployToPlatform.java --url <platformUrl> --key <key> --tenants <t1,t2,...> [options]
- Files reviewed: 25/26 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a couple of concrete reliability/security footguns (page can remain blank due to hidden body on script errors; platform origin allow-list is non-configurable/empty by default; Playwright video handling can NPE) that should be addressed before merging.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
visualizations/shared/index.template.html:28
- Hiding the entire by default can leave the page permanently blank if any script fails to load/execute (CDN outage, JS error, blocked third-party script). Since the Solve/Analyze buttons are already disabled until SETUP.ready resolves, consider keeping the body visible by default and only gating the interactive controls.
visualizations/shared/platform-integration.js:14 - ALLOWED_PARENT_ORIGINS is currently an empty list, meaning the init handshake only accepts same-origin parents. If the platform embeds this UI cross-origin (common for iframes), the init message will be ignored and the page will fail to initialize. Consider making the allow-list configurable at runtime (for example via a global set by the embedding page) so deployments can securely set the expected platform origins without editing this file.
- Files reviewed: 25/26 changed files
- Comments generated: 0 new
- Review effort level: Lite
e594e8a to
89e015a
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness/operational issues in the new scripts and platform integration (notably null-safety in Playwright video handling and an empty allowed-origin list that blocks cross-origin platform embedding).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 25/26 changed files
- Comments generated: 7
- Review effort level: Lite
Description of the change
Checklist
Development
Code Review