Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bulk unsubscribe Candidates in Support #10228

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

dcyoung-dev
Copy link
Collaborator

@dcyoung-dev dcyoung-dev commented Jan 9, 2025

Context

When users unsubscribe from bespoke Notify emails, their email address is exportable from the Notify service and does not automatically unsubscribe them from emails in Apply.
Manual intervention is required to export these email addresses from Notify and update the Candidate in Apply, this PR aims to provide an interface in the Support Console to enable us to update Candidates unsubscribe preferences.

Changes proposed in this pull request

New UI

Screen.Recording.2025-01-09.at.15.16.15.mov
  • Added a Bulk Unsubscribe Form
  • Added a Bulk Unsubscribe Worker
  • Added a Bulk Unsubscribe Service
  • Added form to Support Interface to upload email addresses

Guidance to review

  • Testing locally with several email addresses via the Support Interface
  • Emails submitted should then be marked as unsubscribed from emails

Things to check

  • If the code removes any existing feature flags, a data migration has also been added to delete the entry from the database
  • This code does not rely on migrations in the same Pull Request
  • If this code includes a migration adding or changing columns, it also backfills existing records for consistency
  • If this code adds a column to the DB, decide whether it needs to be in analytics yml file or analytics blocklist, if included inform data insights team of the changes
  • If this code adds a column that may include PII, the sanitise.sql script and 0025-protecting-personal-data-in-production-dump.md ADR have been updated.
  • API release notes have been updated if necessary
  • If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • Attach the PR to the Trello card

@dcyoung-dev dcyoung-dev requested a review from a team January 9, 2025 13:05
@dcyoung-dev dcyoung-dev self-assigned this Jan 9, 2025
@dcyoung-dev dcyoung-dev force-pushed the import-unsubscribed-email-addresses branch from e529ad8 to 250b962 Compare January 9, 2025 13:07
@dcyoung-dev dcyoung-dev force-pushed the import-unsubscribed-email-addresses branch from 250b962 to c2c147f Compare January 9, 2025 14:32
@dcyoung-dev dcyoung-dev force-pushed the import-unsubscribed-email-addresses branch from f16dee0 to 26ee4ba Compare January 9, 2025 15:18
@dcyoung-dev dcyoung-dev marked this pull request as ready for review January 9, 2025 15:20
@dcyoung-dev
Copy link
Collaborator Author

Trello: https://trello.com/c/R8vZn7iE

@dcyoung-dev dcyoung-dev added the deploy_v2 Deploy the review app to AKS label Jan 9, 2025
Added a Bulk Unsubscribe Form
Added a Bulk Unsubscribe Worker
Added a Bulk Unsubscribe Service

Unsubscribing is performed by `update_all` in batches to ensue performance
@dcyoung-dev dcyoung-dev force-pushed the import-unsubscribed-email-addresses branch from 26ee4ba to de11e6c Compare January 9, 2025 15:29
@github-actions github-actions bot temporarily deployed to review_aks-10228 January 9, 2025 15:34 Destroyed
@github-actions github-actions bot temporarily deployed to review_aks-10228 January 9, 2025 15:51 Destroyed
.split("\n")
.map(&:strip)
.compact_blank
Support::Candidates::BulkUnsubscribeWorker.perform_async(unsubscribe_email_addresses)
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably we are doing this async because we are anticipating putting A LOT of email addresses in the box? Just wondering.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah future proofing to be honest - currently we have about 14 to do. My thoughts were that this sets us up to handle bulk through an API - that'd be ideal

Copy link
Contributor

@elceebee elceebee left a comment

Choose a reason for hiding this comment

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

Nice one!

Renames `.erb-lint.yml` to `.erb_lint.yml` as suggested in [lint failures](https://github.com/DFE-Digital/apply-for-teacher-training/actions/runs/12693098266/job/35381314500?pr=10228)
>The config file has been renamed to `.erb_lint.yml` and `.erb-lint.yml` is deprecated. Please rename your config file to `.erb_lint.yml`.
@github-actions github-actions bot temporarily deployed to review_aks-10228 January 9, 2025 16:23 Destroyed
@github-actions github-actions bot temporarily deployed to review_aks-10228 January 9, 2025 16:39 Destroyed
require 'rails_helper'

RSpec.describe SupportInterface::Candidates::BulkUnsubscribe do
describe '.bulk_unsubscribe' do
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add a test that shows if an email address is included that doesn't actually belong to a candidate, it doesn't break the job?

I'm thinking of the scenario where a candidate changes their email address between clicking the unsubscribe button via notify and someone running this job.

expect(candidate2.reload).to be_unsubscribed_from_emails
end

it 'can handle unknown email addresses' do
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@github-actions github-actions bot temporarily deployed to review_aks-10228 January 10, 2025 09:26 Destroyed
@dcyoung-dev dcyoung-dev merged commit 2dfeb64 into main Jan 10, 2025
23 checks passed
@dcyoung-dev dcyoung-dev deleted the import-unsubscribed-email-addresses branch January 10, 2025 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy_v2 Deploy the review app to AKS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants