Skip to content

Fix remaining review issues from PR #335 and #338#339

Merged
ddon merged 2 commits intoBeamLabEU:devfrom
timujinne:dev
Feb 15, 2026
Merged

Fix remaining review issues from PR #335 and #338#339
ddon merged 2 commits intoBeamLabEU:devfrom
timujinne:dev

Conversation

@timujinne
Copy link
Contributor

Summary

Addresses all 7 remaining issues identified by Claude, Mistral, and Kimi reviews of PR #338.

Changes

Security

  • Add Scope.admin? authorization checks for single-record handlers (archive_data, restore_data, toggle_status) in Data Navigator

Bug Fixes

  • Add recursive circular reference validation for category parent assignment (replaces simple self-check)
  • Filter only active products in category featured product dropdown
  • Prevent ancestor cycles in bulk_update_category_parent

Performance

  • Split load_categories into load_static_category_data + load_filtered_categories — static data (all_categories, product_counts) only reloaded on mount and mutations, not on every filter/search/page change

Code Quality

  • Move require Logger to module level in shop.ex
  • Remove unused noop event handler and phx-click="noop" attributes from categories template

Repo Hygiene

  • Remove unused MIM demo images (8.2MB, 11 files) — not referenced anywhere in code

Test Plan

  • mix compile — clean, no warnings
  • mix format — formatted
  • mix credo --strict — no issues
  • mix dialyzer — passes (285 skipped, 0 new)
  • Manual: verify featured product dropdown excludes non-active products
  • Manual: verify circular parent assignment is rejected
  • Manual: verify category filtering doesn't trigger unnecessary queries

- Add admin auth checks for single-record handlers in data_navigator
- Move require Logger to module level in shop.ex
- Filter only active products in category featured product dropdown
- Add recursive circular reference validation for category parents
- Prevent ancestor cycles in bulk category parent update
- Split load_categories into static data cache and filtered query
- Remove unused noop event handler and phx-click attributes
- Remove unused MIM demo images (8.2MB)
@ddon ddon merged commit 81f3f3e into BeamLabEU:dev Feb 15, 2026
6 checks passed
ddon pushed a commit that referenced this pull request Feb 15, 2026
Add visited-node tracking to check_ancestor_cycle to prevent infinite
loops when the database contains a pre-existing cycle. Add PR #339
review documents.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ddon pushed a commit that referenced this pull request Feb 15, 2026
- Add Kimi review: approve with all 7 issues resolved
- Add Mistral review: approve with critical bug fixed

Both reviews confirm PR #339 is production-ready.
Co-Authored-By: Kimi <noreply@moonshot.cn>
Co-Authored-By: Mistral <noreply@mistral.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants