fix(op-work-packages): optional priority_id + status-based is_overdue#65
Merged
Conversation
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.
AdaWorldAPI
pushed a commit
that referenced
this pull request
Jul 1, 2026
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]).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the two codex P2 review comments on #64.
1.
priority_idis nullable (#64 comment)The persisted row carries
priority_id: Option<i64>(op-db/src/work_packages.rs:21) and the API DTO passes it through as optional — but the foundation model required it, so a NULL-priority row couldn't round-trip. Fix:priority_id: Option<Id>; dropped from the required constructor (newis nowsubject, project_id, type_id, status_id, author_id) with awith_priority()builder for the explicit case.2. Overdue tracks open status, not completion (#64 comment)
is_overduekeyed offdone_ratio == 100, butop-queries'overdue()isoverdue().open(), andop-models::Statuscarries anis_closedflag distinct fromdone_ratio(a closed status can sit below 100%). So the old helper flagged closed-but-<100% items as overdue. Fix: the helper now takesstatus_is_closedand returnsopen ∧ past-due, dropping thedone_ratiocoupling entirely — the caller supplies closedness from theStatusrecord.Safety + tests
Both are API changes, but
op-work-packageswas a fresh stub with no consumers yet, so nothing breaks. Tests updated:priority_iddefaultsNone+ awith_priorityassertion;is_overduerewritten around open/closed status (incl. the key case — an open, past-due, 100%-done WP is still overdue). 6 tests pass;fmt --check+ this crate'sclippyclean on 1.95.Branch note
Follows the merged #64; branch restarted from the new
main(bbc6bf3) per the merged-branch rule, single follow-up commit, no overlap with the walledodoo-rs-transcodebranch.🤖 Generated with Claude Code
Generated by Claude Code