Use shared examples to test cancelation behavior

[jmhooper]

Why: So that we can test that cancelation is consistent across IdV
steps.

Note that this tests the behavior with the different protocols because
the difference between cancellation behavior with and without an SP is
surprisingly different.

Also, this commit adds some empty contexts for steps that don't have a
cancel button with the assumption that we want to add a cancel button to
those steps.

Hi! Before submitting your PR for review, and/or before merging it, please
go through the following checklist:

• For DB changes, check for missing indexes, check to see if the changes
  affect other apps (such as the dashboard), make sure the DB columns in the
  various environments are properly populated, coordinate with devops, plan
  migrations in separate steps.

• For route changes, make sure GET requests don't change state or result in
  destructive behavior. GET requests should only result in information being
  read, not written.

• For encryption changes, make sure it is compatible with data that was
  encrypted with the old code.

• For secrets changes, make sure to update the S3 secrets bucket with the
  new configs in all environments.

• Do not disable Rubocop or Reek offenses unless you are absolutely sure
  they are false positives. If you're not sure how to fix the offense, please
  ask a teammate.

• When reading data, write tests for nil values, empty strings,
  and invalid formats.

• When calling redirect_to in a controller, use _url, not _path.

• When adding user data to the session, use the user_session helper
  instead of the session helper so the data does not persist beyond the user's
  session.

• When adding a new controller that requires the user to be fully
  authenticated, make sure to add before_action :confirm_two_factor_authenticated.

[davemcorwin]

Using alias seems like a particularly tricky way of handling this, what about any of the below?

• passing in a proc or lambda to call
• passing in a symbol and calling send on IdvStepHelper
• implementing something like IdvStepHelper.navigate(:complete_idv_steps_before_address_step)

[jmhooper]

Hmm, 🤔, I went with alias because I thought it was less tricky. I think we could set a symbol in a let block though and call send on IdvStepHelper.

[davemcorwin]

why can't you just pass it as an additional argument to `it_behaves_like`, like `:oidc`, or `:saml`?

…

On Thu, Apr 12, 2018 at 2:52 PM, Jonathan Hooper ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In spec/features/idv/cancel_idv_step_spec.rb <#2094 (comment)>: > +feature 'cancel at IdV step', :idv_job do + include IdvStepHelper + + context 'verify step' do + def complete_previous_idv_steps + sign_in_and_2fa_user(user_with_2fa) + visit verify_path unless current_path == verify_path + end + + it_behaves_like 'cancel at idv step' + it_behaves_like 'cancel at idv step', :oidc + it_behaves_like 'cancel at idv step', :saml + end + + context 'profile step' do + alias complete_previous_idv_steps start_idv_at_profile_step Hmm, 🤔, I went with alias because I thought it was less tricky. I think we could set a symbol in a let block though and call send on IdvStepHelper . — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#2094 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGWo47txG_voilRxWKzBHVRpTaZFiwvnks5tn8ymgaJpZM4TSOyQ> .

-- David Corwin 18F <https://18f.gsa.gov/> | Consulting Engineer [email protected]

[jmhooper]

That.... is a very good idea

[jmhooper]

@davemcorwin: I just pushed a commit where we pass the step name as a symbol to the shared examples. PTAL and lmk what you think

[davemcorwin]

nice, i like this, its a bit more explicit about whats happening. This is probably more of a design issue, but do we always want to boot the user all the way out of the IdV flow not just go back?

[davemcorwin]

LGTM

[jmhooper]

This is probably more of a design issue, but do we always want to boot the user all the way out of the IdV flow not just go back?

Probably not, and revisiting these cancellation flows is on of the things we want to do for LOA3. Here I'm just trying to cover existing behavior so we have a framework in place when we do re-visit cancellation.