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

only validate SMTP_FROM if necessary #5442

Merged
merged 1 commit into from
Jan 25, 2025

Conversation

stefan0xC
Copy link
Contributor

#5438 seems to break setups by always requiring smtp_from to be a valid email, even if mail is disabled

reverted part of the change to fix #5441

@BlackDex
Copy link
Collaborator

Thanks, i overlooked that _enable_smtp could be true even if those items were not set.

@BlackDex BlackDex merged commit 2903a3a into dani-garcia:main Jan 25, 2025
5 checks passed
@stefan0xC stefan0xC deleted the fix-smtp-from-check branch January 25, 2025 04:47
@stefan0xC
Copy link
Contributor Author

stefan0xC commented Jan 25, 2025

Yeah, I mean we might want to switch it so that it's generated from either of those option being set, but I'm not sure if this would break things. I have not tested it but I think it might just work. I mean, if you have saved the config.json with mail disabled, you'd probably also have to also enable it manually after configuring your SMTP settings but since it's the first option people who configure stuff via the /admin panel would probably see it, so this should be fine.

edit: oh, except that for people that have _enable_smtp already enabled we still could not remove this check, since they would then face the same issue.

@BlackDex
Copy link
Collaborator

I think it's more that we want someone to disable smtp while keeping there settings. Useful if your SMTP provider is offline for some reason.

@stefan0xC
Copy link
Contributor Author

Ah, right. I also tend to forget how generated works (and probably meant auto). But this probably wouldn't work without breaking things.

@BlackDex BlackDex mentioned this pull request Jan 28, 2025
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.

Error loading config:SMTP_FROM '' is not a valid email address
2 participants