Add resend route

[lukasoppermann]

I think resending a verification email is a very typical thing, I see it on nearly every platform. I think it would be good to add a default route that does the resend and redirects back to the page the user comes from.

[jrean]

I thought about it and yes, I agree with you. It should be offered as a core functionality.
Can you implement and make a PR?
Thank you for the contribution!

[lukasoppermann]

I will make a PR once #50 is merged. Also this makes #49 unnecessary, so once you merge, I send a new PR and this is merged, I will remove the PR #49.

[jrean]

You can process.
Thank you

[lukasoppermann]

Hey, I would like to move the resend method to a controller within the package and while I am doing this, also move the other two methods. I would like to create a UserVerificationController within the package and move all methods to it.

This can be very beneficial, as we might get to a point in the future, where the user does not have to edit the AuthController at all.

Especially if this laravel/ideas#197 gets resolved. In that case we could just add a config file where the user can toggle if an email is sent on registration and he does not have to touch other files at all. This would make installing very easy and also let us refactor stuff more easily, without having users update anything.

[jrean]

Can we stick with the trait for the first release and see how things are going. I'm not yet sure for the need of a controller.

laravel/ideas#197
You said the user is asked to add 2 traits. No, only one trait.

[jrean]

So you would remove the trait and create a controller for getVerification and getVerificationError and getResendVerification? I'm not against. I just try to see the benefits

[lukasoppermann]

Yes, basically.

The benefit would be that we will soon be able to offer a much smoother installation without the user changing her AuthController.

It would separate the package from the framework, which would also possibly be beneficial from a maintenance point of view if something changes, e.g. the AuthController will be removed in the future, or changed, or moved into the core, etc. Whatever happens, we would not need to care.

However until the event is merged we would need to keep the trait to capture the register event.

[lukasoppermann]

So can we go with the new controller? I would make the change in the PR for the resend link, as the changes are actually quite minimal. I would leave the trait in the package and mark it as deprecated, so that people who use the "old" method with their own routes do not have any with a new release.

[fake-fur]

I have existing code from previous version that will now be obsolete with these changes, so will you also add the documentation for people to upgrade existing code to this version please?

[lukasoppermann]

Hey @fake-fur are you asking for an "upgrade to version XY" guide or just for an update of the docs showing the new "features"?

The docs will definitely be updated to reflect any changes. I am not sure an update guide is really needed, as long as backwards compatibility is not broken. You are free to keep your implementation and it should not be a problem.

Am I missing something?

[fake-fur]

well both docs I would imagine no? you are bringing in changes that can break existing code when "composer update" happens no?

[lukasoppermann]

I don't think this change will break anything at all. A composer update should be perfectly save. What would you think could break because of this change?

[fake-fur]

name clashes and having code that targets an old obsolete way of using this package is not a great way to move forward for anyone already using this package no?

[fake-fur]

I guess I'm not against updates but making all existing code essentially obsolete does not help people who have invested time into using this package alreafy

[lukasoppermann]

I would disagree. If it makes the code obsolete you can really care less. Some feature was not there you needed it, you wrote it yourself, now its there, but your implementation still works.

If it breaks your code, however this will pose a problem.

The idea of a package is basically to have one group maintain a code base instead of everybody maintaining there own code base.

As stated above the trait will not be removed for now, so you implementation will not break. This PR should actually not make any work for you.

And in case any feature does break bc, than this is why we have semver. Just rely on the 3.0.* tag and you should be fine.

[fake-fur]

right yes i agree - i am just making the case that existing users of the older codebase should not be left with potentially broken or obsolete code - making a new version tag would be a good solution to allow for graceful upgrades with newer projects, and planned upgrades with older codebases if desired

oftentimes these kind of issues get left behind in the rush for the new shiny

thanks for being aware of them

[lukasoppermann]

Sure no problem. @jrean does a very nice job at tagging every change in a new release.

[lukasoppermann]

Hey @jrean you might have missed my question from above:

So can we go with the new controller? I would make the change in the PR for the resend link, as the changes are actually quite minimal. I would leave the trait in the package and mark it as deprecated, so that people who use the "old" method with their own routes do not have any with a new release.

[jrean]

Yes please, implement, send a PR. I will review and test everything. I reckon it's a wise move.
Thank you

[lukasoppermann]

Thanks for the reply.