feat: add JS-015 architecture rule to enforce confirm_modal over native confirm()
Some checks failed
Some checks failed
Prevents reintroduction of native browser confirm() dialogs by flagging them as architecture errors during pre-commit validation. Points developers to use confirm_modal/confirm_modal_dynamic Jinja2 macros. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -238,6 +238,50 @@ javascript_rules:
|
|||||||
exceptions:
|
exceptions:
|
||||||
- "utils.js"
|
- "utils.js"
|
||||||
|
|
||||||
|
- id: "JS-015"
|
||||||
|
name: "Use confirm_modal macros, not native confirm()"
|
||||||
|
severity: "error"
|
||||||
|
description: |
|
||||||
|
All confirmation dialogs must use the project's confirm_modal or
|
||||||
|
confirm_modal_dynamic Jinja2 macros from shared/macros/modals.html.
|
||||||
|
Never use the native browser confirm() dialog.
|
||||||
|
|
||||||
|
The modal macros provide:
|
||||||
|
- Consistent styled dialogs matching the admin/store theme
|
||||||
|
- Dark mode support
|
||||||
|
- Variant colors (danger=red, warning=yellow, info=blue)
|
||||||
|
- Icon support
|
||||||
|
- Double-confirm pattern for destructive operations
|
||||||
|
|
||||||
|
WRONG (native browser dialog):
|
||||||
|
if (!confirm('Are you sure you want to delete this?')) return;
|
||||||
|
if (!confirm(I18n.t('confirmations.delete'))) return;
|
||||||
|
|
||||||
|
RIGHT (state variable + modal macro):
|
||||||
|
// In JS: add state variable and remove confirm() guard
|
||||||
|
showDeleteModal: false,
|
||||||
|
async deleteItem() {
|
||||||
|
// No confirm() guard — modal already confirmed
|
||||||
|
await apiClient.delete('/admin/items/' + this.item.id);
|
||||||
|
}
|
||||||
|
|
||||||
|
// In template: button sets state, macro shows modal
|
||||||
|
<button @click="showDeleteModal = true">Delete</button>
|
||||||
|
{{ confirm_modal('deleteModal', 'Delete Item', 'Are you sure?',
|
||||||
|
'deleteItem()', 'showDeleteModal', 'Delete', 'Cancel', 'danger') }}
|
||||||
|
|
||||||
|
For dynamic messages (containing JS expressions):
|
||||||
|
{{ confirm_modal_dynamic('deleteModal', 'Delete Item',
|
||||||
|
"'Delete ' + item.name + '?'",
|
||||||
|
'deleteItem()', 'showDeleteModal', 'Delete', 'Cancel', 'danger') }}
|
||||||
|
pattern:
|
||||||
|
file_pattern: "static/**/js/**/*.js"
|
||||||
|
anti_patterns:
|
||||||
|
- "confirm\\("
|
||||||
|
exceptions:
|
||||||
|
- "utils.js"
|
||||||
|
- "vendor/"
|
||||||
|
|
||||||
- id: "JS-010"
|
- id: "JS-010"
|
||||||
name: "Use PlatformSettings for pagination rows per page"
|
name: "Use PlatformSettings for pagination rows per page"
|
||||||
severity: "error"
|
severity: "error"
|
||||||
|
|||||||
@@ -482,6 +482,9 @@ class ArchitectureValidator:
|
|||||||
# JS-009: Check for alert() or window.showToast instead of Utils.showToast()
|
# JS-009: Check for alert() or window.showToast instead of Utils.showToast()
|
||||||
self._check_toast_usage(file_path, content, lines)
|
self._check_toast_usage(file_path, content, lines)
|
||||||
|
|
||||||
|
# JS-015: Check for native confirm() instead of confirm_modal macros
|
||||||
|
self._check_confirm_usage(file_path, content, lines)
|
||||||
|
|
||||||
def _check_toast_usage(self, file_path: Path, content: str, lines: list[str]):
|
def _check_toast_usage(self, file_path: Path, content: str, lines: list[str]):
|
||||||
"""JS-009: Check for alert() or window.showToast instead of Utils.showToast()"""
|
"""JS-009: Check for alert() or window.showToast instead of Utils.showToast()"""
|
||||||
# Skip utils.js (where showToast is defined)
|
# Skip utils.js (where showToast is defined)
|
||||||
@@ -529,6 +532,44 @@ class ArchitectureValidator:
|
|||||||
suggestion="Replace window.showToast('msg', 'type') with Utils.showToast('msg', 'type')",
|
suggestion="Replace window.showToast('msg', 'type') with Utils.showToast('msg', 'type')",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def _check_confirm_usage(self, file_path: Path, content: str, lines: list[str]):
|
||||||
|
"""JS-015: Check for native confirm() instead of confirm_modal macros."""
|
||||||
|
# Skip utility files
|
||||||
|
if file_path.name in ("utils.js",):
|
||||||
|
return
|
||||||
|
|
||||||
|
# Skip vendor libraries
|
||||||
|
if "vendor/" in str(file_path):
|
||||||
|
return
|
||||||
|
|
||||||
|
# Check for file-level noqa comment
|
||||||
|
if "noqa: js-015" in content.lower():
|
||||||
|
return
|
||||||
|
|
||||||
|
for i, line in enumerate(lines, 1):
|
||||||
|
stripped = line.strip()
|
||||||
|
|
||||||
|
# Skip comments
|
||||||
|
if stripped.startswith(("//", "/*")):
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Skip lines with inline noqa comment
|
||||||
|
if "noqa: js-015" in line.lower():
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Check for confirm() usage
|
||||||
|
if re.search(r"\bconfirm\s*\(", line):
|
||||||
|
self._add_violation(
|
||||||
|
rule_id="JS-015",
|
||||||
|
rule_name="Use confirm_modal macros, not native confirm()",
|
||||||
|
severity=Severity.ERROR,
|
||||||
|
file_path=file_path,
|
||||||
|
line_number=i,
|
||||||
|
message="Native browser confirm() dialog - use confirm_modal/confirm_modal_dynamic macro",
|
||||||
|
context=stripped[:80],
|
||||||
|
suggestion="Add a state variable (e.g. showDeleteModal: false), set it in @click, and use confirm_modal macro in the template",
|
||||||
|
)
|
||||||
|
|
||||||
def _check_alpine_data_spread(
|
def _check_alpine_data_spread(
|
||||||
self, file_path: Path, content: str, lines: list[str]
|
self, file_path: Path, content: str, lines: list[str]
|
||||||
):
|
):
|
||||||
|
|||||||
Reference in New Issue
Block a user