Skip to content

Remove unused document capture notice text#5574

Merged
aduth merged 1 commit intomainfrom
aduth-rm-unused-doc-auth-notice
Nov 3, 2021
Merged

Remove unused document capture notice text#5574
aduth merged 1 commit intomainfrom
aduth-rm-unused-doc-auth-notice

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Nov 2, 2021

Why: Because they're never actually presented to the user, who will instead see the message derived from the failing response.

Also helps for a few points of general cleanup:

This logic is only used for the no-JavaScript document capture. In practice, what happens is that the same error message which would be presented in the React application is presented to no-JavaScript.

Example:

image

If you follow the flow of logic, you can see that the failure method does capture this as "extra", but that extra is not used for presenting a notice, and in-fact doesn't appear to be used for much of anything at all:

def failure(message, extra = nil)
flow_session[:error_message] = message
form_response_params = { success: false, errors: { message: message } }
form_response_params[:extra] = extra unless extra.nil?
FormResponse.new(**form_response_params)
end

register_update_step(step, result)
if flow.json
render json: flow.json, status: flow.http_status
return
end
flow_finish and return unless next_step
render_update(step, result)

The actual error message is pulled from the flow_session[:error_message]:

<%= flow_session[:error_message] %>

Originally: #3929

cc @anniehirshman-gsa in case you're aware of these messages being intended to be shown anywhere.

**Why**: Because they're never actually presented to the user, who will instead see the message derived from the failing response.
@aduth aduth requested a review from solipet November 2, 2021 20:53
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@solipet solipet left a comment

Choose a reason for hiding this comment

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

w00t!

Copy link
Contributor

@anniehirshman-gsa anniehirshman-gsa left a comment

Choose a reason for hiding this comment

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

Beautiful ✨

@aduth aduth merged commit 31951f0 into main Nov 3, 2021
@aduth aduth deleted the aduth-rm-unused-doc-auth-notice branch November 3, 2021 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants