Skip to content

advanced user dashboard - #488

Closed
mithereal wants to merge 9 commits into
BeamLabEU:mainfrom
BeamLabUS:main
Closed

advanced user dashboard#488
mithereal wants to merge 9 commits into
BeamLabEU:mainfrom
BeamLabUS:main

Conversation

@mithereal

Copy link
Copy Markdown

i created a more advanced user dashboard that will auto load widgets from modules and place them on a grid, its installed via a mix task
i have updated the billing module to reflect the changes

create a guide explaining how to create a default landing page for only guest users while redirecting everyone else to the dashboard
ddon added a commit that referenced this pull request Apr 13, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ddon

ddon commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Posted a full review at dev_docs/pull_requests/2026/488-advanced-user-dashboard/CLAUDE_REVIEW.md (on the dev branch).

TL;DR — request changes. The widget-based dashboard concept is welcome, but there are several blockers:

Critical

  • V97.down/0 drops phoenix_kit_files / phoenix_kit_media_folders / phoenix_kit_media_folder_links and resets the comment to '94' — looks copy-pasted. Rollback would destroy unrelated media data.
  • Schema/table name mismatch: schema "dashboard_layouts" vs migration phoenix_kit_dashboard_layouts.
  • Dashboard.Index.mount/3 returns {:error, :not_authenticated} (invalid shape, crashes LV) and reads a nonexistent session["phoenix_kit_current_user"] key instead of phoenix_kit_current_scope.
  • Generator injects route into live_session :authenticated, but PhoenixKit uses :phoenix_kit_admin — injection will never match.

High

  • Layout schema violates UUIDv7 convention (default integer :id), wrong PK shape (single-column widget_uuid PK prevents per-user rows), belongs_to :user_uuid misnamed, no indexes, no timestamps, FK on_delete: :nothing.
  • Widgets.Loader hardcodes @known_modules, ignoring PhoenixKit.ModuleDiscovery; user_can_access_module?/2 always returns true.
  • Widgets.Registry GenServer never added to supervision — dead code.
  • RevenueComponent renders an empty div; GridStack save/remove handlers are IO.inspect stubs.
  • Hardcoded /admin/modules path violates the Routes.path/1 rule in CLAUDE.md.

Scope / hygiene

  • phoenix_kit.iml (IntelliJ) committed — should be gitignored.
  • guides/landing-page-unauthenticated.md and scripts/install_dashboard.sh feel out-of-scope / overlap with the mix task.
  • 650-line custom CSS with hardcoded colors bypasses the daisyUI 5 theme system.

Happy to pair on the migration + schema fixes first since those are the hard blockers. Full detail (severity-tagged) in the review file.

@ddon

ddon commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

@mithereal hey, thanks for your PR, but please read review, looks like there are a lot of work first needs to be done... and please attach some screenshots of what you are building and what you are trying to create, we would like to see the end goal!

auto load widgets from modules into the users dashboard
make a task mix phoenix_kit.gen.user.dashboard.advanced
@mithereal

Copy link
Copy Markdown
Author

im trying to build a dashboard which pulls widgets from the modules and places them on a grid
image

@mithereal

Copy link
Copy Markdown
Author

ill prolly put avail modules on the right side and drag + drop now that i think about it

addressed migration
removed sh script
removed guide
@mithereal

Copy link
Copy Markdown
Author

pushed the wip as i will lose my electricity today, and may/not be able to contribute anymore, im trying to keep from being homeless and i only have 1 hand, hope you understand.

@mithereal
mitherealforce-pushed the main branch 4 times, most recently from 9596a77 to 994da18CompareApril 14, 2026 05:51
addressed migration
removed sh script
removed guide
use module_discovery
widget enabled = false
dashboard css uses theme tokens
remove return_ok
use phoenix_kit_admin for session
cleanup
@mithereal
mitherealforce-pushed the main branch 3 times, most recently from 0c54d19 to be743f0CompareApril 14, 2026 06:34
add dashboard layouts to store widget positions
@mithereal
mitherealforce-pushed the main branch 2 times, most recently from bc2cb8c to f29b9d2CompareApril 14, 2026 08:13
@mithereal
mitherealforce-pushed the main branch 3 times, most recently from e21d75c to f89232aCompareApril 14, 2026 09:22
@mithereal
mitherealforce-pushed the main branch 9 times, most recently from 1711410 to e9f7cbaCompareApril 14, 2026 19:54
@mithereal
mitherealforce-pushed the main branch 2 times, most recently from d846f7a to 837ab40CompareApril 14, 2026 22:35
@ddon

ddon commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Lot's of changes from you, will ask Claude to review them...

@ddon

ddon commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Hi @mithereal — thanks for the screenshot, that clarifies the goal (drag-and-drop grid with auto-discovered widgets from enabled modules). Good direction. Full second-round review at dev_docs/pull_requests/2026/488-advanced-user-dashboard/CLAUDE_REVIEW_2.md.

