LG-6845: Fix "Try again" button and SSN "< Back" link/button for IPP Flow

[NavaTim]

The "Try Again" button on the exception/warning pages will now check the referrer for an IPP URL and provide an IPP index page redirect if one is found. The IPP "index" page always redirects to the current step of the IPP flow.

The "Back" link/button used by the IPP flow had the parent flow hardcoded, so I refactored it to accept the flow name as a symbol. The new CancelUpdateSsnAction was also required because the IPP flow stores session state in pii_from_user instead of pii_from_doc.

[aduth]

Thanks for adding this! I think it's good to include the method even if there's no logic associated with that action, for clarity / completeness.

Style nit: For body-less methods like this, the convention I've seen is...

Suggested change

	def exception
	end
	def exception; end

[NavaTim]

Updated body-less method to meet convention.

[aduth]

In other places we've used the @step_url instance variable assigned by FLowStateMachine. Can we do that here as well, to make sure we're consistently using the flow's step_url configuration?

identity-idp/app/views/idv/shared/_document_capture.html.erb

Line 26 in 1986d42

send(@step_url, step: :verify_document_status) :

identity-idp/app/services/flow/flow_state_machine.rb

Line 76 in 1986d42

@step_url = klass::FSM_SETTINGS[:step_url]

[NavaTim]

The remote proofing address page didn't like using @step_url, so I made it support local_assigns[:step_url] as well.

[aduth]

I don't have a lot of confidence in the referer attribute, since it's easy for it to get lost (e.g. does a reload retain referer? Not really sure tbh). I wonder if we could include a parameter in the URL when redirecting to these error pages in DocAuthBaseStep#idv_failure, then use that when determining the try_again_path.

Or, alternatively, we may be able to infer that the user is in the in-person flow if we user_session['idv/in_person'] is not empty.

[NavaTim]

This seemed like it would work in most practical use cases & kept the referrer when I refreshed. I understand the concern, though, and refactored it accordingly.

[aduth]

Couple comments to consider, but overall LGTM 👍

[aduth]

Where do we use this flow value?

[NavaTim]

It was used to determine the current flow, but is no longer needed. Removed.

[aduth]

I was sorta hoping this could be a simple token like "doc_auth" vs. "in_person", though I guess that's not readily available without a bit of extra effort. I wonder if we'd have any issues like described in this StackOverflow comment using original_url ?

Alternatively, there are some ways we could get a token from e.g. self.class.name or @flow.class.name or @flow.controller.class.name, but if we did that, we might want to clean it up so that it's not a namespaced Ruby class name.

[aduth]

Another thought could be to implement an abstract method or variable in VerifyBaseStep, implement that for each flow, and pass that through to or reference from idv_failure.

module Idv
  module Steps
    class DocAuthBaseStep < Flow::BaseStep
      def idv_failure
        # ...
        redirect_to exception_url
      end

      def exception_url
        idv_session_errors_exception_url
      end
    end
  end
end

module Idv
  module Steps
    module Ipp
      class VerifyStep < VerifyBaseStep
        def exception_url
          idv_session_errors_exception_url(flow: :in_person)
        end
      end
    end
  end
end

module Idv
  module Steps
    class VerifyStep < VerifyBaseStep
      def exception_url
        idv_session_errors_exception_url(flow: :doc_auth)
      end
    end
  end
end

[NavaTim]

I think this'd be easier to determine if there was an intermediate step in the class hierarchy for IPP, but seeing as there are shared steps I decided to reduce this to the path instead of the full URL to reduce the probability of the issue mentioned on SO.

[aduth]

I think idv_failure living in DocAuthBaseStep is a bit misleading to how much it's actually shared. This method is only ever called as part of the "Verify" step. It should probably be moved into VerifyBaseStep.

I'm bumping into it as part of LG-6927, so I'll see about moving the method as part of that work.