From ace2e8fe1e1d75083e47980870f4668152ca8182 Mon Sep 17 00:00:00 2001 From: Nishiki Liu Date: Mon, 10 Jun 2024 20:30:55 -0700 Subject: [PATCH] Add additional deletion tests and make them green --- app/models/bulk_deletion_request.rb | 2 +- .../deletion_request_flows_test.rb | 54 +++++++++++++++++++ test/test_helper.rb | 8 ++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/app/models/bulk_deletion_request.rb b/app/models/bulk_deletion_request.rb index 1e6cb26..10439a4 100644 --- a/app/models/bulk_deletion_request.rb +++ b/app/models/bulk_deletion_request.rb @@ -44,7 +44,7 @@ def deliver_emails # # @return [void] def generate_emails - Company.all.find_each.with_index do |company, index| + Company.all.group(:email).find_each.with_index do |company, index| deletion_request = DeletionRequest.new \ company: company, smtp_config: smtp_config, diff --git a/test/integration/deletion_request_flows_test.rb b/test/integration/deletion_request_flows_test.rb index fb13e89..454d88b 100644 --- a/test/integration/deletion_request_flows_test.rb +++ b/test/integration/deletion_request_flows_test.rb @@ -29,6 +29,39 @@ class DeletionRequestFlowsTest < ActionDispatch::IntegrationTest assert_emails 2 end + # It's possible for multiple companies to have the same contact email address + # whether it's because there are umbrella companies at play or something else. + # We should prevent duplicate emails so we're not requesting multiple times. + test "only one email per unique email address is sent" do + # Creating an extra California data broker company separate from fixtures. + Company.create! \ + category: "california_data_broker", + email: "test-ca-broker-2@localhost", + name: "Test CA Broker 2", + website: "https://test-ca-broker-2.localhost" + # Creating another California data broker company with the same email address. + Company.create! \ + category: "california_data_broker", + email: "test-ca-broker-2@localhost", # SAME + name: "Test CA Broker 3", + website: "https://test-ca-broker-3.localhost" + + post \ + bulk_deletion_requests_path, + params: { + email_subject: "Test deletion request subject", + email_body: "Test deletion request body", + smtp_provider: "gmail", + smtp_username: "test_username", + smtp_password: "test_password" + } + + # The first email should be delivered immediately, and there should only be one + # more queued email to ensure we deduplicated email addresses. + assert_emails 1 + assert_enqueued_emails 1 + end + test "sets alert flash for invalid requests" do post \ bulk_deletion_requests_path, @@ -42,4 +75,25 @@ class DeletionRequestFlowsTest < ActionDispatch::IntegrationTest assert_equal ["Smtp config is missing or invalid"], flash[:alert] end + + test "sets alert flash for SMTP authentication errors" do + bulk_deletion_request_mock = Mocktail.of_next BulkDeletionRequest + Mocktail + .stubs { bulk_deletion_request_mock.deliver_emails } + .with { raise Net::SMTPAuthenticationError.new("test") } + + post \ + bulk_deletion_requests_path, + params: { + email_subject: "Test deletion request subject", + email_body: "Test deletion request body", + smtp_provider: "gmail", + smtp_username: "test_username", + smtp_password: "test_password" + } + + assert_equal \ + "SMTP authentication failed. Please check your SMTP username and password.", + flash[:alert] + end end diff --git a/test/test_helper.rb b/test/test_helper.rb index f2a4bda..8eb6488 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -19,6 +19,12 @@ class TestCase # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. fixtures :all - # Add more helper methods to be used by all tests here... + # Reset test states after each test. + # + # @return [void] + def teardown + super + Mocktail.reset + end end end