Use CDO module for link helper #33064
Conversation
bencodeorg
changed the title
| @@ -1,6 +1,6 @@ | |||
| %p | |||
| The following person has requested to connect with their local Code.org Regional Partner | |||
| = link_to('through this form,', 'https://code.org/educate/professional-learning/contact-regional-partner') | |||
| = link_to('through this form,', CDO.code_org_url('educate/professional-learning/contact-regional-partner', CDO.default_scheme)) | |||
There was a problem hiding this comment.
The scheme should be optional; is it needed here?
There was a problem hiding this comment.
TBH I followed precedent from the other mailer -- I looked up what this default_scheme variable contains (http vs https), but couldn't find anything in Rails docs about how it is used in a link_to
There was a problem hiding this comment.
Oh, because I misinterpreted this as an argument to link_to, which it is not 🤦♂ No wonder I couldn't find any docs!
There was a problem hiding this comment.
Ah, sounds like a protocol relative URL is now out of fashion though it's quite useful for our own development environments.
But it sounds like there are particular pitfalls when they're used in emails, so that's probably a good reason to specify the protocol explicitly here.
Fixes a link that caused this Honeybadger error (albeit from staging) -- we think the link worked locally because we load both pegasus and dashboard into a single Ruby instance, whereas we do not do this in other environments. As a result, I was originally using this global pegasus method , instead of the one in the CDO module.
Also improves a hard coded link to use the same approach per suggestion from @breville.
Testing story
Tested manually on my machine.
Reviewer Checklist: