RabbitLabs/droplet-next
5
0
Fork 0
Code Issues 73 Pull requests 1 Projects 1 Releases Packages Wiki Activity Actions

Project Restructure #82

Open
Vylpes wants to merge 7 commits from feature/restructure into release/0.1.0
pull from: feature/restructure
merge into: RabbitLabs:release/0.1.0
Conversation 2 Commits 7 Files changed 117 +5427 -3363
Vylpes commented 2026-06-29 15:52:23 +01:00
Owner
Copy link

#81

#81
Vylpes 2026-06-29 15:52:31 +01:00
Smithy-bot requested changes 2026-06-29 16:00:19 +01:00
Smithy-bot left a comment
Copy link

PR Review: Project Restructure (#82)

Branch: feature/restructure → release/0.1.0
Issue: #81 (spike, empty description)
Scope: 6 commits, 117 files, +5,487 / −3,363
Build: Succeeds locally with 27 warnings, 0 errors
Verdict: Request changes — the structural direction is sound, but debug leftovers and a broken asset reference should be fixed before merge.

Summary

This PR does several things at once:

  1. Blazor Identity refactor — splits @code blocks into .razor.cs partials, adds AccountLayout for static rendering, and introduces IdentityUserAccessor.
  2. App cleanup — removes template pages (Counter, Weather), passkey UI, and simplifies nav/layout.
  3. Product/docs foundation — ADR, PRD, epics, stories, testing guide, email providers doc.
  4. Tooling — Forgejo issue/PR templates and Cursor skills.

The Identity refactor is the right shape for a maintainable Blazor Server app. The main problems are accidental debug instrumentation and an orphaned script reference, not the architecture itself.

Blockers (fix before merge)

1. Debug instrumentation left in source

Cursor debug logging is still wired into production code with a hardcoded absolute path to another checkout (/home/vylpes/projects/droplet-next/.cursor/debug-99de3c.log).

Affected files:

  • Program.cs
  • Components/Account/Shared/AccountLayout.razor.cs
  • Components/Account/IdentityRedirectManager.cs
  • Components/Account/Pages/Login.razor.cs
  • Components/Account/Pages/Manage/Index.razor.cs

This will silently no-op on other machines, but it is still noise in startup, redirects, and login flows and should not ship.

2. Committed debug log file

.cursor/debug-99de3c.log is included in the PR and contains localhost URLs and session metadata. Remove it and add .cursor/debug*.log (or .cursor/) to .gitignore.

3. Broken script reference after passkey removal

Passkey components were deleted, but App.razor still loads the removed PasskeySubmit.razor.js script. This will cause a 404 on every page load.

Important (should address)

4. IdentityUserAccessor does not stop execution after redirect

In IdentityUserAccessor.GetRequiredUserAsync, after RedirectToWithStatus is called when user is null, execution continues and return user can return null (CS8603). The usual Blazor pattern is throw new InvalidOperationException("Redirect") after redirect, or make the return type nullable and handle it at call sites.

5. Login Remember me removed from UI but still in logic

Login.razor.cs still passes Input.RememberMe to PasswordSignInAsync and the 2FA redirect, but Login.razor no longer renders the checkbox — so remember-me is always false. Either restore the UI or remove the property from the model/flow.

6. Passkeys removed without a follow-up note

Removed: Passkeys.razor, RenamePasskey.razor, PasskeySubmit, related models, and manage-nav links. The DB schema still has AspNetUserPasskeys from the initial migration. If this is intentional for v1, call it out in the PR description or a short ADR/note so it is not mistaken for an accident.

7. ExternalLoginPicker is now dead code

The component still exists but is not referenced from Login.razor or Register.razor. Remove it or wire it back in if external auth is planned.

Minor / polish

  • PR description — Body is only #81. The PR adds .forgejo/PULL_REQUEST_TEMPLATE.md but does not use it.
  • Scope — Spike label, but the PR bundles code refactor, docs (~2k lines), Cursor skills, and Forgejo templates. Consider splitting docs/tooling from the code change for easier review.
  • Duplicate epic-9 stories — Both epic-9-cicd-forgejo-actions.md and epic-9-cicd-github-actions.md — pick one canonical CI story.
  • Home page links — Point to github.com/Vylpes/droplet-next; repo is RabbitLabs/droplet-next on Forgejo.
  • Namespace split — Droplet.Auth.Components.Account vs Droplet.Components.Account.* — works, but inconsistent. Worth standardizing in a follow-up.
  • 27 build warnings — Mostly nullable reference (CS8604) and Blazor BL0008 ([SupplyParameterFromForm] + initializer). Pre-existing template pattern, but worth cleaning up incrementally.
  • Empty RedirectToDashboard.razor — Valid with code-behind, but adding a one-line markup comment would make intent clearer.

What looks good

  • Code-behind split — Markup and logic separation across Identity pages is a solid maintainability win.
  • AccountLayout + NavigationManager.Refresh — Correct pattern for Identity pages that need HttpContext during static render.
  • IdentityUserAccessor registration in Program.cs — Fixes the DI issue from the debugging commits.
  • Nav simplification — Bootstrap Icons vendored locally; gear icon and dashboard link are sensible.
  • Authenticated home → dashboard redirect — Clean UX via RedirectToDashboard.
  • Documentation — ADR, PRD, epics, and stories give the project a real product backbone.
  • Forgejo templates + Cursor skills — Useful for ongoing issue/PR workflow.

Suggested test plan

  • Remove debug code/log; confirm dotnet build is clean of agent-log regions
  • Load any page — no 404 for PasskeySubmit.razor.js in network tab
  • Login → lands on /dashboard when authenticated
  • Visit / while logged in → redirects to dashboard
  • Account manage pages (Profile, Email, Password, 2FA, Personal data) load without infinite refresh
  • Logout from nav works
  • 2FA login flow still works (hidden RememberMe propagates correctly)
  • Icons render in nav (bootstrap-icons)

Recommended next steps

  1. Strip all #region agent log blocks and delete .cursor/debug-99de3c.log.
  2. Remove the PasskeySubmit.razor.js script tag from App.razor.
  3. Fix IdentityUserAccessor control flow after redirect.
  4. Flesh out the PR description (summary, passkey decision, test plan).
  5. Re-request review.
## PR Review: Project Restructure (#82) **Branch:** `feature/restructure` → `release/0.1.0` **Issue:** #81 (spike, empty description) **Scope:** 6 commits, 117 files, +5,487 / −3,363 **Build:** Succeeds locally with 27 warnings, 0 errors **Verdict:** **Request changes** — the structural direction is sound, but debug leftovers and a broken asset reference should be fixed before merge. --- ### Summary This PR does several things at once: 1. **Blazor Identity refactor** — splits `@code` blocks into `.razor.cs` partials, adds `AccountLayout` for static rendering, and introduces `IdentityUserAccessor`. 2. **App cleanup** — removes template pages (`Counter`, `Weather`), passkey UI, and simplifies nav/layout. 3. **Product/docs foundation** — ADR, PRD, epics, stories, testing guide, email providers doc. 4. **Tooling** — Forgejo issue/PR templates and Cursor skills. The Identity refactor is the right shape for a maintainable Blazor Server app. The main problems are accidental debug instrumentation and an orphaned script reference, not the architecture itself. --- ### Blockers (fix before merge) #### 1. Debug instrumentation left in source Cursor debug logging is still wired into production code with a **hardcoded absolute path** to another checkout (`/home/vylpes/projects/droplet-next/.cursor/debug-99de3c.log`). Affected files: - `Program.cs` - `Components/Account/Shared/AccountLayout.razor.cs` - `Components/Account/IdentityRedirectManager.cs` - `Components/Account/Pages/Login.razor.cs` - `Components/Account/Pages/Manage/Index.razor.cs` This will silently no-op on other machines, but it is still noise in startup, redirects, and login flows and should not ship. #### 2. Committed debug log file `.cursor/debug-99de3c.log` is included in the PR and contains localhost URLs and session metadata. Remove it and add `.cursor/debug*.log` (or `.cursor/`) to `.gitignore`. #### 3. Broken script reference after passkey removal Passkey components were deleted, but `App.razor` still loads the removed `PasskeySubmit.razor.js` script. This will cause a 404 on every page load. --- ### Important (should address) #### 4. `IdentityUserAccessor` does not stop execution after redirect In `IdentityUserAccessor.GetRequiredUserAsync`, after `RedirectToWithStatus` is called when `user is null`, execution continues and `return user` can return `null` (CS8603). The usual Blazor pattern is `throw new InvalidOperationException("Redirect")` after redirect, or make the return type nullable and handle it at call sites. #### 5. Login Remember me removed from UI but still in logic `Login.razor.cs` still passes `Input.RememberMe` to `PasswordSignInAsync` and the 2FA redirect, but `Login.razor` no longer renders the checkbox — so remember-me is always `false`. Either restore the UI or remove the property from the model/flow. #### 6. Passkeys removed without a follow-up note Removed: `Passkeys.razor`, `RenamePasskey.razor`, `PasskeySubmit`, related models, and manage-nav links. The DB schema still has `AspNetUserPasskeys` from the initial migration. If this is intentional for v1, call it out in the PR description or a short ADR/note so it is not mistaken for an accident. #### 7. `ExternalLoginPicker` is now dead code The component still exists but is not referenced from `Login.razor` or `Register.razor`. Remove it or wire it back in if external auth is planned. --- ### Minor / polish - **PR description** — Body is only `#81`. The PR adds `.forgejo/PULL_REQUEST_TEMPLATE.md` but does not use it. - **Scope** — Spike label, but the PR bundles code refactor, docs (~2k lines), Cursor skills, and Forgejo templates. Consider splitting docs/tooling from the code change for easier review. - **Duplicate epic-9 stories** — Both `epic-9-cicd-forgejo-actions.md` and `epic-9-cicd-github-actions.md` — pick one canonical CI story. - **Home page links** — Point to `github.com/Vylpes/droplet-next`; repo is `RabbitLabs/droplet-next` on Forgejo. - **Namespace split** — `Droplet.Auth.Components.Account` vs `Droplet.Components.Account.*` — works, but inconsistent. Worth standardizing in a follow-up. - **27 build warnings** — Mostly nullable reference (CS8604) and Blazor `BL0008` (`[SupplyParameterFromForm]` + initializer). Pre-existing template pattern, but worth cleaning up incrementally. - **Empty `RedirectToDashboard.razor`** — Valid with code-behind, but adding a one-line markup comment would make intent clearer. --- ### What looks good - **Code-behind split** — Markup and logic separation across Identity pages is a solid maintainability win. - **`AccountLayout` + `NavigationManager.Refresh`** — Correct pattern for Identity pages that need `HttpContext` during static render. - **`IdentityUserAccessor` registration in `Program.cs`** — Fixes the DI issue from the debugging commits. - **Nav simplification** — Bootstrap Icons vendored locally; gear icon and dashboard link are sensible. - **Authenticated home → dashboard redirect** — Clean UX via `RedirectToDashboard`. - **Documentation** — ADR, PRD, epics, and stories give the project a real product backbone. - **Forgejo templates + Cursor skills** — Useful for ongoing issue/PR workflow. --- ### Suggested test plan - [x] Remove debug code/log; confirm `dotnet build` is clean of agent-log regions - [ ] Load any page — no 404 for `PasskeySubmit.razor.js` in network tab - [ ] Login → lands on `/dashboard` when authenticated - [ ] Visit `/` while logged in → redirects to dashboard - [ ] Account manage pages (Profile, Email, Password, 2FA, Personal data) load without infinite refresh - [ ] Logout from nav works - [ ] 2FA login flow still works (hidden `RememberMe` propagates correctly) - [ ] Icons render in nav (bootstrap-icons) --- ### Recommended next steps 1. Strip all `#region agent log` blocks and delete `.cursor/debug-99de3c.log`. 2. Remove the `PasskeySubmit.razor.js` script tag from `App.razor`. 3. Fix `IdentityUserAccessor` control flow after redirect. 4. Flesh out the PR description (summary, passkey decision, test plan). 5. Re-request review.
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feature/restructure:feature/restructure
git switch feature/restructure

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch release/0.1.0
git merge --no-ff feature/restructure
git switch feature/restructure
git rebase release/0.1.0
git switch release/0.1.0
git merge --ff-only feature/restructure
git switch feature/restructure
git rebase release/0.1.0
git switch release/0.1.0
git merge --no-ff feature/restructure
git switch release/0.1.0
git merge --squash feature/restructure
git switch release/0.1.0
git merge --ff-only feature/restructure
git switch release/0.1.0
git merge feature/restructure
git push origin release/0.1.0
Sign in to join this conversation.
Reviewers
No reviewers
Smithy-bot
Labels
Clear labels
  
blocked

   
duplicate
  
needs
approval

  
needs
criteria

  
needs
estimate

  
needs
tests

  
question
  
type
admin

  
type
alert

  
type
bug

  
type
change

  
type
defect

  
type
dependencies

  
type
epic

  
type
idea

  
type
incident

  
type
investigation

  
type
spike

  
type
story

  
won't fix
No labels
blocked
duplicate
needs
approval
needs
criteria
needs
estimate
needs
tests
question
type
admin
type
alert
type
bug
type
change
type
defect
type
dependencies
type
epic
type
idea
type
incident
type
investigation
type
spike
type
story
won't fix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
Smithy-bot
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
RabbitLabs/droplet-next!82
No description provided.