Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update message when user is verified #1798

Merged
merged 1 commit into from
Nov 22, 2017
Merged

Conversation

nickbristow
Copy link
Contributor

@nickbristow nickbristow commented Nov 21, 2017

Why:
New copy for users when they are verified
at loa3.

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.

  • 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.

@nickbristow
Copy link
Contributor Author

login_gov_-_you_have_verified_your_identity_with_login_gov

login_gov_-_you_have_created_your_account_with_login_gov

@nickbristow nickbristow force-pushed the update_verified_message branch 2 times, most recently from b67f3d8 to d3cb7a1 Compare November 22, 2017 14:46
@@ -91,4 +87,20 @@ def requested_attributes
def requested_loa
loa3_requested ? 'loa3' : 'loa1'
end

def loa3?
Copy link
Member

Choose a reason for hiding this comment

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

I can't seem to find where this function is used

accent: content_tag(:strong, I18n.t("titles.sign_up.#{requested_loa}")),
app: APP_NAME
).html_safe])
@current_user.decorate.identity_verified? ? verified_heading : loa_heading
Copy link
Member

Choose a reason for hiding this comment

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

I've done a little digging, and I can't figure out how an LOA3 user reaches the sign up completions view without a verified identity. Can you help me understand why this conditional needs to check for a verified identity here?

Copy link
Contributor Author

@nickbristow nickbristow Nov 22, 2017

Choose a reason for hiding this comment

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

@jmhooper if the user goes directly to login.gov, and then verifies, by say entering the code from a letter. They don't have loa3_request = true in their session. So they see the original message "you have created an account", when we want to show that they have just completed their verification. "You have verified your identity"

what im not sure about is if there is ever a situation where we want to show them the old loa3 message. So it may be worth taking that out

@nickbristow nickbristow force-pushed the update_verified_message branch 3 times, most recently from 0d3b733 to 66dbb01 Compare November 22, 2017 20:44
**Why**:
New copy for users when they are verified
at loa3.
@nickbristow nickbristow merged commit 1fdf066 into master Nov 22, 2017
@nickbristow nickbristow deleted the update_verified_message branch November 22, 2017 21:25
@mkhandekar
Copy link

mkhandekar commented Nov 22, 2017

@nickbristow a small copy change on the 2nd screen there. It should be "You can now continue to AgencyName." instead of "You can now continue to and into AgencyName."

Other than that LGTM 🚢 !

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.

3 participants