Skip to content

Updated invitation flow#1078

Open
korridor wants to merge 23 commits into
mainfrom
feature/user-management
Open

Updated invitation flow#1078
korridor wants to merge 23 commits into
mainfrom
feature/user-management

Conversation

@korridor
Copy link
Copy Markdown
Contributor

@korridor korridor commented May 20, 2026

What does this PR do?

  • Moved jetstream function to REST endpoints
  • Lower case email

Checklist (DO NOT REMOVE)

@codecov
Copy link
Copy Markdown

codecov Bot commented May 20, 2026

Codecov Report

❌ Patch coverage is 96.27792% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.99%. Comparing base (8eab048) to head (f32ec59).

Files with missing lines Patch % Lines
app/Support/Base64File.php 77.77% 4 Missing ⚠️
app/Service/UserService.php 86.95% 3 Missing ⚠️
app/Filament/Resources/UserResource.php 60.00% 2 Missing ⚠️
app/Http/Controllers/Web/UserController.php 92.00% 2 Missing ⚠️
app/Policies/OrganizationPolicy.php 0.00% 2 Missing ⚠️
app/Actions/Jetstream/AddOrganizationMember.php 0.00% 1 Missing ⚠️
app/Http/Controllers/Controller.php 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1078      +/-   ##
============================================
- Coverage     89.06%   88.99%   -0.08%     
- Complexity     1875     1954      +79     
============================================
  Files           275      283       +8     
  Lines          9733     9776      +43     
============================================
+ Hits           8669     8700      +31     
- Misses         1064     1076      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@korridor korridor force-pushed the feature/user-management branch 2 times, most recently from b68825c to 3267acb Compare May 20, 2026 14:25

$user->switchTeam($organization);

// Note: The refresh is necessary for currently unknown reasons. Do not remove it.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we know why it is needed now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the switchTeam or the refresh?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refresh. The comment just sounded like a leftover.

Copy link
Copy Markdown
Contributor Author

@korridor korridor May 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this might be a leftover, but I'll move the switchTeam to the UserService anyways to further reduce the dependency on Jetstream and then remove the refresh.

@Onatcer
Copy link
Copy Markdown
Contributor

Onatcer commented May 20, 2026

@korridor I did change it to a session state now after all because the Inertia URL handling gets flaky once I start messing with the URLs on the client side (Inertia has its own URL state, which overrides it again). We can revisit this when we eventually migrate to a SPA and have full URL control there. E2E tests are in place so it should be an easy migration after the fact.

I also changed the logged-out redirect and added an Auth check there for the case detection. I think this is cleaner than messing with the middleware behaviour and I don't really see a reason why there shouldn't be an Auth check in this controller especially when we just use it for the determining the redirect.

Arguably, the UX path for "A different user is logged in and accepts the invite" is a bit confusing because they get a message that they accepted an invite to an organization but the logged in user cannot actually see the org (because the invite was accepted for a different user). It is probably an edge case tho.

@Onatcer
Copy link
Copy Markdown
Contributor

Onatcer commented May 26, 2026

the photo delete path was not supported, it now takes {photo: null} as delete and a missing photo key as "do not touch". seperate endpoints for photo create and delete would probably be cleaner but i think its ok for now.

@Onatcer
Copy link
Copy Markdown
Contributor

Onatcer commented May 26, 2026

@korridor there is currently no way to reset the email_pending as far as I can see. so when you change the email address but decide afterwards that you actually don't wanna change it, it just stays in email_pending forever.

we could either make it so that the endpoint resets email_pending if the email in the db === request email and email_pending !== null (and the frontend doesn't resend the email if it didn't change) or have a separate endpoint for it. in the frontend, it would be a Cancel button either way.

Comment thread resources/js/Pages/Profile/Partials/UpdateProfileInformationForm.vue Dismissed
@Onatcer Onatcer force-pushed the feature/user-management branch 3 times, most recently from 61ae1f2 to c8623b7 Compare May 27, 2026 17:06
@Onatcer
Copy link
Copy Markdown
Contributor

Onatcer commented May 27, 2026

profile information update and user delete are now moved to the API. I do the password check manually now in the frontend and confirm it with the fortify password.confirm route, technically it is however way easier now to just straight up delete the account with an endpoint or caused by some rogue script/extension. we could add a password check/middleware again to the delete endpoint, not sure if its really necessary tho. it would be kinda weird for an API endpoint.

@Onatcer Onatcer force-pushed the feature/user-management branch from dc5e8e7 to 71a7eeb Compare May 29, 2026 15:06
@Onatcer Onatcer force-pushed the feature/user-management branch from 71a7eeb to f32ec59 Compare May 29, 2026 15:45
@Onatcer Onatcer marked this pull request as ready for review May 29, 2026 15:51
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.

3 participants