Delete user if email isn't confirmable

[mftaff]

fixes #130

Note: I am unable to test this solution because of the inability to setup locally using vagrant (see #353). However, it is a starting point, and I am happy to make any changes required :-)

[sethherr]

Hey @mftaff - bummer the vagrant setup isn't working. We definitely need to have tests for this though. Once you push up a change to this pull request, it will run on Travis - so even if you can't run the tests locally, you can add a new test to the spec file and it will be tested on our CI - warning: this isn't the fastest way to do development, it's much better to get things running locally

The place you need to add a spec is in spec/workers/email_confirmation_worker_spec.rb.

The spec can stub raising the exception, it should test that when the exception is raised it deletes the user.

[mftaff]

👍 Cool cool. I just got rspec running locally. I am writing a test now.

[mftaff]

I am having a bit of trouble with creating the test. It seems that when in test mode, sidekiq isn't actually sending emails: [rspec-sidekiq] WARNING! Sidekiq will *NOT* process jobs in this environment. See https://github.com/philostler/rspec-sidekiq/wiki/FAQ-&-Troubleshooting

I may have misunderstood what that link said, but it seems that when testing, the mailers are just being created, but not actually sent out.

Do you know how to get around this? Some way to actually try and deliver (or test the emails validity) the email and this way we should be able to catch any SMTPSyntaxErrors.

Here is what I have so far:

describe EmailConfirmationWorker do
  ...
  it 'deletes user if email is invalid' do
    user = FactoryGirl.create(:user)
    user.update(email: 'notaemail@fakeonotreal.blorgh')
    EmailConfirmationWorker.new.perform(user.id)
    # assuming the EmailConfirmationWorker raised an SMTPSyntaxError the user should be deleted:
    expect(User.count).to be_zero 
  end
end

[mftaff]

As an aside I just came up with expect { user.reload }.to raise_error ActiveRecord::RecordNotFound as the way to check if the user was deleted, as apposed to checking User.count which is less specific.

[sethherr]

Sorry for the slow response @mftaff ! I think checking that the error is raised is good. I think you could actually stub the delivery of the message and the raising of the error, since you're correct, the mailers don't actually deliver in testing.

[mftaff]

No worries!

I have been trying to do exactly that - however, so far I haven't been successful. I haven't written tests for mailers before, so this is a bit new for me. Do you have an idea what this test needs to look like?