Merged
Conversation
507e61c to
39acc2b
Compare
Contributor
There was a problem hiding this comment.
3 issues found across 19 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/console/src/pages/organizations/cookie-banners/configuration/cookies/_components/CategorySection.tsx">
<violation number="1" location="apps/console/src/pages/organizations/cookie-banners/configuration/cookies/_components/CategorySection.tsx:271">
P1: Moving a cookie is implemented as two non-atomic updates, so a partial failure can duplicate the cookie across categories.</violation>
</file>
<file name="pkg/coredata/cookie_banner_version.go">
<violation number="1" location="pkg/coredata/cookie_banner_version.go:42">
P1: This snapshot schema change is not backward-compatible with existing stored version JSON (`required`), so legacy snapshots deserialize with an empty `kind` and can lose necessary-category semantics.</violation>
</file>
<file name="pkg/cookiebanner/service.go">
<violation number="1" location="pkg/cookiebanner/service.go:973">
P2: Uncategorised is only auto-created when the deleted category has cookies, so banners missing that system category can stay invalid after deleting empty categories.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
4 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/console/src/pages/organizations/cookie-banners/configuration/cookies/_components/EditCookieRow.tsx">
<violation number="1" location="apps/console/src/pages/organizations/cookie-banners/configuration/cookies/_components/EditCookieRow.tsx:35">
P2: Don't initialize editable state once from props here. With index keys in the parent, the row can be reused for a different cookie while keeping stale form values, so Save may write the wrong data.</violation>
</file>
<file name="pkg/cookiebanner/service.go">
<violation number="1" location="pkg/cookiebanner/service.go:951">
P1: Reject moves where source and target category are identical; currently this duplicates the cookie when both IDs are the same.</violation>
</file>
<file name="pkg/server/api/console/v1/graphql/cookie_banner.graphql">
<violation number="1" location="pkg/server/api/console/v1/graphql/cookie_banner.graphql:245">
P1: Reject source==target here; a self-move can duplicate the cookie.</violation>
<violation number="2" location="pkg/server/api/console/v1/graphql/cookie_banner.graphql:247">
P2: Use a stable cookie identifier instead of the name; name-only lookup is ambiguous.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
gearnode
reviewed
Apr 21, 2026
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="pkg/server/api/console/v1/cookie_banner_resolvers.go">
<violation number="1" location="pkg/server/api/console/v1/cookie_banner_resolvers.go:540">
P2: Return a non-NotFound error for banner mismatches; both categories were already found, so this path is an invalid move, not a missing resource.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Replace the `required` boolean column on cookie_categories with a `kind` enum (NORMAL, NECESSARY, UNCATEGORISED). The Necessary category remains undeletable and always-on for consent; the new Uncategorised category is also undeletable but users can opt out of it. When a category is deleted, its cookies are merged into the Uncategorised category (lazy-created for legacy banners that don't have one yet). Signed-off-by: Émile Ré <emile@getprobo.com>
Signed-off-by: Émile Ré <emile@getprobo.com>
Moving a cookie between categories previously required two sequential updateCookieCategory mutations, which was not atomic and could leave data in an inconsistent state if the second call failed. This adds a dedicated moveCookieToCategory mutation that performs both updates in a single transaction. Signed-off-by: Émile Ré <emile@getprobo.com>
Signed-off-by: Émile Ré <emile@getprobo.com>
Signed-off-by: Émile Ré <emile@getprobo.com>
Signed-off-by: Émile Ré <emile@getprobo.com>
Signed-off-by: Émile Ré <emile@getprobo.com>
f4591e5 to
9848795
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes ENG-302
Summary by cubic
Adds an Uncategorised cookie category and replaces the
requiredflag with akindenum to simplify consent logic, category management, and UI. Also adds an atomicmoveCookieToCategorymutation and improves delete flows with confirmation and clearer error handling.New Features
requiredtokind(NORMAL,NECESSARY,UNCATEGORISED) across backend, GraphQL, andpackages/cookie-banner.UNCATEGORISEDcategory per banner. Deleting aNORMALcategory moves its cookies there.NECESSARYandUNCATEGORISEDare undeletable;NECESSARYis always on.moveCookieToCategoryfor atomic cookie moves. Console adds a per-cookie “Move to” dropdown, delete confirmations with clear messaging, and toasts that surface server errors; badges/buttons reflectkind. Edit/add forms were extracted into components for a cleaner cookie editor.Migration
pkg/coredata/migrations/20260420T143027Z.sqlto addkind, backfill fromrequired, droprequired, and enforce oneNECESSARYand oneUNCATEGORISEDper banner.CookieCategory.required→kind; removerequiredfromCreateCookieCategoryInput. UsemoveCookieToCategoryinstead of twoupdateCookieCategorycalls when moving cookies.probo-categorynow uses akindattribute; consent defaults inpackages/cookie-bannerdepend onkind === "NECESSARY".Written for commit 9848795. Summary will update on new commits.