feat(preferences): add preferences JSONB column and custom type to User model - BED-7958#2675
Open
cami-specter wants to merge 13 commits intomainfrom
Open
feat(preferences): add preferences JSONB column and custom type to User model - BED-7958#2675cami-specter wants to merge 13 commits intomainfrom
cami-specter wants to merge 13 commits intomainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
cami-specter
force-pushed
the
wip-BED-7958-create-migration-model-preferences
branch
from
df5860a to
d14cfe9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
cmd/api/src/database/migration/migrations/v9.1.0.sql (1)
75-76: Consider adding a database-level check constraint to enforce JSON object structure.While the column lacks a
jsonb_typeof(preferences) = 'object'check constraint, the application mitigates this through thePreferences.Scan()method, which validates the structure viajson.Unmarshalduring reads and writes through GORM'sValue()method. A database-level constraint would provide defense-in-depth against direct SQL manipulation, though it is not required for normal operation.If added, guard the constraint with existence check:
DO $$ BEGIN IF NOT EXISTS ( SELECT 1 FROM pg_constraint WHERE conname = 'users_preferences_is_object' ) THEN ALTER TABLE users ADD CONSTRAINT users_preferences_is_object CHECK (jsonb_typeof(preferences) = 'object'); END IF; END $$;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/src/database/migration/migrations/v9.1.0.sql` around lines 75 - 76, Add a DB-level check constraint to ensure the users.preferences JSONB column is always a JSON object by adding a guarded ALTER TABLE that creates constraint users_preferences_is_object with CHECK (jsonb_typeof(preferences) = 'object'); implement it using the DO $$ BEGIN IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'users_preferences_is_object') THEN ALTER TABLE users ADD CONSTRAINT users_preferences_is_object CHECK (jsonb_typeof(preferences) = 'object'); END IF; END $$; so the constraint is only added when missing and references the users table and preferences column.cmd/api/src/model/preferences.go (3)
39-49:Scandoes not reset the receiver, leading to potential stale entries.
json.Unmarshalinto a non-nil map merges keys rather than replacing them. If aPreferencesvalue is ever reused/rescanned (e.g., GORM reusing a destination), previously-present keys that are absent in the new JSONB payload will silently persist. Consider resetting before unmarshaling:Proposed fix
if bytes, typeOK = value.([]byte); !typeOK { return fmt.Errorf("expected []byte representation of JSONB, received %T", value) } + *s = Preferences{} return json.Unmarshal(bytes, s)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/src/model/preferences.go` around lines 39 - 49, The Scan method for Preferences must clear the receiver before calling json.Unmarshal to avoid merging into an existing map; in the Preferences.Scan function reset the receiver (e.g., set *s = nil or an empty Preferences value) right after successful []byte type assertion and before json.Unmarshal, so each scan replaces prior keys instead of merging with stale entries.
53-55: Consider a value receiver onValue()fordriver.Valuer.
driver.Valueris conventionally implemented on a value receiver so the interface is satisfied by bothPreferencesand*Preferences. With a pointer receiver, only*Preferencessatisfiesdriver.Valuer; when the ORM or a query helper passes a non-addressablePreferencesvalue (e.g., as a map field embedded in a struct passed by value, or insideanyarguments),database/sqlwill fall through to default conversion and fail to JSON-encode.GORM typically resolves the field to a pointer, so this works today, but the asymmetry with
Scan(which rightly uses a pointer receiver because it mutates) is worth reconsidering:Proposed change
-func (s *Preferences) Value() (driver.Value, error) { +func (s Preferences) Value() (driver.Value, error) { return json.Marshal(s) }Similarly,
GormDBDataTypedoes not need a pointer receiver.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/src/model/preferences.go` around lines 53 - 55, The Value() method currently has a pointer receiver which prevents non-addressable Preferences values from satisfying database/sql/driver.Valuer; change the receiver to a value receiver on Value() (i.e., func (s Preferences) Value() (driver.Value, error)) so both Preferences and *Preferences implement driver.Valuer, and likewise change GormDBDataType to use a value receiver if it currently uses a pointer; keep Scan() as a pointer receiver since it mutates the receiver. Ensure json.Marshal(s) still works with the value receiver.
39-46:Scanrejectsjson.RawMessageand other[]byte-derived types.The type assertion
value.([]byte)only succeeds when the dynamic type is exactly[]byte. Named types backed by[]byte(e.g.,json.RawMessage) will be rejected even though they are semantically the same payload. Most Postgres drivers (includingpgx/lib/pq) handjsonbcolumns back as[]byte, so this is usually fine in practice, but it also means the test at Line 34-38 (TestPreferences_Scan_InvalidJSON) is actually exercising the type-assertion error path rather than JSON parse failure — see separate comment on the test file.Consider using a type switch if you want to accept both forms:
Optional refactor
- if bytes, typeOK = value.([]byte); !typeOK { - return fmt.Errorf("expected []byte representation of JSONB, received %T", value) - } + switch v := value.(type) { + case []byte: + bytes = v + case string: + bytes = []byte(v) + default: + return fmt.Errorf("expected []byte representation of JSONB, received %T", value) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/src/model/preferences.go` around lines 39 - 46, The Scan method currently uses a strict type assertion value.([]byte) which rejects named byte-slice types like json.RawMessage; change Preferences.Scan to accept those by using a type switch (e.g., case []byte, case json.RawMessage, case string, case *string) and convert to a []byte for JSON unmarshalling (also handle nil). Update the error path to report the actual dynamic type when none of the accepted cases match and ensure the JSON unmarshal error is returned when parsing fails (this will also make TestPreferences_Scan_InvalidJSON exercise JSON parsing rather than the type-assertion failure).cmd/api/src/model/preferences_test.go (1)
30-30: Preferrequire.Error/require.NoErroroverNotNil/Nilfor errors.Throughout this file (Lines 30, 37, 43, 61, 84, 88, 101, 120, 124, 129), error assertions use
require.NotNil(t, err)/require.Nil(t, err).require.Errorandrequire.NoErrorproduce clearer failure messages and are the idiomatic testify pattern for error checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/api/src/model/preferences_test.go` at line 30, In preferences_test.go replace all assertions that check the error variable using require.NotNil(t, err) with require.Error(t, err) and replace require.Nil(t, err) with require.NoError(t, err); search for the error variable "err" in that test file (the occurrences around the earlier failing asserts) and update each assertion to use the idiomatic testify helpers to produce clearer failure messages.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/api/src/model/auth.go`:
- Line 455: User.Preferences is currently map-backed and can be nil, causing
Value() to return JSON null; update the Preferences.Value() and
Preferences.MarshalJSON() methods to normalize nil receivers to an empty object
by returning "{}" (as JSON bytes) instead of nil/null, so DB driver.Valuer and
JSON marshaling produce {} consistent with the migration and OpenAPI schema;
locate the Preferences type and implement this nil-check inside the Value() and
MarshalJSON() methods to return the byte representation of an empty JSON object
when the underlying map is nil.
In `@cmd/api/src/model/preferences_test.go`:
- Around line 34-38: The test TestPreferences_Scan_InvalidJSON is asserting the
wrong failure path because it passes a json.RawMessage (a named []byte type) so
Preferences.Scan hits the type assertion error instead of json.Unmarshal; update
the test to pass a plain []byte containing actually-invalid JSON (or valid JSON
that cannot unmarshal into map[string]PreferenceItem) so that Preferences.Scan
exercises the json.Unmarshal branch and returns the JSON parsing error from Scan
(target the Preferences.Scan method and the value.([]byte) assertion when
locating code).
---
Nitpick comments:
In `@cmd/api/src/database/migration/migrations/v9.1.0.sql`:
- Around line 75-76: Add a DB-level check constraint to ensure the
users.preferences JSONB column is always a JSON object by adding a guarded ALTER
TABLE that creates constraint users_preferences_is_object with CHECK
(jsonb_typeof(preferences) = 'object'); implement it using the DO $$ BEGIN IF
NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname =
'users_preferences_is_object') THEN ALTER TABLE users ADD CONSTRAINT
users_preferences_is_object CHECK (jsonb_typeof(preferences) = 'object'); END
IF; END $$; so the constraint is only added when missing and references the
users table and preferences column.
In `@cmd/api/src/model/preferences_test.go`:
- Line 30: In preferences_test.go replace all assertions that check the error
variable using require.NotNil(t, err) with require.Error(t, err) and replace
require.Nil(t, err) with require.NoError(t, err); search for the error variable
"err" in that test file (the occurrences around the earlier failing asserts) and
update each assertion to use the idiomatic testify helpers to produce clearer
failure messages.
In `@cmd/api/src/model/preferences.go`:
- Around line 39-49: The Scan method for Preferences must clear the receiver
before calling json.Unmarshal to avoid merging into an existing map; in the
Preferences.Scan function reset the receiver (e.g., set *s = nil or an empty
Preferences value) right after successful []byte type assertion and before
json.Unmarshal, so each scan replaces prior keys instead of merging with stale
entries.
- Around line 53-55: The Value() method currently has a pointer receiver which
prevents non-addressable Preferences values from satisfying
database/sql/driver.Valuer; change the receiver to a value receiver on Value()
(i.e., func (s Preferences) Value() (driver.Value, error)) so both Preferences
and *Preferences implement driver.Valuer, and likewise change GormDBDataType to
use a value receiver if it currently uses a pointer; keep Scan() as a pointer
receiver since it mutates the receiver. Ensure json.Marshal(s) still works with
the value receiver.
- Around line 39-46: The Scan method currently uses a strict type assertion
value.([]byte) which rejects named byte-slice types like json.RawMessage; change
Preferences.Scan to accept those by using a type switch (e.g., case []byte, case
json.RawMessage, case string, case *string) and convert to a []byte for JSON
unmarshalling (also handle nil). Update the error path to report the actual
dynamic type when none of the accepted cases match and ensure the JSON unmarshal
error is returned when parsing fails (this will also make
TestPreferences_Scan_InvalidJSON exercise JSON parsing rather than the
type-assertion failure).
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 92ca851d-5be4-48d8-83aa-82bcc45a7aa4
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
cmd/api/src/api/v2/auth/auth_test.gocmd/api/src/database/migration/migrations/v9.1.0.sqlcmd/api/src/model/auth.gocmd/api/src/model/preferences.gocmd/api/src/model/preferences_test.gogo.modpackages/go/openapi/doc/openapi.jsonpackages/go/openapi/src/schemas/model.user.yaml
cami-specter
force-pushed
the
wip-BED-7958-create-migration-model-preferences
branch
from
d14cfe9 to
a61bb01
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/api/src/model/preferences.go`:
- Around line 34-58: Add a MarshalJSON method on the Preferences type that
normalizes nil or zero-value maps to an empty JSON object (i.e. return
[]byte("{}") instead of null) so API JSON serialization matches the
database/default schema; implement func (s Preferences) MarshalJSON() ([]byte,
error) to check for nil/len==0 and return []byte("{}") otherwise return
json.Marshal(map[string]PreferenceItem(s)); this complements the existing
Value() and Scan() implementations and will require updating tests that
currently expect null.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 0ac111ea-589a-4c56-9f91-8b6869bb8306
📒 Files selected for processing (7)
cmd/api/src/api/v2/auth/auth_test.gocmd/api/src/database/migration/migrations/v9.1.0.sqlcmd/api/src/model/auth.gocmd/api/src/model/preferences.gocmd/api/src/model/preferences_test.gopackages/go/openapi/doc/openapi.jsonpackages/go/openapi/src/schemas/model.user.yaml
✅ Files skipped from review due to trivial changes (4)
- cmd/api/src/database/migration/migrations/v9.1.0.sql
- cmd/api/src/api/v2/auth/auth_test.go
- cmd/api/src/model/preferences_test.go
- packages/go/openapi/doc/openapi.json
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/api/src/model/auth.go
Comment on lines
+34
to
+58
| // Preferences represents a map of preference names to their PreferenceItem values, stored as JSONB in the database. | ||
| type Preferences map[string]PreferenceItem | ||
|
|
||
| // Scan verifies that the input value is []byte (the Go representation of JSONB) and unmarshals it into the receiver. | ||
| // Uses a pointer receiver because it must modify the map. This matches the pattern in types.JSONUntypedObject. | ||
| // sql.Scanner implementation | ||
| func (s *Preferences) Scan(value any) error { | ||
| var ( | ||
| bytes []byte | ||
| typeOK bool | ||
| ) | ||
| if bytes, typeOK = value.([]byte); !typeOK { | ||
| return fmt.Errorf("expected []byte representation of JSONB, received %T", value) | ||
| } | ||
|
|
||
| return json.Unmarshal(bytes, s) | ||
| } | ||
|
|
||
| // Value marshals the Preferences map into []byte for storage as JSONB. | ||
| // driver.Valuer implementation | ||
| func (s Preferences) Value() (driver.Value, error) { | ||
| if len(s) == 0 { | ||
| return []byte("{}"), nil | ||
| } | ||
| return json.Marshal(s) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether Preferences has a custom JSON marshaler and whether null preferences are expected anywhere.
rg -n -C3 --type=go 'func \(s \*?Preferences\) MarshalJSON|Preferences\s+Preferences|preferences":\s*null'
rg -n -C3 'ADD COLUMN IF NOT EXISTS preferences|preferences JSONB NOT NULL DEFAULT|^\s+preferences:'Repository: SpecterOps/BloodHound
Length of output: 4932
Add MarshalJSON() to normalize nil Preferences for API output.
Currently, a zero-value Preferences map marshals to JSON null when the field is uninitialized, conflicting with the OpenAPI schema (type: object) and the JSONB NOT NULL DEFAULT '{}' database constraint. While Value() converts nil/empty preferences to {} for database storage, this only affects writes—not API JSON serialization. Test cases in cmd/api/src/api/v2/auth/auth_test.go (lines 2483, 2566, 2588) currently expect "preferences":null and will require updates.
Proposed fix
type Preferences map[string]PreferenceItem
+// MarshalJSON normalizes nil/empty preferences to an empty object for API responses.
+func (s Preferences) MarshalJSON() ([]byte, error) {
+ if len(s) == 0 {
+ return []byte("{}"), nil
+ }
+
+ type preferences Preferences
+ return json.Marshal(preferences(s))
+}
+
// Scan verifies that the input value is []byte (the Go representation of JSONB) and unmarshals it into the receiver.
// Uses a pointer receiver because it must modify the map. This matches the pattern in types.JSONUntypedObject.
// sql.Scanner implementation
func (s *Preferences) Scan(value any) error {
@@
func (s Preferences) Value() (driver.Value, error) {
- if len(s) == 0 {
- return []byte("{}"), nil
- }
- return json.Marshal(s)
+ return s.MarshalJSON()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Preferences represents a map of preference names to their PreferenceItem values, stored as JSONB in the database. | |
| type Preferences map[string]PreferenceItem | |
| // Scan verifies that the input value is []byte (the Go representation of JSONB) and unmarshals it into the receiver. | |
| // Uses a pointer receiver because it must modify the map. This matches the pattern in types.JSONUntypedObject. | |
| // sql.Scanner implementation | |
| func (s *Preferences) Scan(value any) error { | |
| var ( | |
| bytes []byte | |
| typeOK bool | |
| ) | |
| if bytes, typeOK = value.([]byte); !typeOK { | |
| return fmt.Errorf("expected []byte representation of JSONB, received %T", value) | |
| } | |
| return json.Unmarshal(bytes, s) | |
| } | |
| // Value marshals the Preferences map into []byte for storage as JSONB. | |
| // driver.Valuer implementation | |
| func (s Preferences) Value() (driver.Value, error) { | |
| if len(s) == 0 { | |
| return []byte("{}"), nil | |
| } | |
| return json.Marshal(s) | |
| // Preferences represents a map of preference names to their PreferenceItem values, stored as JSONB in the database. | |
| type Preferences map[string]PreferenceItem | |
| // MarshalJSON normalizes nil/empty preferences to an empty object for API responses. | |
| func (s Preferences) MarshalJSON() ([]byte, error) { | |
| if len(s) == 0 { | |
| return []byte("{}"), nil | |
| } | |
| type preferences Preferences | |
| return json.Marshal(preferences(s)) | |
| } | |
| // Scan verifies that the input value is []byte (the Go representation of JSONB) and unmarshals it into the receiver. | |
| // Uses a pointer receiver because it must modify the map. This matches the pattern in types.JSONUntypedObject. | |
| // sql.Scanner implementation | |
| func (s *Preferences) Scan(value any) error { | |
| var ( | |
| bytes []byte | |
| typeOK bool | |
| ) | |
| if bytes, typeOK = value.([]byte); !typeOK { | |
| return fmt.Errorf("expected []byte representation of JSONB, received %T", value) | |
| } | |
| return json.Unmarshal(bytes, s) | |
| } | |
| // Value marshals the Preferences map into []byte for storage as JSONB. | |
| // driver.Valuer implementation | |
| func (s Preferences) Value() (driver.Value, error) { | |
| return s.MarshalJSON() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/api/src/model/preferences.go` around lines 34 - 58, Add a MarshalJSON
method on the Preferences type that normalizes nil or zero-value maps to an
empty JSON object (i.e. return []byte("{}") instead of null) so API JSON
serialization matches the database/default schema; implement func (s
Preferences) MarshalJSON() ([]byte, error) to check for nil/len==0 and return
[]byte("{}") otherwise return json.Marshal(map[string]PreferenceItem(s)); this
complements the existing Value() and Scan() implementations and will require
updating tests that currently expect null.
Contributor
Author
There was a problem hiding this comment.
Intentionally not adding MarshalJSON(). Other fields that marshal to null don't have custom MarshalJSON() methods either. Also, the column is NOT NULL DEFAULT '{}'::jsonb, so nil preferences will never come from the database, only from test Go structs.
Contributor
There was a problem hiding this comment.
@cami-specter, understood — that's a reasonable design decision. Since the NOT NULL DEFAULT '{}'::jsonb constraint guarantees non-null values from the database, and the codebase doesn't apply custom MarshalJSON() to similar fields, omitting it keeps things consistent. I'll drop this suggestion.
✏️ Learnings added
Learnt from: cami-specter
Repo: SpecterOps/BloodHound PR: 2675
File: cmd/api/src/model/preferences.go:34-58
Timestamp: 2026-04-17T21:01:00.618Z
Learning: In BloodHound (SpecterOps/BloodHound), the `Preferences` type (`map[string]PreferenceItem`) in `cmd/api/src/model/preferences.go` intentionally does NOT have a custom `MarshalJSON()` method. The `preferences` column is `NOT NULL DEFAULT '{}'::jsonb`, so nil values only arise in test Go structs, not from DB reads. This is consistent with the broader codebase convention where similar fields do not define custom JSON marshalers to handle nil/empty cases.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: mvlipka
Repo: SpecterOps/BloodHound PR: 1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: mvlipka
Repo: SpecterOps/BloodHound PR: 2382
File: cmd/api/src/api/v2/ad_related_entity.go:44-57
Timestamp: 2026-02-23T17:06:23.991Z
Learning: In BloodHound, AD nodes are represented by a base kind (ad.Entity) plus specific kinds (ad.User, ad.Computer, ad.Group, etc.), and Azure nodes by az.Entity. The query.KindIn function matches if a node has any of the specified kinds, so using the base kind (ad.Entity) will correctly include all AD nodes regardless of their specific subtype. During reviews, ensure code that filters or queries graph nodes via KindIn uses the base kind to capture all inherited subkinds for AD nodes, and similarly recognizes base kinds for Azure nodes. This design pattern should be consistently respected across files that model or query graph schema kinds.
Learnt from: superlinkx
Repo: SpecterOps/BloodHound PR: 2421
File: cmd/api/src/model/auth.go:154-154
Timestamp: 2026-02-25T23:53:13.603Z
Learning: In BloodHound, rename time-based expiration fields from Expiration to ExpiresAt in all model structs under cmd/api/src/model to align with existing ExpiresAt usage (e.g., AuthSecret.ExpiresAt) and to let GORM map to expires_at automatically without needing explicit gorm:"column:expires_at" tags.
cweidenkeller
approved these changes
Apr 17, 2026
mistahj67
reviewed
Apr 18, 2026
| "value": {}, | ||
| "enterprise": { | ||
| "type": "boolean" | ||
| } |
Contributor
There was a problem hiding this comment.
Should this be community?
Contributor
Author
There was a problem hiding this comment.
Yes, thanks for catching that!
Contributor
Author
There was a problem hiding this comment.
This field/property has been removed.
| // PreferenceItem represents a single user preference with its value and a community flag. | ||
| type PreferenceItem struct { | ||
| Value any `json:"value"` | ||
| Community bool `json:"community"` |
Contributor
There was a problem hiding this comment.
Lets chat on monday, I am wondering if bool is the best fit here
mistahj67
requested changes
Apr 18, 2026
…er model - add Preferences custom type (map[string]PreferenceItem) with Scanner/Valuer/GormDBDataType - add PreferenceItem struct with value and enterprise flag - add migration to add preferences JSONB column to users table (v9.1.0) - add unit tests for Preferences Scan and Value methods - update OpenAPI docs for the GET user endpoint (returning preferences) - update existing unit tests fixes: BED-7958 # Conflicts: # cmd/api/src/database/migration/migrations/v9.1.0.sql
# Conflicts: # go.sum
cami-specter
force-pushed
the
wip-BED-7958-create-migration-model-preferences
branch
from
6ccb996 to
e6d18b2
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/api/src/model/preferences.go`:
- Around line 28-31: PreferenceItem currently only stores Value and omits the
community metadata, causing community to be lost during Scan() and Value()
round-trips; add a Community field to the PreferenceItem struct (e.g., Community
string `json:"community"`), and update any database marshalling/unmarshalling
methods (the PreferenceItem's Scan() and Value() implementations) to
include/parse the community field so it is persisted and restored along with
Value. Ensure JSON tags match existing conventions and update tests/usage where
PreferenceItem is constructed or decoded.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 19dbf172-ff8c-4e68-9c25-8755890053ad
📒 Files selected for processing (7)
cmd/api/src/api/v2/auth/auth_test.gocmd/api/src/database/migration/migrations/v9.1.0.sqlcmd/api/src/model/auth.gocmd/api/src/model/preferences.gocmd/api/src/model/preferences_test.gopackages/go/openapi/doc/openapi.jsonpackages/go/openapi/src/schemas/model.user.yaml
✅ Files skipped from review due to trivial changes (4)
- packages/go/openapi/src/schemas/model.user.yaml
- cmd/api/src/api/v2/auth/auth_test.go
- cmd/api/src/model/preferences_test.go
- packages/go/openapi/doc/openapi.json
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/api/src/database/migration/migrations/v9.1.0.sql
- cmd/api/src/model/auth.go
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
3 participants
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.
Description
Currently, we don't have a way to save user preferences in one place. This PR (subtask of https://specterops.atlassian.net/browse/BED-7619) adds a preferences JSONB column to the
userstable and a corresponding Preferences custom type to the User model. This will enable users to store settings like dark mode and preferred domain.Motivation and Context
Resolves BED-7958
How Has This Been Tested?
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Database
Documentation
Tests