Conversation
…assignment display, and contacts redirect
ToddHebebrand
left a comment
There was a problem hiding this comment.
Thanks — both product claims check out, and the monitoring one is a real gap worth landing.
Verified: resolveDeviceEventLogSettings and resolvePolicyMonitoringSettings in apps/api/src/routes/agents/helpers.ts were the only assignment resolvers with no role/OS gate; services/featureConfigResolver.ts:134 (buildRoleOsFilterConditions) and services/configurationPolicy.ts:2204 already apply it. Partner-wide fan-out is preserved (policyOwnershipCondition untouched). /device-groups/bulk really has no route in groups.ts, so that call was dead. Tenancy on the new groups.ts query is fine (existing org-isolation and partner-wide SELECT policies cover it, and groupIds are pre-scoped).
Must-fix
- Reuse the canonical filter instead of re-implementing it. The new inline predicate treats
roleFilter.length === 0as match-all, while the SQL form inbuildRoleOsFilterConditions(IS NULL OR x = ANY(...)) treats an empty array as match-none. Two resolvers disagreeing on the same row is how this bug happened in the first place. Please call the shared helper (or extract a purematchesRoleOsFilterfrom it and use it in both places) so there is one definition. - Tests.
helpers.eventLogDefaults.test.tsand its siblings exist; add cases where an assignment with a non-matchingosFilterloses to a matching one, and one for the empty-array semantics you settle on. The newgroups.tspolicy read has no route test either. - Split the Caddy change into its own PR. The directives are fine as written (
redirsorts beforehandlein Caddy's default order, so they do fire), but/partner/customers/*does not exist as a route onmain, so the redirects are fork-specific. The/organizations→/settings/organizationsone is the part I want to look at separately:/organizations/[id]is live upstream and there is an account-board page planned at bare/organizations, so a permanent-ish redirect there would collide with it. A standalone PR with a sentence on what your fork routes look like will make that quick to decide.
Nice-to-have
groups.tsmapGroupRowemitspolicyId,policyNameandpolicyfor the same thing; pick one.groups.tsorders assignments byprioritywith nocreatedAttiebreak, so two equal-priority rows can flip between reads; the canonical resolver breaks the tie.- Showing a single group-level assignment as "the policy" mislabels effective config when a device/site/org assignment overrides it; "group assignment" is the honest label.
DeviceGroupsPage.tsxbulk apply usesPromise.allwithpriority: 0hardcoded; a retry after partial success hits the unique constraint and reads as a blanket failure. ConsiderallSettledand surfacing per-group results.
CI is approved and running. Happy to re-review quickly once the API side is updated.
Why
roleFilterandosFiltercriteria on policy assignments were not evaluated, which could cause a policy targeted at a different OS/role to win over the appropriate policy./partner/customers/*and/organizationsled to 404s instead of redirecting to the settings organizations views.What
roleFilterandosFilterbefore picking the highest priority winner inapps/api/src/routes/agents/helpers.ts.policyonDeviceGroupresponses inapps/api/src/routes/groups.ts.apps/web/src/components/devices/DeviceGroupsPage.tsxto fetch configuration policies and apply assignments via/configuration-policies/:id/assignments.docker/Caddyfile.prodfor/partner/customers/*and/organizationsto/settings/organizations.Requested in discussion #5797.