Fixes #8525 - Rename "Mail" to "Email" in user preferences

[ShimShtein]

No description provided.

[orrabin]

[test]

[dLobatog]

Thanks! A few comments:

• .po files shouldn't be modified, they are auto generated. You can override the label in app/views/users/_form:42
• More instances that should be changed:
  • ../cleanforeman/app/models/setting/general.rb:18
  • ../cleanforeman/app/controllers/application_controller.rb:83
  • mail notifications code, API params and so on.

@ohadlevy @domcleal I think we're almost consistent on the use of 'mail' across the whole UI, and most of the code uses Mail, so I can't say I understand the usefulness of this issue. Your thoughts?

@ShimShtein please hold off making further changes until more people comment on this, at first glance it seems like a non-issue and we should actually turn the few 'email' to 'mail', but it might just be me!

[domcleal]

I don't think the code versus the UI is particularly important, but it should be consistent within the UI, either way. I suppose I'd have preferred to stick with "email" as that was already in use before 3a36bdf introduced a second term, but perhaps "mail" would be simpler at this stage.

Editing the en .po file is fine in order to override the default attribute labels in the UI, or add a :label to app/views/users/_form.html.erb.

[ShimShtein]

I didn't want to break translations, so I changed the .po file.
The code is pretty consistent with "mail" as a field name - wouldn't try to change that, besides that there's no reasons for confusion - we are not referring to different types of mail anywhere.
I am not an English expert, but personally I prefer the "Email" consistency in the UI, as @domcleal has noted.
About two more files that @elobato has pointed:

• /app/controllers/application_controller.rb:83, should be fixed the same way - I will add it in the moment we agree on a proper way to change it.
• /app/models/setting/general.rb:18, needs to be changed inline - didn't find any mentioning of this string in other files.

[dLobatog]

@ShimShtein @domcleal @stbenjam ping.

At this point I guess if this goes in we'll have to change all the UI for mail notifications to email notifications right? Otherwise this PR is not really keeping it consistent. I do prefer 'email' too but 'mail' would take way less effort.

[ShimShtein]

@elobato @domcleal What do you say about this version? Looks pretty consistent to me...

[ohadlevy]

@elobato ping?

[domcleal]

don't change this, it's the name of a standard LDAP attribute

[ShimShtein]

Thanks @domcleal, reverted the change in the latest commit.

[domcleal]

So I think this is okay by me, the only inconsistencies that remain are a) internal stuff, or b) the API. Hopefully when somebody gets around to implementing this in Hammer they can paper over the latter.

@elobato did you have any further concerns?

[dLobatog]

Merged as 416a0ae, thanks @ShimShtein!