Point email footer links to correct sites#1247
Conversation
|
Re: |
zachmargolis
left a comment
zachmargolis
left a comment
There was a problem hiding this comment.
LGTM thanks for updating these
There was a problem hiding this comment.
No need to quote this IIRC:
a href=MarketingSite.base_url
app/services/marketing_site.rb
Outdated
There was a problem hiding this comment.
Let's .to_s this for consistency?
There was a problem hiding this comment.
anything in a view will have to_s called on it by default (HELLO RAILS ✨ ).
I think you know this and so you are suggesting that we call to_s here so we know in our tests we are testing the correct behavior? Want to make sure I understand
There was a problem hiding this comment.
thoughts on using a link_to helper instead?
There was a problem hiding this comment.
That's what I went for at first but couldn't get it to work for some reason. The email wouldn't even get sent! If you know how, please let me know. I'll try it again too.
app/services/marketing_site.rb
Outdated
There was a problem hiding this comment.
anything in a view will have to_s called on it by default (HELLO RAILS ✨ ).
I think you know this and so you are suggesting that we call to_s here so we know in our tests we are testing the correct behavior? Want to make sure I understand
monfresh
force-pushed
the
mb-fix-email-footer-links
branch
2 times, most recently
from
d6bd4f8 to
a4de40a
Compare
|
@jessieay The |
|
@jessieay re: |
**Why**: Up until now, we had placeholder links in the email footer because we didn't have sites to point them to. Now that we do, we need to update the links. Note that the https://login.gov/policy link doesn't seem to be live yet. I'm not sure if that's the wrong URL or if the page hasn't been published yet.
monfresh
force-pushed
the
mb-fix-email-footer-links
branch
from
a4de40a to
4f61552
Compare
**Why**: Up until now, we had placeholder links in the email footer because we didn't have sites to point them to. Now that we do, we need to update the links. Note that the https://login.gov/policy link doesn't seem to be live yet. I'm not sure if that's the wrong URL or if the page hasn't been published yet.
3 participants
Why: Up until now, we had placeholder links in the email footer
because we didn't have sites to point them to. Now that we do, we need
to update the links.
Note that the https://login.gov/policy link doesn't seem to be live yet.
I'm not sure if that's the wrong URL or if the page hasn't been
published yet.