Update backpack_users_have_email helper and add email_column to config

[jcastroa87]

WHY

This PR was explain in https://github.com/Laravel-Backpack/CRUD/pull/4225

BEFORE - What was wrong? What was happening before this PR?

If we change email column on user table, route:list fail and backpack_users_have_email always will return false, even if foo column contain email

AFTER - What is happening after this PR?

Now developers can set username columna and email column and could be different columns

HOW

How did you achieve that, in technical terms?

Update backpack_users_have_email, now developers are free to use any column as username and any as email, they can set this in base.php

Is it a breaking change?

No, but maybe because is needed update base.php config file

How can we test the before & after?

Change name to email column on user table and set it on base.php

[pxpm]

Hello @jorgetwgroup I was not able to reproduce the route:list issue, could you ? I changed my auth column to be username and deleted the email column from database and I could still route:list.

Having this now on the config led me to think that helper now is an overkill, since you can:

• email_column => email
• email_column => email_with_other_name
• email_column => false

Does we really still need to have the helper at all ? Shouldn't the developer provided configuration be enough ?

Also I think this PR needs to address this change in some other places like: https://github.com/Laravel-Backpack/CRUD/blob/057588417a9ae5ed780329de76cc9de406d58072/src/resources/views/base/auth/register.blade.php#L30

Because changing the email_column to have another name should not influence the field type as long as it still an email column.

Let me know, Pedro

[jcastroa87]

Hello @pxpm

Are you sure you can not reproduce the error?

I have CRUD and PRO on main branch.

config as this

User table have not "email" name column

And with "php artisan route:list" get this:

This is because helper function always try to find "email" column, then without email column always will fail and thats stop list to show

@tabacitu can you check on your side if can reproduce the warning error please.

Cheers.

[pxpm]

@jorgetwgroup I could reproduce it now. Sorry, probably doing something dumb I didn't noticed. 🙃

Let's address the other issues I raised!

Cheers

[jcastroa87]

Hi @pxpm

It was not so easy, but is done.

Now is possible fully replace email column. Lets go step by step:

1. In config/backpack/base.php there is options:

` 'authentication_column' => 'username', 'authentication_column_name' => 'Username',

'email_column' => 'other_email', `

Is possible set authentication_column as any column, diff than "email", same to email_column, could be anything (of course data need to be real email)

2. If "email_column" is set different than "email" is needed override trait on User Model, then, add:

` use CanResetPassword;

public function getEmailForPasswordReset() { return $this->{backpack_email_column()}; } `

If authentication_column = email_column, then login, register and reset password will check if format is email, is this is different, then will set input as text type.

I recommend tag this as "breaking change", because if not use email is needed to replace stuff in Model.

I just think maybe there is a few translation i didnt pay attention, example on reset form say "Confirm email", maybe need to be replace, because if username is not an email, could be named as "Confirm username".

Btw this add the featured to have repeat emails on user table :)

Cheers.

[tabacitu]

Well... this took a turn for the worse 😅

@pxpm I don't know what your feedback was, but it may have made this PR spiral out of control 😅 Previously, it was small, non-breaking and easy to merge. Now... with 19 files changed... I hope you know me well enough to realise this is never going to make it 😅

Sorry guys, I'd rather not fix the problem, than make a mess in so many files. Making changes in the authentication, something that 100% of our customers use, should be made with surgical precision. I know these are all "small changes", but 19 "small changes" turn this into "a big change".

Rule of thumb: The fix should ALWAYS be appropriate to the problem. If the problem is small, the fix must be small. Otherwise it's not worth it. Either we can go back to a small, clean PR (even if it has caveats), or we say "sorry, can't do it, too messy".

Thank youuu 🙏

[tabacitu]

Change of plans - if you guys want to merge this, we'll merge this. But keep in mind this is a FEATURE now, not a BUG FIX. So we'll treat it as such. Test test test test.

@pxpm & @jorgetwgroup - you two decide what we do with this.

[tabacitu]

@jorgetwgroup - just talked about this. Pedro said (and I agree) that it's better for us to postpone merging this until February, when we launch the next version. Way too many files changed, that'd give us more confidence that we don't break people's apps by mistake. People have customized Backpack in ways we can't even imagine 😀

PS. We have a new tag: next-version that we'll use for that too, so we can filter out the PRs/issues that are of no concern at the moment.