-
-
Notifications
You must be signed in to change notification settings - Fork 834
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
Deleting driver from SMTP Server cause blank page/error #1169
Comments
Aka driver field should be mandatory or default back to smtp. |
Maybe add a dropdown list with all available and usable options will be the right choice. Extracted from here, options will be:
|
Drivers could be dynamically added via a package, not sure that whitelisting a few is smart. Unless extensions are allowed to add to the whitelist. |
One FreeFlarum user had his forum bricked by entering a bogus smtp driver (he entered |
I got the same error as mentioned above:
This error is caused by InstalledSite.php#L147. And also somehow while installing, it doesn't finishes the migration process. |
Okay my installation process failed because of extensions, which aren't updated ( The same error appreared for both of these extensions:
I'll look into these errors. --- Edit |
Hey folks, I just pushed a change to how mail drivers are registered and configured. This should make it much easier to build in validation of mail settings. Other ideas that should now be pretty easy:
Because of how annoying this is, I'd like to see this issue fixed in the next version, possibly including some of these suggestions. Nice chance for community members to step in. 😀 |
This can be used for e.g. validation, or a dropdown in the frontend. It can also be extended by extensions, such as flagrow/mail-drivers. Refs #1169.
@franzliedke related to ac5e26a#commitcomment-32420176 What is the suggested solution to get this fixed for beta 9? Right now it seems configuring mailgun for instance (needs key/secret/domain information) is impossible due to the exception thrown. Should I give the binding solution a go? |
Yes, please. |
This adds an interface for mail drivers to implement, defining several methods that we need throughout Flarum to configure, validate and use the various email drivers we can support through Laravel. More mail drivers can be added by `extend()`ing the container binding "mail.supported_drivers" with an arbitrary key and the name of a class that implements our new `DriverInterface`. This will ensure that drivers added by extensions can be properly built and validated, even in the frontend.
@luceos @clarkwinkelmann I just tweaked this a bit more: // For now, extending through private API
$container->extend('mail.supported_drivers', function ($drivers) {
return $drivers + ['fancymail' => MyFancyMailDriver::class];
});
// In the future, we may offer an extender for this use case. Next steps: Add more methods to the interface for other use cases, such as defining and validating required configuration. When that is done, getting the remaining email drivers from flagrow/mail-drivers into core should be very easy. (For beta.9, we may simply want to update that extension to use the new |
Okay, now I implemented the dropdown of available mail drivers, and made sure the SMTP settings are only shown when that driver is selected. Full TODO list:
|
First of all, initially I had no idea what approach you had in mind, now that you laid it out with subtasks it's much easier to comprehend. Thank you! Secondly do you really want to bloat core with all those drivers? It makes more sense to move them to their own extension and allow separate installation to suit the webmaster needs. Thirdly, with the tasks laid out is that something to delay beta 9 for? |
This includes an API endpoint for fetching the list of possible drivers and their configuration fields. In the future, this can be extended to include more meta information about each field.
Thanks for the reminder, I still had two unfinished commits lying around. Just pushed those and we now have support for more Laravel drivers, and their configuration through the admin UI. :) |
I am happy with this. The rest can be taken care of in flarum/issue-archive#252, #1763 and the already existing #1655. |
This can be used for e.g. validation, or a dropdown in the frontend. It can also be extended by extensions, such as flagrow/mail-drivers. Refs flarum#1169.
This adds an interface for mail drivers to implement, defining several methods that we need throughout Flarum to configure, validate and use the various email drivers we can support through Laravel. More mail drivers can be added by `extend()`ing the container binding "mail.supported_drivers" with an arbitrary key and the name of a class that implements our new `DriverInterface`. This will ensure that drivers added by extensions can be properly built and validated, even in the frontend.
This includes an API endpoint for fetching the list of possible drivers and their configuration fields. In the future, this can be extended to include more meta information about each field.
Bug report
Flarum info
Additional comments
After delete driver field into SMTP Server settings, we get an error:
Log files
The text was updated successfully, but these errors were encountered: