feat(hosts): reusable color-coded host tags#667
Conversation
Create reusable, color-coded tags and assign multiple to each host; the host card is tinted with the first tag's color (a soft, glassy fill) and shows one chip per tag. Backend: HostTag model + hosts_tags_association M2M (migration a821c186b426), tag CRUD/operation/router at /api/host/tags (RBAC via the hosts resource), and tag_ids/tags on host create/modify/response. Tags order_by id for a deterministic tint color; bulk host delete clears tag links explicitly (SQLite has no FK cascade). Frontend: theme-aware palette, react-query hooks, a tag picker (assign + inline create/edit/delete) in the host form, and card/row coloring across grid, list, and table views. The orval client was hand-extended because codegen is broken on Node 22. Tests: unit (schema validation) + API/e2e (CRUD, delete-tag cascade, dedup, invalid id, clone, reorder, bulk-delete). Verified on SQLite, MySQL/MariaDB, and PostgreSQL/TimescaleDB.
WalkthroughThis PR introduces host tagging: a new ChangesHost Tagging Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant HostRouter
participant HostOperation
participant HostCRUD
participant Database
Client->>HostRouter: POST /api/host/tags {name, color}
HostRouter->>HostOperation: create_host_tag(new_tag, admin)
HostOperation->>HostCRUD: get_host_tag_by_name(name)
HostCRUD->>Database: SELECT host_tags WHERE name
HostOperation->>HostCRUD: create_host_tag(new_tag)
HostCRUD->>Database: INSERT host_tags
HostOperation-->>HostRouter: HostTag
HostRouter-->>Client: 201 Created
Client->>HostRouter: POST /api/hosts {tag_ids}
HostRouter->>HostOperation: create_host(new_host)
HostOperation->>HostOperation: validate_host_tags(tag_ids)
HostOperation->>HostCRUD: resolve_host_tags(tag_ids)
HostCRUD->>Database: SELECT host_tags WHERE id IN tag_ids
HostOperation->>HostCRUD: create_host(new_host)
HostCRUD->>Database: INSERT hosts, hosts_tags_association
HostOperation-->>HostRouter: Host (with tags)
HostRouter-->>Client: 201 Created
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Request timed out after 900000ms (requestId=942f6bc3-e3c9-4d82-aa0c-ab74bf19e866) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
dashboard/src/features/hosts/components/hosts-list.tsx (2)
184-184: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated
tag_idsderivation into a shared helper.The same
host.tags?.map(tag => tag.id).filter((id): id is number => typeof id === 'number') ?? []expression is duplicated inhandleEdit,handleDuplicate, and the drag-reorder payload builder. Extracting a small helper (e.g.getHostTagIds(host: BaseHost): number[]) would reduce duplication and keep the three call sites in sync if this logic ever changes.♻️ Proposed refactor
+const getHostTagIds = (host: BaseHost): number[] => host.tags?.map(tag => tag.id).filter((id): id is number => typeof id === 'number') ?? [] + const handleEdit = (host: BaseHost) => { ... - tag_ids: host.tags?.map(tag => tag.id).filter((id): id is number => typeof id === 'number') ?? [], + tag_ids: getHostTagIds(host),(apply similarly at the
handleDuplicateand drag-reorder call sites)Also applies to: 372-372, 576-576
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/features/hosts/components/hosts-list.tsx` at line 184, The `tag_ids` extraction logic is duplicated across `handleEdit`, `handleDuplicate`, and the drag-reorder payload builder in `hosts-list.tsx`. Create a shared helper like `getHostTagIds(host: BaseHost): number[]` that performs the current `host.tags?.map(...).filter(...) ?? []` derivation, then replace each repeated inline expression with that helper so all three call sites stay consistent.
984-987: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTint computation duplicated with
sortable-host.tsx.
getTagColorStyle(host.tags[0].color).tinthere mirrors the identical "use first tag's tint" logic inSortableHost(dashboard/src/features/hosts/components/sortable-host.tsx:48-49). Consider a small shared helper (e.g.getHostRowTint(host, selected)) so the grid/list/card views stay consistent if the tinting rule changes later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/features/hosts/components/hosts-list.tsx` around lines 984 - 987, The host row tint logic is duplicated between the hosts list and SortableHost, so extract the “first tag tint unless selected” behavior into a shared helper such as getHostRowTint(host, selected) and use it from the rowClassName logic in hosts-list and the equivalent tint code in sortable-host. Keep the helper responsible for checking selection and tags, then return the tint from getTagColorStyle so both views stay consistent if the rule changes.dashboard/src/service/hostTags.ts (1)
9-13: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFragile substring-based cache invalidation.
Matching query keys by
.includes('host')is brittle — any query key that happens to contain "host" as a substring gets invalidated (over-invalidation/perf cost), and if the actual host-list key is ever renamed to something without "host" in it, this silently stops working with no compile-time warning.Prefer explicitly invalidating the known query keys (e.g.,
hostTagsQueryKeyplus the host list key used inhosts-list.tsx,['getGetHostsQueryKey']) instead of pattern matching.♻️ Proposed fix
-const invalidateHostAndTagQueries = (queryClient: ReturnType<typeof useQueryClient>) => - queryClient.invalidateQueries({ - predicate: query => - query.queryKey.some(part => typeof part === 'string' && part.toLowerCase().includes('host')), - }) +const invalidateHostAndTagQueries = (queryClient: ReturnType<typeof useQueryClient>) => + Promise.all([ + queryClient.invalidateQueries({ queryKey: hostTagsQueryKey }), + queryClient.invalidateQueries({ queryKey: ['getGetHostsQueryKey'], exact: true }), + ])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboard/src/service/hostTags.ts` around lines 9 - 13, The cache invalidation in invalidateHostAndTagQueries relies on a brittle substring match against query keys, which can over-invalidate unrelated queries and break silently if keys change. Update this helper to explicitly invalidate the known host-related keys used by the dashboard, including hostTagsQueryKey and the host list query key from hosts-list.tsx (the getHosts query key), instead of using a predicate with includes('host'). Ensure the queryClient.invalidateQueries call targets those concrete keys directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dashboard/src/features/hosts/components/host-tag-picker.tsx`:
- Around line 125-130: The delete action in handleRemove should require explicit
user confirmation before calling removeTag.mutate, since the trash icon
permanently deletes a tag and can affect other hosts. Update host-tag-picker’s
delete flow around handleRemove and the trash button to open the app’s
AlertDialog confirmation pattern instead of deleting immediately, then only
invoke removeTag.mutate(id) after confirm; also disable the delete control while
removeTag.isPending to prevent duplicate requests.
---
Nitpick comments:
In `@dashboard/src/features/hosts/components/hosts-list.tsx`:
- Line 184: The `tag_ids` extraction logic is duplicated across `handleEdit`,
`handleDuplicate`, and the drag-reorder payload builder in `hosts-list.tsx`.
Create a shared helper like `getHostTagIds(host: BaseHost): number[]` that
performs the current `host.tags?.map(...).filter(...) ?? []` derivation, then
replace each repeated inline expression with that helper so all three call sites
stay consistent.
- Around line 984-987: The host row tint logic is duplicated between the hosts
list and SortableHost, so extract the “first tag tint unless selected” behavior
into a shared helper such as getHostRowTint(host, selected) and use it from the
rowClassName logic in hosts-list and the equivalent tint code in sortable-host.
Keep the helper responsible for checking selection and tags, then return the
tint from getTagColorStyle so both views stay consistent if the rule changes.
In `@dashboard/src/service/hostTags.ts`:
- Around line 9-13: The cache invalidation in invalidateHostAndTagQueries relies
on a brittle substring match against query keys, which can over-invalidate
unrelated queries and break silently if keys change. Update this helper to
explicitly invalidate the known host-related keys used by the dashboard,
including hostTagsQueryKey and the host list query key from hosts-list.tsx (the
getHosts query key), instead of using a predicate with includes('host'). Ensure
the queryClient.invalidateQueries call targets those concrete keys directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2cd94b48-ba0e-4956-bd23-c58b4ef7a899
📒 Files selected for processing (17)
app/db/crud/host.pyapp/db/migrations/versions/a821c186b426_add_host_tags.pyapp/db/models.pyapp/models/host.pyapp/operation/host.pyapp/routers/host.pydashboard/src/constants/hostTagColors.tsdashboard/src/features/hosts/components/host-tag-picker.tsxdashboard/src/features/hosts/components/hosts-list.tsxdashboard/src/features/hosts/components/sortable-host.tsxdashboard/src/features/hosts/components/use-hosts-list-columns.tsxdashboard/src/features/hosts/dialogs/host-modal.tsxdashboard/src/features/hosts/forms/host-form.tsdashboard/src/service/api/index.tsdashboard/src/service/hostTags.tstests/api/test_host_tag.pytests/test_host_tags_unit.py
| const handleRemove = (id: number) => { | ||
| removeTag.mutate(id, { | ||
| onSuccess: () => onChange(value.filter(x => x !== id)), | ||
| onError: () => toast.error(t('hostTags.deleteFailed', { defaultValue: 'Failed to delete tag' })), | ||
| }) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add a confirmation step before deleting a tag.
handleRemove immediately calls removeTag.mutate(id) with no confirmation. Unlike the chip's "X" (which only toggles selection locally via toggle), this trash icon permanently deletes the tag record, which per the PR description cascades and un-links it from every host that currently uses it. A stray click inside a host's edit form can silently affect unrelated hosts with no way to undo. Also consider disabling the delete button while removeTag.isPending to avoid duplicate delete requests from rapid clicks.
🛡️ Proposed fix: confirm before delete
- const handleRemove = (id: number) => {
- removeTag.mutate(id, {
- onSuccess: () => onChange(value.filter(x => x !== id)),
- onError: () => toast.error(t('hostTags.deleteFailed', { defaultValue: 'Failed to delete tag' })),
- })
- }
+ const handleRemove = (id: number, name: string) => {
+ if (removeTag.isPending) return
+ if (!window.confirm(t('hostTags.confirmDelete', { name, defaultValue: `Delete tag "${name}"? It will be removed from all hosts.` }))) {
+ return
+ }
+ removeTag.mutate(id, {
+ onSuccess: () => onChange(value.filter(x => x !== id)),
+ onError: () => toast.error(t('hostTags.deleteFailed', { defaultValue: 'Failed to delete tag' })),
+ })
+ }A dedicated AlertDialog (consistent with the app's existing bulk-action confirmation pattern) would be preferable to window.confirm for styling consistency.
Also applies to: 194-199
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dashboard/src/features/hosts/components/host-tag-picker.tsx` around lines 125
- 130, The delete action in handleRemove should require explicit user
confirmation before calling removeTag.mutate, since the trash icon permanently
deletes a tag and can affect other hosts. Update host-tag-picker’s delete flow
around handleRemove and the trash button to open the app’s AlertDialog
confirmation pattern instead of deleting immediately, then only invoke
removeTag.mutate(id) after confirm; also disable the delete control while
removeTag.isPending to prevent duplicate requests.
2 participants
Summary
Adds reusable, color-coded host tags. You can create a palette of named tags, assign multiple tags to any host, and the host is tinted with its first tag's color (a soft, glassy fill) with one chip shown per tag. Tags are managed inline from the host form (create / edit / delete) and render consistently across the grid, list, and table host views.
Motivation: on a large fleet, hosts are hard to scan visually. Tags give a lightweight, user-defined way to group and color hosts (by region, provider, purpose, etc.) without adding rigid schema.
What's included
Backend
HostTagmodel +hosts_tags_associationmany-to-many table (Alembic migrationa821c186b426)./api/host/tags(GET/POST) and/api/host/tags/{tag_id}(PUT/DELETE), gated by the existinghostsRBAC resource.tag_idsaccepted on host create/modify;tagsreturned on host responses.Frontend
Tests
Database compatibility
Verified end to end on SQLite, MySQL / MariaDB, and PostgreSQL / TimescaleDB.
Notes for reviewers
dashboard/src/service/api/index.tsis Orval-generated, butbun run gen:apicurrently crashes on Node >= 22 (bundled Spectral/ajv throws at import). The few new types were hand-added to match what a regen would produce; they are self-healing on the next real regen on Node <= 20 / in CI.Checklist
ruff check .passes (lint clean)ruff formatclean on all changed filesdev; migration chains after dev's head (b6c9d0e1f2a3), soalembic headsshows a single headSummary by CodeRabbit
New Features
Bug Fixes