Update newsletter subscription #16785
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16785 +/- ##
==========================================
- Coverage 79.68% 79.68% -0.01%
==========================================
Files 159 159
Lines 8522 8525 +3
==========================================
+ Hits 6791 6793 +2
- Misses 1731 1732 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
maureenlholland
force-pushed
the
update-newsletter-subscription
branch
from
b20b18f to
bee77ff
Compare
| } | ||
|
|
||
| if (response) { | ||
| if (response.status !== undefined) { |
There was a problem hiding this comment.
change for response structure
| 'application/x-www-form-urlencoded' | ||
| ); | ||
| xhr.setRequestHeader('X-Requested-With', 'XMLHttpRequest'); | ||
| xhr.timeout = 10000; // increased AWS timeout |
There was a problem hiding this comment.
change for slower AWS requests (this might be due to local testing? hopefully prod will be faster)
There was a problem hiding this comment.
According to Foundation: "the calls when the lambda is warm should be way quicker (~1s)"
Will still add a loader as suggested
UPDATE: we have a "spinner_color" param on newsletter macro but I don't think it does anything anymore. Spinner was removed in https://github.com/mozilla/bedrock/pull/9178/files#diff-89b12c56d1788e07ed2d017d8cef8ece3284d529932739e52c2a25839917d557L107
We may get a new production URL before launch, but this is the only one available for now
stephaniehobson
added
Needs Review
| error = form.querySelector('.error-terms'); | ||
| break; | ||
| default: | ||
| error = form.querySelector('.error-try-again-later'); |
There was a problem hiding this comment.
add Sentry call here? We don't have same visibility into AWS request responses as we did to Basket
There was a problem hiding this comment.
Could you please also fork and update the newsletter unit tests? https://github.com/mozilla/protocol/blob/main/tests/unit/newsletter.js
Co-authored-by: Stephanie Hobson <[email protected]>
This should be cleaned up and backported to main Protocol component - Adds loading SVG (based on existing loading icon) - Toggles display of loader to match form field disabled logic
| # Foundation newsletters are handled through Campaign Monitor | ||
| FOUNDATION_URL = config( | ||
| "FOUNDATION_URL", | ||
| default="https://kmq73rfvbh.execute-api.us-east-2.amazonaws.com" if DEV else "https://abdri3ttkb.execute-api.us-east-2.amazonaws.com", |
There was a problem hiding this comment.
wasn't sure how to test Prod Mode + switch (the switch is OFF in prod)
I ended up removing the switch conditionals in footer-newsletter and helpers and running Prod Mode to confirm the correct URL was applied
There was a problem hiding this comment.
You can log to django-admin locally and set the switch if you config a CMS super user like step 4 here: https://mozmeao.github.io/platform-docs/cms/?h=wagtail_admin_email#non-sso-authentication (maybe with other accounts too but that's what I have done).
| # Foundation newsletters are handled through Campaign Monitor | ||
| FOUNDATION_URL = config( | ||
| "FOUNDATION_URL", | ||
| default="https://kmq73rfvbh.execute-api.us-east-2.amazonaws.com" if DEV else "https://abdri3ttkb.execute-api.us-east-2.amazonaws.com", |
There was a problem hiding this comment.
You can log to django-admin locally and set the switch if you config a CMS super user like step 4 here: https://mozmeao.github.io/platform-docs/cms/?h=wagtail_admin_email#non-sso-authentication (maybe with other accounts too but that's what I have done).
maureenlholland
force-pushed
the
update-newsletter-subscription
branch
from
ef212f5 to
0bb61b6
Compare
3 participants
If this changeset needs to go into the FXC codebase, please add the
WMO and FXClabel.One-line summary
Update multi-newsletter email forms to use Foundation only. Update Foundation newsletter action URL to separate system.
for more context, see https://docs.google.com/document/d/10bScfykHIXp8XLWV87nuME9GfWNDS02bQnxM6PiBEco/ [Moz only]
Significant changes and points to review
Developer on Foundation side was kind enough to update the accept request content type to
application/x-www-form-urlencodedThis works with no JS
Protocol JS was copied and tweaked to suit AWS response structure and allow a longer timeout
Once things are confirmed from Foundation side, there should be a follow-up task to remove switch and any non-Foundation email forms
Related: newsletter pref center copy change #16764
Issue / Bugzilla link
https://mozilla-hub.atlassian.net/browse/WT-241
Testing