Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive design documentation for a Tags system on a legacy branch. The documentation provides a foundation for implementing user-facing tags that enable better organization and discovery of entities across the system. The design is intentionally lightweight and provisional given the legacy nature of the target branch.
Changes:
- Added PRD defining product goals, use cases, scope, and success metrics for a Tags feature
- Added RFC proposing technical architecture including data model, API design, and implementation considerations
- Added README documenting the purpose and structure of the design artifacts
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/designs/tags/README.md | Introduces the design documentation folder with overview of included documents and usage guidelines |
| docs/designs/tags/PRD.md | Defines product requirements including problem statement, goals, scope, UX/functional requirements, and future vision for Saved Views |
| docs/designs/tags/RFC.md | Proposes technical implementation with data model, API design, backend/frontend behavior, and migration considerations |
| docs/designs/tags/PR.md | Provides a PR template/draft with summary, rationale, scope boundaries, and reviewer checklist |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
dosubot
bot
added
the
size:L
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Tag CRUD | ||
|
|
||
| - `GET /v1/tags?workspace_id=...` | ||
| - `POST /v1/tags` | ||
| - `PATCH /v1/tags/{tag_id}` | ||
| - `DELETE /v1/tags/{tag_id}` | ||
|
|
||
| ### Tag Assignment | ||
|
|
||
| - `POST /v1/{entity_type}/{entity_id}/tags` (assign) | ||
| - `DELETE /v1/{entity_type}/{entity_id}/tags/{tag_id}` (unassign) | ||
| - `GET /v1/{entity_type}/{entity_id}/tags` | ||
|
|
||
| ### Entity List Filtering | ||
|
|
||
| - Add query params: | ||
| - `tag_ids=...` or `tags=...` | ||
| - optional `tag_match=and|or` (default TBD) | ||
|
|
||
| ## 5. Backend Behavior | ||
|
|
||
| - Normalize names on create/rename. | ||
| - Reject duplicate names in same workspace. | ||
| - Enforce workspace isolation on all joins. |
There was a problem hiding this comment.
The API section scopes GET /v1/tags via workspace_id query param, but the other tag CRUD and assignment endpoints don’t specify how workspace is determined (auth context vs request body vs derived from entity). Please clarify the workspace scoping for POST/PATCH/DELETE and for {entity_type}/{entity_id}/tags so consumers know what to send and how isolation is enforced.
| ### Tag CRUD | |
| - `GET /v1/tags?workspace_id=...` | |
| - `POST /v1/tags` | |
| - `PATCH /v1/tags/{tag_id}` | |
| - `DELETE /v1/tags/{tag_id}` | |
| ### Tag Assignment | |
| - `POST /v1/{entity_type}/{entity_id}/tags` (assign) | |
| - `DELETE /v1/{entity_type}/{entity_id}/tags/{tag_id}` (unassign) | |
| - `GET /v1/{entity_type}/{entity_id}/tags` | |
| ### Entity List Filtering | |
| - Add query params: | |
| - `tag_ids=...` or `tags=...` | |
| - optional `tag_match=and|or` (default TBD) | |
| ## 5. Backend Behavior | |
| - Normalize names on create/rename. | |
| - Reject duplicate names in same workspace. | |
| - Enforce workspace isolation on all joins. | |
| ### Workspace scoping | |
| - All tag data is tenant-scoped by `workspace_id`. | |
| - Clients MUST NOT be able to read, create, update, or assign tags across workspaces. | |
| - The server derives `workspace_id` from: | |
| - The explicit `workspace_id` query parameter (for `GET /v1/tags`), and | |
| - The authenticated workspace context and/or target entity (for all other endpoints). | |
| ### Tag CRUD | |
| - `GET /v1/tags?workspace_id=...` | |
| - `workspace_id` query param is REQUIRED and determines which workspace’s tags are listed. | |
| - The caller MUST be authorized for the requested `workspace_id`. | |
| - `POST /v1/tags` | |
| - `workspace_id` is derived from the authenticated workspace context; the request body MUST NOT override it. | |
| - The new tag is always created in the caller’s current workspace. | |
| - `PATCH /v1/tags/{tag_id}` | |
| - `tag_id` is looked up in the caller’s workspace only. | |
| - Requests that attempt to operate on a tag outside the caller’s workspace MUST be rejected. | |
| - `DELETE /v1/tags/{tag_id}` | |
| - `tag_id` is resolved within the caller’s workspace only. | |
| - Cross-workspace deletes MUST NOT be allowed. | |
| ### Tag Assignment | |
| - `POST /v1/{entity_type}/{entity_id}/tags` (assign) | |
| - The `{entity_type}/{entity_id}` is first resolved to a concrete entity and its `workspace_id`. | |
| - All `tag_id`s in the request MUST belong to the same `workspace_id` as the entity. | |
| - Assignments that would cross workspace boundaries MUST be rejected. | |
| - `DELETE /v1/{entity_type}/{entity_id}/tags/{tag_id}` (unassign) | |
| - The entity and `tag_id` are both resolved and validated to belong to the same `workspace_id`. | |
| - `GET /v1/{entity_type}/{entity_id}/tags` | |
| - Returns only tags from the entity’s `workspace_id`. | |
| ### Entity List Filtering | |
| - Add query params: | |
| - `tag_ids=...` or `tags=...` | |
| - optional `tag_match=and|or` (default TBD) | |
| - filters are always evaluated within the current `workspace_id` context | |
| ## 5. Backend Behavior | |
| - Normalize names on create/rename. | |
| - Reject duplicate names in same workspace. | |
| - Enforce workspace isolation on all joins and API operations: | |
| - All lookups for `tags` and `entity_tags` MUST include `workspace_id` in the predicate. | |
| - `GET /v1/tags` MUST require `workspace_id` and restrict results to that workspace. | |
| - `POST/PATCH/DELETE /v1/tags` MUST derive `workspace_id` from the authenticated context and MUST NOT allow the client to change it. | |
| - `{entity_type}/{entity_id}/tags` endpoints MUST derive `workspace_id` from the target entity and validate that all referenced tags share the same `workspace_id`. |
| - `tag_ids=...` or `tags=...` | ||
| - optional `tag_match=and|or` (default TBD) |
There was a problem hiding this comment.
The filtering proposal lists both tag_ids and tags as possible query params and leaves tag_match default as TBD. To avoid ambiguous client/server behavior, pick a single canonical parameter name and specify the expected encoding (e.g., comma-separated vs repeated params) plus the default match semantics.
| - `tag_ids=...` or `tags=...` | |
| - optional `tag_match=and|or` (default TBD) | |
| - `tag_ids` (repeated): e.g. `?tag_ids=<tag_id_1>&tag_ids=<tag_id_2>` | |
| - optional `tag_match=and|or` (default `and`; `and` = entity must have all listed `tag_ids`, `or` = entity must have at least one) |
|
|
||
| ## 8. Security and Auditing | ||
|
|
||
| - Authorization follows existing entity-level permissions. |
There was a problem hiding this comment.
Security section says authorization follows existing entity-level permissions, but tag CRUD operations are not inherently entity-scoped. Please document what permission model applies to tag CRUD (e.g., workspace-level roles) and how it interacts with workspace-scoped uniqueness/isolation.
| - Authorization follows existing entity-level permissions. | |
| - Tag CRUD (`/v1/tags*`) is authorized via workspace-level permissions (e.g., caller must have permission to manage tags within the target `workspace_id`). | |
| - Tag assignment/unassignment endpoints require both: (a) authorization to modify the target entity under existing entity-level permissions, and (b) authorization in the entity’s workspace to modify its tags. | |
| - All tag operations are constrained to the caller’s authorized workspace; `workspace_id` is required/derived from context, and tag uniqueness and lookups are always enforced per workspace to prevent cross-workspace access. |
ref: tags branch without PR