Repository navigation
chore: migrate task assigning - #1199
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The generated shared UI artifacts (shared/ + index.html) appear out of sync with visualizations/shared conventions/CI enforcement, and the task-assigning pom.xml versioning pattern diverges from other service quickstarts.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Migrates the task-assigning quickstart to the Timefold Service Module architecture (ModelRest endpoints, ModelConvertor, demo-data generator, shared UI shell), aligning it with the other service-based quickstarts in this repository.
Changes:
- Switch task-assigning from a custom REST/solver controller to the Service Module (
ModelRest,ModelConvertor, validator, demo-data generator). - Replace the bespoke UI (
app.js) with the shared service-quickstart UI shell + a new quickstart-specificvisualize.js. - Add/adjust tests for the new REST contract, demo data, constraint verification, validation, and solver manager behavior.
File summaries
| File | Description |
|---|---|
| visualizations/sync.sh | Adds task-assigning to the shared UI sync script’s quickstart list/metadata. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/support/TestHelper.java | Adds test builders/helpers for constructing solver model + DTO inputs. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/solver/TaskAssigningEnvironmentTest.java | Updates environment test to build input locally and optionally run multithreaded. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/solver/TaskAssigningConstraintProviderTest.java | Refactors constraint tests to use TestHelper and updated domain model. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/solver/SolverManagerTest.java | Adds SolverManager-based solving test for the new service model integration. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/service/TaskAssigningValidatorTest.java | Adds tests for domain-specific dataset validation issues. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/rest/TaskAssigningSchedulingResourceIT.java | Removes legacy integration test targeting old endpoints. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/rest/TaskAssigningResourceTest.java | Removes legacy REST test targeting old endpoints. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/rest/TaskAssigningOpenApiValidationTest.java | Adds OpenAPI/Bean Validation compliance test via REST. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/integrationtest/TaskAssigningResourceIT.java | Adds new native integration test against /v1/... service endpoints. |
| use-cases/task-assigning/src/test/java/org/acme/taskassigning/demo/DemoDataBuilderTest.java | Adds tests for the new demo data builder invariants. |
| use-cases/task-assigning/src/main/resources/META-INF/resources/visualize.js | Implements task-assigning specific visualization rendering using the shared page controller. |
| use-cases/task-assigning/src/main/resources/META-INF/resources/shared/timefold-quickstart.css | Adds shared styling copy for service-quickstart shell. |
| use-cases/task-assigning/src/main/resources/META-INF/resources/shared/solver-client.js | Adds shared REST client wrapper for service endpoints. |
| use-cases/task-assigning/src/main/resources/META-INF/resources/shared/quickstart-page.js | Adds shared page controller copy (currently not in sync with visualizations/shared). |
| use-cases/task-assigning/src/main/resources/META-INF/resources/shared/environment.js | Adds a shared file not present in visualizations/shared (likely sync drift). |
| use-cases/task-assigning/src/main/resources/META-INF/resources/shared/color-picker.js | Adds shared color picker copy. |
| use-cases/task-assigning/src/main/resources/META-INF/resources/shared/api-guide.js | Adds shared API guide modal copy. |
| use-cases/task-assigning/src/main/resources/META-INF/resources/index.html | Updates UI shell and script includes for service-style UI (currently appears out of sync with shared template). |
| use-cases/task-assigning/src/main/resources/META-INF/resources/app.js | Removes legacy UI implementation. |
| use-cases/task-assigning/src/main/resources/application.properties | Migrates configuration toward service model metadata and termination settings. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/solver/TaskAssigningConstraintProvider.java | Adds ConstraintInfo/justifications and shared constraint metadata. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/solver/TaskAssigningConstraintGroup.java | Defines constraint group metadata for platform/UI consumption. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/service/validation/TaskAssigningIssue.java | Adds typed validation issue hierarchy for dataset validation. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/service/TaskAssigningValidator.java | Implements domain-specific dataset validation (duplicates, references, assignment consistency). |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/service/TaskAssigningModelConvertor.java | Adds convertor between DTO input/output and solver model, incl. constraint weight overrides. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/rest/TaskAssigningResource.java | Switches REST surface to ModelRest interface (endpoints provided by service module). |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/rest/TaskAssigningDemoResource.java | Removes legacy demo-data endpoint implementation. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/rest/exception/ScheduleSolverExceptionMapper.java | Removes legacy REST exception mapper. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/rest/exception/ScheduleSolverException.java | Removes legacy REST exception type. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/rest/exception/ErrorInfo.java | Removes legacy error DTO. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/rest/DemoDataGenerator.java | Removes legacy demo-data generator (replaced by service demo-data generator). |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/output/TaskAssigningOutputMetrics.java | Adds service output metrics type. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/output/TaskAssigningOutput.java | Adds service model output DTO. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/output/EmployeeOutputDTO.java | Adds employee output DTO for assignments. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/output/AssignedTaskOutputDTO.java | Adds assigned-task output DTO with start time. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/input/TaskTypeInputDTO.java | Adds task-type input DTO with defaults. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/input/TaskInputDTO.java | Adds task input DTO. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/input/TaskAssigningInputMetrics.java | Adds service input metrics type. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/input/TaskAssigningInput.java | Adds service model input DTO. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/input/TaskAssigningConfigOverrides.java | Adds config overrides DTO for constraint weights. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/input/EmployeeInputDTO.java | Adds employee input DTO with defaults and assignment list. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/dto/input/CustomerInputDTO.java | Adds customer input DTO. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/TaskType.java | Removes Jackson identity annotations to fit the service DTO-based model. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/TaskAssigningSolution.java | Implements service SolverModel + input/output metrics + constraint overrides. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/TaskAssigningConstraintProperties.java | Introduces shared constraint constants + bendable score sizes. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/Task.java | Refactors constructors/JSON annotations, adds equals/hashCode, updates shadow sources formatting. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/justification/TaskAssigningJustification.java | Adds constraint justification types for platform score analysis. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/Employee.java | Removes Jackson identity/ignore annotations, adds equals/hashCode. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/Customer.java | Removes Jackson identity annotations. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/demo/TaskAssigningDemoDataGenerator.java | Adds service demo-data generator implementation. |
| use-cases/task-assigning/src/main/java/org/acme/taskassigning/demo/DemoDataBuilder.java | Adds DTO-based demo dataset builder. |
| use-cases/task-assigning/README.md | Updates quickstart documentation for the new architecture and commands. |
| use-cases/task-assigning/pom.xml | Migrates to timefold-solver-service-parent (but version/revision pattern differs from other service quickstarts). |
| README.md | Updates repository overview row to indicate task-assigning is a Service Model quickstart. |
Review details
- Files reviewed: 55/55 changed files
- Comments generated: 3
- 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.
🔵 Needs a closer look
It performs a broad architectural migration (REST API shape, UI shell, build parent, model/DTO/validation/metrics), which warrants final human review to confirm runtime behavior and compatibility across the quickstart and shared UI sync expectations.
Review details
- Files reviewed: 55/55 changed files
- Comments generated: 0 new
- Review effort level: Lite
triceo
left a comment
There was a problem hiding this comment.
LGTM assuming the platform can actually handle this.
There was a problem hiding this comment.
🟡 Changes recommended
The OpenAPI/config override documentation for the unassigned-tasks constraint weight is inconsistent with the implemented score level (medium vs soft), which will mislead API consumers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 55/55 changed files
- Comments generated: 1
- Review effort level: Lite
also update demo data
c2faa90 to
644689b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are a few correctness/documentation issues to address (including a shared UI helper edge-case that can break rendering for larger datasets) before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/justification/TaskAssigningJustification.java:71
- This section header says "Soft constraints", but it also contains
UnassignedTaskJustification, which is tied to the medium-level constraint. Renaming the header to reflect medium+soft will keep the doc consistent with the scoring model.
// ************************************************************************
// Soft constraints
// ************************************************************************
- Files reviewed: 55/55 changed files
- Comments generated: 3
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
There is a concrete NullPointerException risk in Employee.getEndTime() due to unguarded auto-unboxing of a potentially null task end time.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
use-cases/task-assigning/src/main/java/org/acme/taskassigning/domain/Employee.java:101
getEndTime()can throw a NullPointerException when the employee has at least one task whosestartTimeis still null (becauseTask.getEndTime()returnsnullin that case and theLongis auto-unboxed tolong). This can surface in constraints/metrics/justifications if a partially-initialized solution is inspected.
Consider returning 0 when the last task has no end time yet (and also guarding against tasks being null when the no-arg constructor is used).
- Files reviewed: 55/55 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
The shared QuickstartPage controller can throw at runtime when Solve is clicked before demo data has populated loadedSchedule, causing the first status poll to dereference null.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
use-cases/task-assigning/src/main/resources/META-INF/resources/shared/quickstart-page.js:183
getStatus()assumesthis.loadedScheduleis already set whenthis.jobId != null(it dereferences it inmergeModelOutput(),renderScore(), andrenderSchedule()). Insolve(),jobIdcan become non-null before any prior demo-data load has setloadedSchedule(e.g. user clicks Solve immediately after init), causing a runtime error on the first status poll.
Initialize loadedSchedule from the demo-data payload before creating the run (or disable Solve until demo data is loaded).
- Files reviewed: 55/55 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The migrated domain model introduces NullPointerException risks in core scheduling calculations (Task.startTimeSupplier() and Employee.getEndTime()) that should be made null-safe before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 55/55 changed files
- Comments generated: 2
- Review effort level: Lite
Description of the change
migrate the task assignment
Checklist
Development
Code Review