feat(op-work-packages): WorkPackage model foundation#64
Conversation
Fill the op-work-packages stub with the WorkPackage entity — OpenProject's headline unit of work (Issue-equivalent), converging with Redmine's Issue on canonical concept project_work_item (0x0102). Mirrors the op-journals house pattern (plain model struct + newtype + new/builder + tests) and wires into op-core's trait system: - WorkPackage struct: the core Rails columns — subject, description (Formattable), project/type/status/priority/author FKs, optional assignee/responsible/parent, start/due dates, estimated_hours, done_ratio, lock_version, timestamps. - DoneRatio newtype: whole-percent 0..=100, clamped on construction. - new(required fields) + with_* builder; domain helpers is_complete, is_child, has_assignee, is_overdue(today). - impls op_core Identifiable / Timestamped / ProjectScoped / Lockable / Entity (TABLE_NAME="work_packages", TYPE_NAME="WorkPackage"). 6 unit tests (defaults, DoneRatio clamp, builder, is_overdue logic, trait impls, JSON round-trip). fmt clean; `cargo clippy -p op-work-packages` reports 0 warnings in this crate's own code; green on toolchain 1.95. (`-D warnings` transitively surfaces pre-existing op-core lints already on main — op-core untouched here, out of scope.) Follow-ups: op-canon class_id wiring; WorkPackage write-model + service.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 923b173a87
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Status FK (New / In progress / Closed / …). | ||
| pub status_id: Id, | ||
| /// Priority FK (Low / Normal / High / …). | ||
| pub priority_id: Id, |
There was a problem hiding this comment.
When a work package is created or read without a priority, this model cannot represent it because priority_id is a required Id. The existing database row uses Option<i64> for priority_id (crates/op-db/src/work_packages.rs:21), and the API create DTO accepts priority_id: Option<Id> (crates/op-api/src/handlers/work_packages.rs:291), so persisted NULL-priority rows would either fail conversion/deserialization into this new foundation model or force callers to invent a synthetic priority id. Make this field and the constructor parameter optional to match the existing data shape.
Useful? React with 👍 / 👎.
| /// not yet complete. A WP with no due date, or one already done, is never | ||
| /// overdue. | ||
| pub fn is_overdue(&self, today: NaiveDate) -> bool { | ||
| self.due_date.is_some_and(|due| due < today) && !self.is_complete() |
There was a problem hiding this comment.
For a work package whose status is closed but whose done_ratio is less than 100, this helper reports it as overdue. Existing query code treats overdue work packages as past-due and open (crates/op-queries/src/builder.rs:547-552), while status closedness is tracked separately from the percentage (crates/op-models/src/status.rs:32-49), so closed/done state is not equivalent to done_ratio == 100. Consumers using this helper for overdue badges or notifications will surface closed items as overdue unless the check uses/accepts status closedness instead of is_complete().
Useful? React with 👍 / 👎.
Address the two codex P2 review comments on #64. 1. priority_id is nullable in OpenProject (op-db WorkPackageRow carries Option<i64>, the op-api DTO Option<Id>), but the foundation model required it — a NULL-priority row couldn't be represented. Make priority_id Option<Id>; drop it from the required new(...) params (now subject, project_id, type_id, status_id, author_id) and add with_priority(). 2. is_overdue keyed off done_ratio == 100, but overdue-ness tracks OPEN status, not completion (op-queries overdue() == overdue().open(); a Status carries an is_closed flag distinct from done_ratio). The helper now takes status_is_closed and returns open AND past-due, dropping the done_ratio coupling. Both are safe API changes — op-work-packages was a fresh stub with no consumers yet. Tests updated (priority defaults None + with_priority; is_overdue rewritten around open/closed status). 6 tests pass; fmt + this crate's clippy clean on 1.95.
Add the WorkPackage write path on top of the #64/#65 model, mirroring the op-journals service house pattern. - NewWorkPackage — the create write-model / input DTO (Deserialize, so an API layer can parse a request body). Required NOT NULL columns + optional priority/assignee/parent/description; to_work_package() maps it onto a fresh domain entity. - WorkPackageStore — async persistence port (insert / get / list_for_project). - WorkPackageService — create (validate subject non-blank -> build -> persist, returns the entity with its assigned id), find (NotFound), list_for_project. - MemoryWorkPackageStore — in-memory impl for tests/prototyping. - WorkPackageError + WorkPackageResult (thiserror). 6 service tests (persist+id, blank-subject rejection, optional-field mapping, NotFound, project filter, write-model deserialize/default) + the 6 model tests = 12 pass. fmt clean; op-work-packages clippy-clean (0 warnings in this crate; the transitive -D warnings op-core lints are pre-existing, untouched). Deps: async-trait + thiserror (runtime), tokio (dev, for #[tokio::test]).
2 participants
What
Fills the
op-work-packagesstub with theWorkPackageentity — OpenProject's headline unit of work (theIssue-equivalent), converging with Redmine'sIssueon the canonical conceptproject_work_item(0x0102).Mirrors the
op-journalshouse pattern (plain model struct + newtype +new/builder + tests) and wires intoop-core's trait system:WorkPackage— the core Rails columns:subject,description(Formattable),project_id/type_id/status_id/priority_id/author_idFKs, optionalassigned_to_id/responsible_id/parent_id,start_date/due_date,estimated_hours,done_ratio,lock_version, timestamps.DoneRationewtype — whole-percent0..=100, clamped on construction.new(required fields)+with_*builder; domain helpersis_complete,is_child,has_assignee,is_overdue(today).op_coreIdentifiable/Timestamped/ProjectScoped/Lockable/Entity(TABLE_NAME = "work_packages",TYPE_NAME = "WorkPackage").Tests
6 unit tests: defaults,
DoneRatioclamp, builder relations,is_overduelogic (past-due ∧ incomplete only), theop-coretrait impls, and a JSON round-trip.Gate
cargo +1.95 fmt --checkclean;cargo clippy -p op-work-packagesreports 0 warnings in this crate's own code;test -p op-work-packages6 pass.clippy -p op-work-packages -D warningstransitively surfaces pre-existingop-corelints (should_implement_traitinpagination.rs, an unused import / unreachable patterns) that are already onmain—op-coreis untouched by this PR. Happy to clean those up in a separateop-corePR if you'd like.Branch note
Follows the merged #63; branch restarted from the new
main(6bc77fd) per the merged-branch rule, single follow-up commit on top, no overlap with the walledodoo-rs-transcodebranch.Follow-ups:
op-canonclass_idwiring onWorkPackage; the write-model +CreateService(mirroringjournal_service).🤖 Generated with Claude Code
Generated by Claude Code