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

Improve error messaging and track user errors on forms to Google Analytics #426

Merged
merged 3 commits into from Oct 29, 2015

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Oct 26, 2015

Collect anonymous metrics on the different types of user error when they’re changing their email or passphrase, or when setting up 2-step verification.

  • Add a helper for tracking messages in analytics
  • Start tracking the error messages in analytics

This also includes some improvements to the passphrase change forms:

Before

pasted_image_at_2015_10_08_04_03_pm_720

After

screen shot 2015-10-23 at 13 20 58

cc @boffbowsh @jamiecobbett

Collect metrics on the different types of user error when changing
their email or passphrase, or when setting up 2-step verification.
@boffbowsh
Copy link
Contributor

@boffbowsh boffbowsh commented Oct 26, 2015

You've got red on you[r commit].

@@ -64,3 +64,7 @@
</div>
</div>
</div>

<div>
<%= f.submit t("users.edit.change"), :class => 'btn btn-primary add-top-margin' %>

This comment has been minimized.

@benlovell

benlovell Oct 28, 2015
Contributor

Could we use the 1.9 style hash keys here, per the style guide?

This comment has been minimized.

@fofr

fofr Oct 28, 2015
Author Contributor

Good point, updated and rebased.

@benlovell
Copy link
Contributor

@benlovell benlovell commented Oct 28, 2015

Looks good otherwise. I'm looking into the build failures as they're happening against SSO on other branches/master too. 😞

Avoid using the default devise error method which includes a poor
heading and old markup.

* Consolidate errors in partial
* Move submit button into partial
* Start tracking the error messages in analytics.
@fofr fofr force-pushed the track-more-errors branch from 4b74127 to 25cf560 Oct 28, 2015
@tijmenb
Copy link
Contributor

@tijmenb tijmenb commented Oct 28, 2015

This is super nice. Minor suggestion, maybe using the content_tag helper could prevent you from having to do the string concatenation / html_safe things.

def track_analytics_data(type, message)
  {
    'module' => 'auto-track-event',
    'track-action' => "alert-#{type}",
    'track-label' => flash_text_without_email_addresses(message)
  }
end
<%= content_tag :li, data: track_analytics_data(:danger, message) %>
  <%= message %>
<% end %>

That could apply to https://github.com/alphagov/signonotron2/blob/track-more-errors/app/views/shared/_bootstrap_flash_messages.html.erb too.

@fofr
Copy link
Contributor Author

@fofr fofr commented Oct 29, 2015

@tijmenb Ooh, that's much nicer.

@benlovell
Copy link
Contributor

@benlovell benlovell commented Oct 29, 2015

Also, you can avoid yielding to the block in content_tag and simplify this a teeny bit by passing message as the second argument.

Use `content_tag` and pass in a group of data attributes to avoid
string concatenation and `html_safe` and to simplify use in templates.

This also simplifies the flash helper.
@fofr fofr force-pushed the track-more-errors branch from 68dbe57 to ae612d4 Oct 29, 2015
@fofr
Copy link
Contributor Author

@fofr fofr commented Oct 29, 2015

@tijmenb @benlovell Implemented that suggestion in ae612d4, looking much nicer.

@benlovell

This comment has been minimized.

Copy link
Contributor

@benlovell benlovell commented on ae612d4 Oct 29, 2015

😍

benlovell added a commit that referenced this pull request Oct 29, 2015
Improve error messaging and track user errors on forms to Google Analytics
@benlovell benlovell merged commit e06a627 into master Oct 29, 2015
2 checks passed
2 checks passed
@govuk-ci
Test signon changes against gds-sso master "Testing gds-sso against changes #257 succeeded on Jenkins"
Details
@govuk-ci
default "Build #858 succeeded on Jenkins"
Details
@benlovell benlovell deleted the track-more-errors branch Oct 29, 2015
@tijmenb
Copy link
Contributor

@tijmenb tijmenb commented Oct 29, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants