WIP mec's improvement planning #104
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Looking at ways to improve the Gem and fix support for Rails > 7.0.8 Overriding the Rails::MailerController is a recipie for pain and is the source of the issue we see in Rails > 7.0.8. Right now I think all this does is allows a custom layout (the GOV.UK one) - we can do that without the heavy handiness by suppling a layout in the Mailer class - this commit contains that change. To support the layout simply being in the views, we have to become a 'Engine' so we can add views by replicating the Rails directories (there are other ways but this is nice and simple). Becuase engines are just Railties, everything should work as it did, again that change is here - needs better naming. https://api.rubyonrails.org/classes/Rails/Engine.html With all that done, the previews just work™ the mailer suppling the layout overide for Mail::Notify::Mailer subclasses only. I've only checked the 'view' version so far - but I can't see it being a problem for the 'template' version. With these changes all of the rails stuff in the specs is redundent, I think - so everything becomes much simpler. Right now I am testing this with a fully fledged Rails 7.1.2 install - it would be nice to come up with a way to run 'tests' in all 3 major versions of Rails - may Docker could help? I would like to get this working and go for 1.2.1 so testing outside of the repo is probably enough for now though! Until next Friday! :wave:
Continuing to delve into this. Collected notes in the wiki on GH. I think I have a better or at least different way to prepend the mailer_controller preview method for our purposes. We want to check if the email is going to be delivered by Notify and render a different preview when that is the case. The main blocker right now is not having a real Notify account to test with - we call the api to generate a preview, which is what we want to show in the Rails preview, but without a real API key I can't get a handle on that.
mec
force-pushed
the
wip-mec-improvements
branch
from
cef6319 to
230dfa1
Compare
|
I've taken a different direction for the improvements - this was useful though! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Looking at ways to improve the Gem and fix support for Rails > 7.0.8
Overriding the Rails::MailerController is a recipie for pain and is the
source of the issue we see in Rails > 7.0.8.
Right now I think all this does is allows a custom layout (the GOV.UK
one) - we can do that without the heavy handiness by suppling a layout
in the Mailer class - this commit contains that change.
To support the layout simply being in the views, we have to become a
'Engine' so we can add views by replicating the Rails directories (there
are other ways but this is nice and simple).
Becuase engines are just Railties, everything should work as it did,
again that change is here - needs better naming.
https://api.rubyonrails.org/classes/Rails/Engine.html
With all that done, the previews just work™ the mailer suppling the
layout overide for Mail::Notify::Mailer subclasses only.
I've only checked the 'view' version so far - but I can't see it being a
problem for the 'template' version.
With these changes all of the rails stuff in the specs is redundent, I
think - so everything becomes much simpler.
Right now I am testing this with a fully fledged Rails 7.1.2 install -
it would be nice to come up with a way to run 'tests' in all 3 major
versions of Rails - may Docker could help?
I would like to get this working and go for 1.2.1 so testing outside of
the repo is probably enough for now though!
Until next Friday! 👋