Skip to content

fix(ia): merge email settings#3846

Merged
chickenn00dle merged 4 commits into
epic/iafrom
fix/merge-email-settings
Mar 20, 2025
Merged

fix(ia): merge email settings#3846
chickenn00dle merged 4 commits into
epic/iafrom
fix/merge-email-settings

Conversation

@chickenn00dle

@chickenn00dle chickenn00dle commented Mar 19, 2025

Copy link
Copy Markdown
Contributor

All Submissions:

Changes proposed in this Pull Request:

Closes https://app.asana.com/0/1205919985867982/1209710969266199

This PR merges the email settings from Audience > Transactional Emails and Newspack > Settings > Emails into the settings page for the latter.

Screenshot 2025-03-19 at 14 31 28

How to test the changes in this Pull Request:

  1. As admin, navigate to Newspack > Audience
  2. Verify there is no longer a Transactional Emails tab present
  3. Navigate to Newspack > Settings > Emails
  4. Verify all newspack email templates are present here, including all of the templates shown in the screenshot.
  5. Smoke test the various templates, including:
  • Editing
  • Resetting
  • Toggling

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@chickenn00dle chickenn00dle marked this pull request as ready for review March 19, 2025 18:46
@chickenn00dle chickenn00dle requested a review from a team as a code owner March 19, 2025 18:46

@miguelpeixe miguelpeixe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests well, but the Audience emails are rendering when I've not gone through the audience setup. They should probably not be rendered in that case, as they will not be used and it can be misleading.

@chickenn00dle

Copy link
Copy Markdown
Contributor Author

Tests well, but the Audience emails are rendering when I've not gone through the audience setup. They should probably not be rendered in that case, as they will not be used and it can be misleading.

Good catch @miguelpeixe!

Fixed in 9ffd08b

@chickenn00dle chickenn00dle added [Status] Needs Review The issue or pull request needs to be reviewed [Status] Needs Design Review labels Mar 19, 2025

@miguelpeixe miguelpeixe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for revising! Tests well.

@github-actions github-actions Bot added [Status] Approved The pull request has been reviewed and is ready to merge and removed [Status] Needs Review The issue or pull request needs to be reviewed labels Mar 19, 2025
@thomasguillot

Copy link
Copy Markdown
Contributor

✅ Design

@chickenn00dle chickenn00dle merged commit 9ae9adc into epic/ia Mar 20, 2025
@chickenn00dle chickenn00dle deleted the fix/merge-email-settings branch March 20, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Information Architecture [Status] Approved The pull request has been reviewed and is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants