Update users email to be optional

As mentioned in this issue, users’ email is not mandatory on creation but it is on update.

To be more specific, the email is mandatory only if the user already has an email saved before.
So you will be able to update a user with an empty email if the user didn’t have an email before.

My question is - do you think we should keep this behavior, or maybe we should always allow to update a user with an empty email address?

The line in the code we would have to change (delete the last brackets after the &&) - https://github.com/theforeman/foreman/blob/9374e4c5b5fe21b50b1aa2ec39579d6d53e90d75/app/models/user.rb#L86

This subject is also discussed here.

Any suggestions on this subject are more than welcome!

5 Likes

Sounds good to me, I saw this issue in a few other discussions when trying to log into Foreman with external users (FreeIPA / AD)

Yes, this solves logging in to the WebUI whenever the authentication system is unable to provide the user’s e-mail address.

And as I mentioned in the other thread, I legitimately don’t know why Foreman would want to send me e-mail :wink:

1 Like

As email notification is optional and I do not know many environments where it is used at all, I think having it optional is good.

I am not sure if we can get then a situation where someone tries to enable notification without Foreman being configured for it and/or the account not having an email set. Perhaps unlikely, but worth a catch?

My two cents - I think it’s reasonable change because such situation can happen even today. As part of the change, we need to make sure the email notifications code does not fail when there’s some user with notifications configured without an email address. It should probably just log a warning to STDOUT, since it’s triggered from cron, that would then result in email to the sysadmin. Nofar, let me know if you need any more details on this, I’m happy to show you how to configure such scenario.

I’d wait until this is opened for 2 weeks, if we don’t get any negative feedback, I think we can consider it agreed.

3 Likes

Alright, it has been opened for 2 weeks, I don’t see a way to mark the original post as a solution, but I think we can consider this accepted and we can proceed with https://github.com/theforeman/foreman/pull/9279

This is indeed not supported. Discourse views it as a question and you don’t answer your own question. Only replies can be solutions. I think your comment can be seen as a solution, so I did that.

Also, it’s in the developers section, not RFCs so I’ve also moved it. On developers you can’t mark things as resolved.