TL;DR — progress, but still request changes. You've closed real ground from the first review (V97 no longer drops unrelated media tables, schema/table names match, phoenix_kit_current_scope is used, ModuleDiscovery is wired, Widget struct + enabled filter are clean). The overall shape of the feature is right.

However it still doesn't compile end-to-end. Here's the tightest path to mergeable, in priority order:

Just make it compile (CI is --warnings-as-errors)

  1. Template is brokenpriv/templates/user_dashboard_advanced_page.ex. After the generator's string substitution, page_title: @page_title becomes page_title: Example (bare identifier, not a string). Quote it: page_title: "<%= @page_title %>". Same for subtitle={@description}.
  2. get_session/put_session on a LiveView socket (lines 20, 49) — those are Plug.Conn functions; they'll raise at runtime. Thread session via mount(_params, session, socket) instead.
  3. w.Widget.x, w.Widget.y, etc. (template lines 124–127) — not valid field access. It's just w.x, w.y, w.w, w.h.
  4. title="Welcome, {@current_user.email}" in index.ex:39{…} inside a string literal is not HEEx interpolation; use title={"Welcome, " <> @current_user.email}.
  5. handle_event("save_grid", ...) builds layouts but never uses it, references undefined user, and has unreachable tail code.
  6. Kill warnings in lib/phoenix_kit/utils/widget.ex (unused user, module_name, self-alias at line 20), lib/phoenix_kit/dashboard/layout.ex:64 (unused user), and lib/mix/tasks/phoenix_kit.gen.user.dashboard.advanced.ex (unused import Plug.Conn, hooks_imports/0, parse_int/2). 7 warnings total = 7 CI failures.

Critical correctness (3-line fixes each)

  1. V97.down/0 doesn't roll back — drops an empty-column index, never drops the table, and resets the version comment to '97' instead of '96'. Needs drop_if_exists(table(:phoenix_kit_dashboard_widget_layouts)) and COMMENT ... IS '96'.
  2. Schema is missing timestamps(type: :utc_datetime) in lib/phoenix_kit/dashboard/layout.ex:6-17 — migration declares them, schema doesn't, so inserts will fail on NOT NULL.
  3. widgets_for/1 does get_by!(Layout, uuid: user.uuid) — queries by widget PK with user's uuid, always returns nil (rescued to []). Should be from(l in Layout, where: l.user_uuid == ^user.uuid) |> repo().all().
  4. upsert_layout/2conflict_target — unique index is [:user_uuid, :uuid] but conflict_target is [:user_uuid]. Will raise "no unique or exclusion constraint matching".
  5. Changeset unique_constraint name mismatch — line 27 uses :dashboard_layouts_user_uuid_index but migration creates :phoenix_kit_dashboard_widget_layouts_unique_index.

Hygiene (blocking)

  1. phoenix_kit.iml (IntelliJ, 278 lines) is still committed — first review flagged it. git rm phoenix_kit.iml and add to .gitignore.
  2. test/phoenix_kit/widgets/loader_test.exs aliases PhoenixKit.Widgets.Loader / PhoenixKit.Widgets.Widget — neither exists (real module is PhoenixKit.Utils.Widget). Update or delete.
  3. mix.exs version bump to 1.7.96 conflicts with main (main is already at 1.7.96). Rebase onto main and drop the bump.

Still carrying over from first review

  1. user_can_access_module?/2 at widget.ex:122 still hardcoded to true — comment says "PhoenixKit.Users.Permissions.user_can_access_module?(user, module_name)" but it's not called. Either wire the real check or fail closed for admin modules.

Medium bugs you'll want to hit before landing

  1. Dashboard.Index.mount reads session["phoenix_kit_current_user"] — that key isn't set by PhoenixKit auth. Use socket.assigns[:phoenix_kit_current_user]. Currently @current_user.email will crash render.
  2. Grid hook n.el.dataset.id vs template data-uuid={w.uuid} — mismatch means every save_grid push sends {id: undefined}. Pick one.
  3. Two context-menu JS files with different names (context_menu.js generated, dashboard_context_menu_remove.js committed). Pick one.

Sorry to hear about everything you're dealing with — no rush on our side. If pairing would help knock out items 1–6 together I'm happy to jump in; just ping here. The concept and half the scaffolding are solid, it's mostly the last-mile wiring that needs tightening.

@ddon

ddon commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

@mithereal my opinion that this is too big change for you for a start... too many things you touch, database migrations, schemas, etc... You should start with something more easier I think :) I am scared to even try for now... As you can see Claude created pretty long list of issues!

@mithereal
mitherealforce-pushed the main branch 2 times, most recently from 6165eb0 to 738d93fCompareApril 15, 2026 00:02
use create_or_update_file vs create_file
@mithereal
mitherealforce-pushed the main branch 2 times, most recently from 4b3d4fb to 2dd911aCompareApril 15, 2026 00:24
fix the sidebar toggle
@mithereal

Copy link
Copy Markdown
Author

i will refactor in a diff branch and reopen the pr

ddon pushed a commit that referenced this pull request Apr 15, 2026
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for freeto 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

@mithereal@ddon