Add 'count' operation type to security rules UI#267
Conversation
- Add "Count" checkbox to security rules interface - Add 'count' case in type description mapping - Add "count" to operation type enum in schema Related to DEV-859 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughAdds a new "count" access rule type: UI checkbox, JS logic enforcing a Read→Count dependency (display, validation, and save handling), and schema enum update to accept "count" as a valid access rule type. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Browser UI (interface.html)
participant Logic as JS Controller (js/interface.js)
participant Store as Persistence/Save
User->>UI: Toggle "Count" checkbox
UI->>Logic: Notify change (checkbox state)
Logic->>UI: Enable/disable Count if "select" present
User->>UI: Click Save
UI->>Logic: Request validation & prepare payload
Logic->>Logic: Enforce Read→Count dependency (auto-check/disable or omit count when select present)
Logic->>Store: Save access rule payload (with adjusted types)
Store-->>Logic: Acknowledge save
Logic-->>UI: Confirm saved / update UI
UI-->>User: Show result
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@js/interface.js`:
- Around line 2350-2352: The switch statement that maps access rule types to
human-readable labels (the case branches including 'select', 'insert', 'update',
'delete', and the new 'count') lacks inline documentation; add a concise comment
immediately above that switch (referencing the switch that assigns to the
variable description and the case 'count' branch) that lists the type→label
mappings: 'select' → 'Read', 'insert' → 'Write', 'update' → 'Update', 'delete' →
'Delete', 'count' → 'Count', and notes these are the valid rule types expected
by the code.
When "Read" (select) permission is checked: - Auto-check the "Count" checkbox - Disable it to visually indicate the dependency When saving, 'count' is not saved to the rule's type array if 'select' is already present, since select implies count permission. This keeps the stored data clean while showing the user the relationship. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@js/interface.js`:
- Around line 2044-2048: Add unit tests for the Read→Count dependency covering
configureAddRuleUI, updateSaveRuleValidation, and the save rule logic: set up a
test framework (Jest or Mocha) and write tests that (1) call configureAddRuleUI
with rule.type containing 'select' to assert '#chk-count' is checked and
disabled, (2) call configureAddRuleUI without 'select' to assert '#chk-count' is
enabled, (3) simulate toggling '#chk-select' and call updateSaveRuleValidation
to assert '#chk-count' becomes checked+disabled when '#chk-select' is checked
and enabled when unchecked, and (4) test the save routine (the function that
composes rule.type on save) to ensure when both 'select' and 'count' are checked
only 'select' is saved, and when only 'count' is checked 'count' is saved;
target tests at the UI-control IDs (`#chk-select`, `#chk-count`) and the functions
configureAddRuleUI and updateSaveRuleValidation (and the save handler)
referenced in the diff.
- Around line 2532-2543: The current logic that iterates
$typeCheckbox.filter(':checked').each and pushes values into rule.type can miss
filtering 'count' when 'select' appears later in DOM; instead first collect all
checked values into a temporary array (e.g., checkedTypes), then set rule.type =
checkedTypes.filter(...) to remove 'count' if 'select' is present (use
Array.prototype.indexOf or includes on checkedTypes to detect 'select'); update
the code around $typeCheckbox.filter(':checked').each and rule.type assignments
to use this two-step collect-then-filter approach.
♻️ Duplicate comments (1)
js/interface.js (1)
2353-2371: Add inline documentation for the access rule type mappings.The switch statement mapping access rule types to human-readable labels lacks documentation. Add a brief comment above the switch documenting the valid type→label mappings:
'select'→'Read','insert'→'Write','update'→'Update','delete'→'Delete','count'→'Count'.As per coding guidelines, all code should be fully documented.
📝 Suggested documentation
type: rule.type.map(function(type) { var description; + // Map access rule types to human-readable labels + // Valid types: select (Read), insert (Write), update (Update), delete (Delete), count (Count) switch (type) { case 'select': description = 'Read';
| // Apply Read→Count dependency after loading | ||
| if (rule.type.indexOf('select') !== -1) { | ||
| $('#chk-count').prop('checked', true); | ||
| $('#chk-count').prop('disabled', true); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Testing plan for Read→Count dependency logic.
Per coding guidelines, consider the following unit test scenarios for the new feature:
- configureAddRuleUI: When
rule.typeincludes'select', verify#chk-countis checked and disabled. - configureAddRuleUI: When
rule.typeexcludes'select', verify#chk-countremains enabled. - updateSaveRuleValidation: When
#chk-selectis checked, verify#chk-countbecomes checked and disabled. - updateSaveRuleValidation: When
#chk-selectis unchecked, verify#chk-countis enabled. - Save rule: When both
'select'and'count'are checked, verify only'select'is saved torule.type. - Save rule: When only
'count'is checked (without'select'), verify'count'is saved.
Note: This project currently lacks a test framework. Establishing one (e.g., Jest or Mocha) would be a prerequisite.
Also applies to: 2138-2147, 2535-2542
🤖 Prompt for AI Agents
In `@js/interface.js` around lines 2044 - 2048, Add unit tests for the Read→Count
dependency covering configureAddRuleUI, updateSaveRuleValidation, and the save
rule logic: set up a test framework (Jest or Mocha) and write tests that (1)
call configureAddRuleUI with rule.type containing 'select' to assert
'#chk-count' is checked and disabled, (2) call configureAddRuleUI without
'select' to assert '#chk-count' is enabled, (3) simulate toggling '#chk-select'
and call updateSaveRuleValidation to assert '#chk-count' becomes
checked+disabled when '#chk-select' is checked and enabled when unchecked, and
(4) test the save routine (the function that composes rule.type on save) to
ensure when both 'select' and 'count' are checked only 'select' is saved, and
when only 'count' is checked 'count' is saved; target tests at the UI-control
IDs (`#chk-select`, `#chk-count`) and the functions configureAddRuleUI and
updateSaveRuleValidation (and the save handler) referenced in the diff.
Summary
interface.htmlinterface.jsschema.jsonRelated
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Behaviour Changes
✏️ Tip: You can customize this high-level summary in your review settings.