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

Fix for cascading mailer path being added when using partials in email templates #2233

Closed

Conversation

CyberFerret
Copy link
Contributor

As per #2231, when using multiple partials within an email template, the mailer name keeps getting added to the file path for each subsequent partial in the same template, resulting in an Errno::ENOENT: No such file or directory @ rb_sysopen error.

Added a check to settings.view in the render routine to see if the mailer path was already in the string before adding it.

… partials in email templates

As per padrino#2231, when using multiple partials within an email template, the mailer name keeps getting added to the file path for each subsequent partial in the same template, resulting in an `Errno::ENOENT: No such file or directory @ rb_sysopen` error.

Added a check to `settings.view` in the render routine to see if the mailer path was already in the string before adding it.
@nesquena
Copy link
Member

@CyberFerret Thanks for this. Could you fix this error as part of this PR or a separate, looks related to the mailer. Even better if you could add an extra unit test associated with this fix

@CyberFerret
Copy link
Contributor Author

@CyberFerret Thanks for this. Could you fix this error as part of this PR or a separate, looks related to the mailer. Even better if you could add an extra unit test associated with this fix

Odd - I just added an 'unless' qualifier to one existing line. The error mentions 'mailer' not being found - could be a dependency error? Was this caused by me forking the repo first in order to test my fix? I am not clear how to resolve this, because it appears to be in a section of the app that I didn't touch.

@nesquena
Copy link
Member

I understand, I think you are correct that it is somewhat unrelated, however it may be an easy fix and it would be nice to get the build passing in order to merge this. Separately, would you able to look into adding a test associated with this change?

@CyberFerret
Copy link
Contributor Author

This has been resolved via another PR that one of our developer submitted.

@CyberFerret CyberFerret closed this Sep 2, 2021
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.

2 participants