feat(settings): add privacy filter editor#830
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #830 +/- ##
==========================================
+ Coverage 32.46% 34.10% +1.64%
==========================================
Files 35 36 +1
Lines 2039 2105 +66
Branches 362 386 +24
==========================================
+ Hits 662 718 +56
- Misses 1356 1366 +10
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR adds a dedicated Settings panel for editing
Confidence Score: 5/5The change is safe to merge; all writes go through the existing store update path and no data is silently mutated on load. The store integration is straightforward and the validation logic is sound. The two findings are minor UX polish items — a duplicate error message and an unguarded textarea during save — neither of which causes incorrect data to be written or read. src/util/privacyFilters.ts (duplicate error for redact + empty replacement) and src/views/settings/PrivacyFilterSettings.vue (textarea not disabled while saving). Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant PrivacyFilterSettings
participant validatePrivacyFiltersInput
participant SettingsStore
participant Server
User->>PrivacyFilterSettings: types JSON in textarea
PrivacyFilterSettings->>validatePrivacyFiltersInput: validate(editorText) [computed]
validatePrivacyFiltersInput-->>PrivacyFilterSettings: "{ rules, errors }"
Note over PrivacyFilterSettings: canSave = hasUnsavedChanges && errors.length===0 && !isSaving
User->>PrivacyFilterSettings: clicks Save
PrivacyFilterSettings->>SettingsStore: "update({ privacy_filters: rules })"
SettingsStore->>SettingsStore: $patch(new_state)
SettingsStore-->>PrivacyFilterSettings: watcher fires syncFromStore(false)
SettingsStore->>Server: POST /0/settings/privacy_filters
Server-->>SettingsStore: 200 OK
SettingsStore->>Server: GET /0/settings (reload)
Server-->>SettingsStore: updated settings
SettingsStore-->>PrivacyFilterSettings: update() resolves
PrivacyFilterSettings->>PrivacyFilterSettings: "syncFromStore(true) — reset editorText & savedText"
User->>PrivacyFilterSettings: clicks Discard
PrivacyFilterSettings->>PrivacyFilterSettings: syncFromStore(true) — revert to store value
Reviews (6): Last reviewed commit: "fix(settings): avoid duplicate field val..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| field?: string; | ||
| pattern: string; | ||
| action: PrivacyFilterAction; | ||
| replacement?: string; |
There was a problem hiding this comment.
field is declared optional (field?: string) in PrivacyFilterRule, meaning the server schema accepts rules without it. The validator, however, treats field as required and pushes an error on line 124 when it's absent. If a user already has server-side privacy-filter rules without a field (written via a different client or an older config), the editor will load and display them correctly, but clicking Save will fail with "field is required" — making those rules impossible to round-trip through this UI. Either change the type to field: string to signal the restriction everywhere, or drop the required check from the validator and let the server enforce its own rules.
There was a problem hiding this comment.
Fixed in 0b6fb9c: field is now declared as a required field: string in the PrivacyFilterRule interface, matching the validator's required check. Server-side rules without a field would be schema-invalid, so making it required in the type is the right call.
| const pattern = candidate.pattern; | ||
| if (typeof pattern !== 'string') { | ||
| errors.push(`Rule ${ruleNumber}: \`pattern\` must be a string.`); | ||
| } |
There was a problem hiding this comment.
pattern strings pass client-side validation (only the type is checked, not the value), so a user can save {"pattern": ""} — a rule that silently matches every event. Adding a non-empty check here catches the mistake before the server applies the rule.
| const pattern = candidate.pattern; | |
| if (typeof pattern !== 'string') { | |
| errors.push(`Rule ${ruleNumber}: \`pattern\` must be a string.`); | |
| } | |
| const pattern = candidate.pattern; | |
| if (typeof pattern !== 'string') { | |
| errors.push(`Rule ${ruleNumber}: \`pattern\` must be a string.`); | |
| } else if (pattern.trim() === '') { | |
| errors.push(`Rule ${ruleNumber}: \`pattern\` cannot be an empty string.`); | |
| } |
There was a problem hiding this comment.
Fixed in 0b6fb9c: added else if (pattern.trim() === '') check so an empty pattern string is rejected with a clear error instead of silently passing through.
| const action = candidate.action; | ||
| if (action !== 'drop' && action !== 'redact') { | ||
| errors.push(`Rule ${ruleNumber}: \`action\` must be either "drop" or "redact".`); | ||
| return; | ||
| } | ||
|
|
||
| if (field === undefined) { | ||
| errors.push( | ||
| `Rule ${ruleNumber}: \`field\` is required so the rule only matches the intended event data.` | ||
| ); | ||
| } |
There was a problem hiding this comment.
action early-return suppresses the field required error
When action fails validation the callback returns early (line 120), so the field === undefined check at line 123 never runs. A rule that has both an invalid action and a missing field will only surface one error to the user — they fix the action, then have to save again to discover the missing field. The early return is needed to avoid a redundant action === 'redact' check, but the field guard could be moved above it to ensure all errors for a rule are reported in one pass.
There was a problem hiding this comment.
Fixed in c4a33c8: moved the early-return guard to the end of all per-field checks so both an invalid action and a missing field are reported in the same pass. No more needing two save attempts to see all errors.
|
@greptileai review |
|
CI is green and the Greptile findings are addressed. I tried to merge this, but the current token does not have |
TimeToBuildBob
force-pushed
the
bob/privacy-filter-settings-ui
branch
from
c4a33c8 to
6ff6c26
Compare
|
@greptileai review |
2 participants
Summary
privacy_filtersWhy
aw-server-rust#600shipped the server-side privacy filter engine, but users still had no discoverable way to configureprivacy_filtersfromaw-webui.Testing
npm test -- privacyFilters.test.node.ts --runInBandnpm run lintnpm run build