fix(views): make view settings and filters per-user instead of global - #434
fix(views): make view settings and filters per-user instead of global#434vhervatin wants to merge 4 commits into
Conversation
View settings and filters (sort, field sum, page size, visible fields,
collapsed columns, and every filter dimension) were stored in the single
shared `config` JSONB column of the `sprint_views` row, keyed only by
project + context. Any member's change to the "View settings" panel
therefore overwrote that shared row and — reinforced by the project-wide
`view.updated` broadcast — changed what every other member saw.
Views remain shared project entities (name, type, position, and a shared
default config), but personal settings/filters now live in a new
`user_view_configs` table keyed by (view_id, user_id) and are overlaid on
read. Writes from the settings panel target the current user's override
via a new `PUT /projects/{projectId}/views/{viewId}/config` endpoint and
emit no project-wide event, so a member's tweaks stay private. A viewer
(read-only member) can personalize their own view since the endpoint only
requires sprints:read.
The overlay runs after the shared view cache is read, so the cache never
holds one user's config; unauthenticated readers of public projects fall
back to the shared default.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Greptile SummaryThe PR moves view settings and filters from shared view configuration to per-user overrides while retaining shared structural view properties.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported silent reset is prevented by rejecting omitted or null configuration before conversion or persistence.
|
| Filename | Overview |
|---|---|
| services/api/internal/transport/http/handler/view_handler.go | Adds the authenticated personal-config endpoint and now rejects omitted or null config before conversion and persistence. |
| services/api/internal/transport/http/dto/view_dto.go | Defines the personal-config request DTO and conversion into the domain configuration. |
| services/api/internal/service/sprint/view_service.go | Persists user-scoped overrides and overlays them onto per-request view objects. |
| services/api/internal/repository/postgres/view_repository.go | Implements bulk retrieval and upsert operations for per-user view configurations. |
| services/api/migrations/000042_add_user_view_configs.sql | Adds the user-view configuration table, composite key, cascading foreign keys, and user lookup index. |
| apps/web/src/components/projects/interactions/interaction-layout.tsx | Redirects settings, filters, and default-filter seeding from shared view updates to personal configuration updates. |
| apps/web/src/lib/interaction-api.ts | Adds the client call for replacing the authenticated user's personal view configuration. |
Sequence Diagram
sequenceDiagram
participant Web
participant Handler
participant ViewService
participant SharedCache
participant UserConfigRepo
Web->>Handler: "PUT /projects/{projectId}/views/{viewId}/config"
Handler->>Handler: Require authenticated user and non-null config
Handler->>ViewService: SetUserViewConfig(project, view, user, config)
ViewService->>UserConfigRepo: Upsert (view_id, user_id)
UserConfigRepo-->>ViewService: Stored personal override
ViewService-->>Web: View with personal config
Web->>Handler: GET views
Handler->>SharedCache: Read shared views
SharedCache-->>Handler: Shared view definitions
Handler->>UserConfigRepo: Read current user's overrides
UserConfigRepo-->>Handler: Personal configs
Handler-->>Web: Views with personal configs overlaid
Reviews (2): Last reviewed commit: "fix(views): implementa fake repo de inte..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| type UpdateUserViewConfigRequest struct { | ||
| Config *ViewConfigDTO `json:"config"` | ||
| } | ||
|
|
||
| // ToViewConfig maps the request to a domain ViewConfig (empty when omitted). | ||
| func (r UpdateUserViewConfigRequest) ToViewConfig() sprintdom.ViewConfig { | ||
| return toViewConfig(r.Config) |
There was a problem hiding this comment.
When an authenticated member sends an omitted or null config for a non-plugin view, ToViewConfig converts it to an empty configuration and the handler persists it, silently resetting that user's saved settings and filters instead of rejecting the request.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- New
user_view_configstable (000042) keyed(view_id, user_id)withON DELETE CASCADEto views and users, plus auser_idindex. - Per-user overlay on read —
OverlayUserConfigsapplies a user's personal config after the shared cache read inListViews/GetView, leaving nil-user (public) readers on the shared default. - New
PUT /projects/{projectId}/views/{viewId}/config(SetUserViewConfig) writes only the caller's override, checks project ownership, and emits no project-wide event; gated bysprints:read. - Web routing — settings-panel saves and column collapse now go through
updateMyViewConfig; the full effective config is always sent. - Tests — 5 new service unit tests prove isolation (override applies only to its owner, other users keep the shared default, wrong-project rejected, nil user no-op).
I verified the cache-pollution concern directly: cache.Store JSON-serializes and each Get (store.go:29) re-unmarshals fresh objects, so OverlayUserConfigs mutating the returned view slice in place cannot pollute the shared cache — the stored bytes are never touched. The migration numbering is sequential (000041 precedes it) and the test assertions are exact (they genuinely fail without the fix).
Two design-level observations below; neither blocks merge and both are for you/the product owner to weigh, not require a change.
ℹ️ Personal config snapshots don't receive later shared-default changes
The overlay replaces a view's Config wholesale with the user's stored override, and the client stores a full snapshot on every write. Once any member personalizes, a later project-wide change to the shared default (a field added to visible_fields project-wide, a shared group_by/page size set by an owner via the unaffected PATCH /views/{viewId}) will not surface for that member — their snapshot hides the new shared fields. If the intent is "shared default = baseline everyone inherits until they customize," a field-level merge on read (override wins per field, shared fills the rest) would preserve that; a full replace means personalization effectively freezes the config at the moment of the first edit. This is a real product tradeoff worth confirming.
Technical details
# Shared-default divergence after personalization
## Affected sites
- services/api/internal/service/sprint/view_service.go — `OverlayUserConfigs` sets `v.Config = cfg` (wholesale) when an override exists; `SetUserViewConfig` stores the full config it is handed.
- apps/web/src/components/projects/interactions/interaction-layout.tsx — both settings-panel and column-collapse writes send the full `activeView.config` snapshot.
## Required outcome
- Decide and confirm whether per-user config should be a full snapshot (current) or a field-level overlay on top of the evolving shared default. Field-merge keeps future shared-default changes reaching personalized users; snapshot does not.ℹ️ Default-filters init still writes derived defaults to the shared row
interaction-layout.tsx (updateViewById(projectId, view.id, { config }) in the effect that seeds default filters for views with filters === undefined) still targets the shared PATCH /views/{viewId}, not the per-user endpoint. The config it persists is derived from the current user's defaultPageTaskTypeIds, so this path re-introduces exactly the "one member's setup leaks to all" behavior the PR sets out to remove. It only fires when the effective filters is undefined, but once it does it writes one user's derived defaults into the shared row for everyone. This code is untouched by the PR, but it sits directly in tension with the new per-user premise — worth confirming it isn't accidentally re-seeding the shared config.
Technical details
# Per-user derived filters still seeded into the shared row
## Affected sites
- apps/web/src/components/projects/interactions/interaction-layout.tsx — default-filters initialization effect calls `updateViewById` (shared PATCH) with `buildDefaultViewConfig` output.
## Required outcome
- Confirm whether view-default filter seeding should also go to the per-user config now that settings/filters are per-user; if not, no change needed.DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
@pikann what do you think about I developer a gantt view project? A new type view in a projet! |
|
Hi @vhervatin Thanks for the great contribution! The per-user view setting is a solid feature, it nicely solves the issue of users wanting to filter or adjust settings without affecting others. Overall, the PR looks good to me. A couple of suggestions:
Regarding your question about the Gantt view: To answer your thought on making it a new view type, I think it would be a great fit if we implement/integrate it inside the Roadmap view instead. Thanks again for your hard work! <3 |
|
Also, looking ahead, we could add a toggle/switch in the view settings to let users explicitly set a view as "Personal View" |
…o config - test/integration: adiciona GetUserViewConfigs/UpsertUserViewConfig ao fakeViewRepoIT (faltava implementar a interface ViewRepository, quebrando o typecheck do golangci-lint e a suíte de testes) - handler: UpdateMyViewConfig rejeita config nulo/ausente (PUT substitui a config pessoal; um body sem config apagaria as preferências do usuário) - web: seed de filtros default grava no override do próprio usuário via updateMyViewConfig, nunca na linha compartilhada Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Default-filters seeding now targets the per-user override — the init effect in
interaction-layout.tsx(buildDefaultViewConfig+ the previously-flaggedupdateViewById) now callsupdateMyViewConfig, so a member's derived default filters land in their personal config instead of leaking into the sharedsprint_viewsrow. I confirmedbuildDefaultViewConfigspreads the shared config as its base, so the derived-filters snapshot still carries the existing shared fields (no field loss on overlay). This addresses the prior "one member's setup leaks to all" concern. - Nil-config guard on the per-user PUT —
UpdateMyViewConfignow rejects an omitted/nullconfigwith a 400. Since a PUT wholesale-replaces the stored override, this prevents an empty body from silently wiping a user's saved settings.toViewConfig(nil)already yields an empty config, so the guard is the real protection; the web client always sends a non-nil full config, so no false positives. - Integration fake repo extended —
fakeViewRepoITgained auserConfigsstore plusGetUserViewConfigs/UpsertUserViewConfigto satisfy the widened repository interface. Pure test scaffolding.
The changes are tightly scoped and correctly close the loop on the per-user config design from the initial review.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Resolve conflict in view_service_test.go: keep both the per-user view config tests and upstream's cross-project isolation tests; update the per-user tests to the new 4-arg NewViewService(repo, sprintRepo, taskRepo, publisher) signature. Renumber migration 000042_add_user_view_configs.sql -> 000048 to avoid collision with upstream's new 000042_add_environments.sql.
|
Rebased onto the latest What was conflicting The branch predated the v0.14.1 changes that touched the same view code:
Validation (Go 1.26, against the merged tree): The only pending check is the fork-PR workflow gate ( Question: is there anything else you'd like changed on our side before this can be merged? Happy to adjust the resolution, squash, or rebase differently if that fits your workflow better. |
There was a problem hiding this comment.
ℹ️ Merge with upstream/master (v0.14.1) is clean — one cosmetic nit inline.
Reviewed changes
- Rebased onto upstream/master (v0.14.1) — the per-user view config feature (overlay-on-read, per-user
SetUserViewConfig, thePUT .../configendpoint, web routing,user_view_configstable) carries through the merge intact. Verified viago build ./...,go vet, and the sprint / handler / postgres repository test suites. - Adapted
NewViewServicecall sites — master's cross-project hardening addedsprintRepo/taskRepoparams (ownership checks inListViews/CreateView/ReorderViews/MoveTask/BulkMoveTasks); the per-user tests now inject permissive fakes andapp.go:160wires the real repos. - Renumbered migration
000042→000048— correctly slotted after master's new000042–000047;embed.gopicks it up automatically via//go:embed *.sql.
ℹ️ Nitpicks
services/api/migrations/000048_add_user_view_configs.sql:1— the doc header still reads-- 000042_add_user_view_configs.sqlafter the rename; sibling migrations (000045–000047) all use the matching filename in their header comment.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| @@ -0,0 +1,30 @@ | |||
| -- 000042_add_user_view_configs.sql | |||
There was a problem hiding this comment.
Stale doc header after the rename: the file is now 000048_add_user_view_configs.sql, but this comment still says 000042. Sibling migrations (000045–000047) all open with a header matching their filename, so this one is inconsistent.
MoveTask/BulkMoveTasks passaram a validar ownership da task (fix upstream 0cf5129), então um task_id inexistente agora retorna 404. Os testes de posição de view usavam UUIDs fake e esperavam 204 — o que quebra ao mesclar o v0.14.1. Passam a criar tasks reais via API antes de mover/listar. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI update — E2E was red due to a pre-existing test/behavior mismatch on
|
There was a problem hiding this comment.
ℹ️ No new issues in this delta — a test-only fix, correctly scoped.
Reviewed changes
- Adapted the four view-position e2e tests to create real tasks —
MoveTask/BulkMoveTasksnow enforce task ownership (upstream hardening 0cf5129), so the previously-faked task UUIDs would 404 instead of the asserted 204. Each test now seeds its tasks viacreateTaskViaAPIin its own project and view, keeping the existing position-count and task-id echo assertions unchanged.
The swap is consistent with the ownership check: tasks are created in the same project by the same member, each test uses a dedicated project/view, and per-view assertions (single position, 3 positions after bulk, upsert overwrite) are unaffected. The prior migration-header nit (-- 000042 comment in 000048_add_user_view_configs.sql) remains open and unchanged.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Hi @vhervatin , |
|
Hi @pikann — yes, please go ahead and push your UX adjustments directly to the branch. Edits from maintainers are enabled, so you're all set. Happy to have them, and I'll keep my side in sync. 🙏 |
2 participants
Summary
View settings and filters were global: any member changing the "View settings" panel (sort, field sum, page size, visible fields, collapsed columns, and every filter dimension) overwrote what all other members saw.
They were stored in the single shared
configJSONB column of thesprint_viewsrow, keyed only by project + context — there was nouser_idanywhere in the view entity/table/repo/service/handler. The project-wideview.updatedbroadcast then pushed that change to every connected member.This PR keeps views as shared project entities (name, type, position, and a shared default config) but makes settings/filters per-user:
user_view_configstable keyed by(view_id, user_id),ON DELETE CASCADEto bothsprint_viewsandusers.PUT /projects/{projectId}/views/{viewId}/configwrites only the current user's override and emits no project-wide event, so a member's tweaks stay private. It requiressprints:read(not write), so a read-only member can still sort/filter their own view.PATCH /views/{viewId}(name/type/position/shared default) is unchanged; the web client now routes settings-panel saves and column-collapse to the per-user endpoint. Realtimeview.*events already invalidate the views query, so a structural change by another user refetches and re-applies the caller's own effective config.Migration only adds a table; no existing data is changed (everyone keeps seeing today's shared config until they personalize).
Type of Change
Testing
go build ./...,gofmt,go vetclean; migration applied and the upsert/select + endpoint verified against a live Postgres.tsc -b && vite buildclean; existing realtime test passes.Checklist