Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set the mail username optional in setup #3294

Merged
merged 2 commits into from
Dec 29, 2018

Conversation

nalysius
Copy link
Contributor

During the setup process email informations were asked:

  • Driver
  • Host
  • Username
  • Password

In some situations the username is not useful because the Cachet's
host may be configured to forward email to a server.

The problem is the username was required, so we had to set a username
and then update the .env file to remove it.

To fix this problem, the mail username has been set to optional in
the setup. So if someone needs a username it still can use this field,
and otherwise people can let it empty.

See: #3244

@nalysius nalysius changed the title Set the mail username optional in setup [WIP] Set the mail username optional in setup Oct 20, 2018
@nalysius
Copy link
Contributor Author

I've updated the SetupController, but I've to change because for now if MAIL_USERNAME is null the Subscribe button will never be viewable.

@jbrooksuk
Copy link
Member

@anthonybocci did we not change the subscriber test to simply be whether the setting is enabled or not?

@jbrooksuk jbrooksuk self-requested a review December 27, 2018 22:54
@jbrooksuk jbrooksuk added this to the V2.4.0 milestone Dec 27, 2018
During the setup process email informations were asked:
  - Driver
  - Host
  - Username
  - Password

In some situations the username is not useful because the Cachet's
host may be configured to forward email to a server.

The problem is the username was required, so we had to set a username
and then update the .env file to remove it.

To fix this problem, the mail username has been set to optional in
the setup. So if someone needs a username it still can use this field,
and otherwise people can let it empty.

See: cachethq#3244
During the setup the "mail_username" was required and it was then
undone, so using the sendmail driver we can let the username empty.

It would be bad to let the username optional for every drivers, because
in some configurations, like SMTP, the username is required for the SMTP
server so if the user let it empty its mail configuration will be bad.

The mail_username is now optional only if the mail driver is sendmail.

See: cachethq#3244
@nalysius
Copy link
Contributor Author

@jbrooksuk I just checked, your right the test has been updated.
So, i've just udpated the file so the username is optional only for the sendmail driver.

@jbrooksuk
Copy link
Member

Okay, cool! I’m on my phone but I think this can be updated from WIP and then merged.

@nalysius nalysius changed the title [WIP] Set the mail username optional in setup Set the mail username optional in setup Dec 29, 2018
@nalysius
Copy link
Contributor Author

@jbrooksuk I've removed the "WIP", you can check (because I'm not familiar with the Laravel validator) :)

@jbrooksuk
Copy link
Member

LGTM!

@jbrooksuk jbrooksuk merged commit e857143 into cachethq:2.4 Dec 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants