databrickslabs / databrickslabs/ontos
[Improvement]: Comments: resolve / reopen threads
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 212
- Forks
- 71
- Avg merge
- 4d 10h
- Merged PRs (30d)
- 43
Description
Parent PRD
#783
What to build
Add thread-level resolve / reopen to comments. Introduce nullable resolved_at (timestamptz) and resolved_by (string) fields on the comments table, orthogonal to the existing ACTIVE/DELETED status. Resolution operates on a root comment only (parent_id IS NULL; replies cannot be resolved independently — the parent_id column arrives with slice #THREAD but resolve/close ships against roots regardless and is forward-compatible). Add resolve/reopen endpoints under the existing comments feature permission. Permission to resolve/reopen is stricter than general comment-write: thread author, an active business owner of the entity, or an admin (owner lookup reuses BusinessOwnersManager.get_owners_for_object). Each transition writes one user-action audit entry and emits one change-log/system entry into the unified timeline. Frontend renders a resolved root collapsed with a "Resolved by X" badge and adds a filter chip to show/hide resolved threads inline (no separate tab) on both the sidebar and embedded timeline variants.
See PRD sections "Solution" (resolve/close), "Implementation decisions" (resolve/close model), and user stories 12–17.
Acceptance criteria
- Migration adds nullable
resolved_atandresolved_by, orthogonal tostatus - Resolve sets both fields; reopen nulls both; round-trip works
- Resolve/reopen allowed for thread author, active entity business owner, or admin; a non-owner writer is rejected
- Attempting to resolve a reply (non-root) is rejected
- Each resolve/reopen writes exactly one audit entry and one timeline system entry at the correct chronological position
- Resolved root renders collapsed with a "Resolved by X" badge on sidebar and embedded variants
- Filter chip shows/hides resolved threads inline; resolved threads are not moved to a separate tab
- Pytest (route + manager) covers permission matrix (author/owner/admin allow, non-owner deny), round-trip, non-root rejection, audit + timeline emission (owner lookup + notifications mocked)
Blocked by
None — can start immediately.
User stories addressed
12, 13, 14, 15, 16, 17
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the existing comments feature endpoints and route/manager pytest coverage, then read the permission path using BusinessOwnersManager.get_owners_for_object. Trace the sidebar and embedded timeline variants for resolved-thread rendering and filtering. Done means the migration, permission matrix, root-only round-trip, audit and timeline entries, and both frontend variants satisfy the listed acceptance criteria.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api, authorization, backend, database, frontend, testing
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100