databrickslabs / databrickslabs/ontos

[Improvement]: Comments: resolve / reopen threads

Open
#784 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type/improvement
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_at and resolved_by, orthogonal to status
  • 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.