Made logic to get mobile phone number overridable.

[kim-dongit]

Hi Michael,

another PR. I have put the logic to get the user's mobile phone number into a method of the trait. This is overridable by the developer in the model using the trait. So this way other field(s), model, or a non-database source could be used. And if the value would need tweaking, this is also possible,

This makes the add mobile migration fully optional. If you like the change, its functionality should be documented of course.

Best regards, Kim.

[michaeldzjap]

@kim-dongit I have to a bit about if I want to accept this PR for the following reasons:

1 - If you use another field, just any arbitrary field (e.g. one that is perhaps not even necessarily a mobile phone field) this would break the logic in MessageBirdVerify#sendSMSToken and the name of your method would still incorrectly imply it is mobile phone related. You could say that it is the responsibility of the user to ensure this logic stays in tact. Perhaps this is true, but nonetheless you open up more the possibility for this kind of thing to go wrong.
2 - With regards to your suggestions of using a non-database source; Although certainly debatable, I personally don't like the idea of handling any non-database related logic in a model (or model trait), even when it concerns a model related property (e.g. mobile phone number).

The only added benefit (but definitely one worth considering) is the fact that your PR makes it possible to use a relation instead of restricting it to a mobile column on User without breaking logic in MessageBirdVerify#sendSMSToken.

The more I think about it, the last point (i.e. relation argument) and the fact that it is only a small change is a good enough reason to accept the PR, but I want to think it over a bit more before making a definite decision.

[kim-dongit]

@michaeldzjap Thank you for your feedback and consideration. By default, everything would be as it was. So for existing applications using the package, everything would be the same. For existing (and new) applications not yet using the package, this will add flexibility. You mentioned the case that a relation might have a column with the mobile number, but the column could also be named differently, e.g. 'mobile_phone_number' or 'cell_phone'. It could also be stored in several columns, e.g. one for the country code, one for the area code and one for the subscriber number. You are right that the developer is responsible for providing a valid number to receive the sms, just as he is currently in the varchar field. I would like this change as in the case I am working on uses an existing database schema and the field is both in a relation of the user model and named differently.

[michaeldzjap]

You mentioned the case that a relation might have a column with the mobile number, but the column could also be named differently, e.g. 'mobile_phone_number' or 'cell_phone'. It could also be stored in several columns, e.g. one for the country code, one for the area code and one for the subscriber number.

Yes, good points. OK, you convinced me 😉 . Will merge it and release a patch version in the next couple of days.

[kim-dongit]

Great, thank you Michael!

[michaeldzjap]

@kim-dongit I just published v2.4.1 which includes this PR.

[kim-dongit]

@michaeldzjap Thank you!