Skip to content

Verification improvements #1051

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Verification improvements #1051

wants to merge 2 commits into from

Conversation

talelmishali
Copy link
Contributor

@talelmishali talelmishali commented May 11, 2022

This PR is to fix a new bug introduced after the recent improvement made on #1048. Upon new Jetstream installation where the user does not enable Fortify feature for email verification Features::emailVerification(), the route verification.send is not registered and can create an error while trying to display the button for re-sending email verification link.

On the Inertia stack we can see it here


As it tries to render verification.send route, we will receive:

Uncaught (in promise) Error: Ziggy error: route 'verification.send' is not in the route list.

Currently, the PR fixes the bug only on the Inertia stack, I'll wait to hear from the Laravel team for what direction would like to go with it before spending more time on the Livewire stack fix.

Thanks again for all the hard work,
Tal

- Show re-send verification email only if the user has to verify email and route exists.
Fix StyleCI check
@driesvints
Copy link
Member

Hey @talelmishali. It seems you sent this in as a draft PR. Those aren't reviewed (See our contributor guide). I've sent in an alternative to this one: #1053

Thanks!

@driesvints driesvints closed this May 13, 2022
@talelmishali
Copy link
Contributor Author

Hey @talelmishali. It seems you sent this in as a draft PR. Those aren't reviewed (See our contributor guide). I've sent in an alternative to this one: #1053

Thanks!

I'll know for next time, I didn't want to send a PR that's is not fully ready to be merged. Anyway, thanks for the quick fix.

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