Skip to content
This repository has been archived by the owner. It is now read-only.

/ fxa-content-server Archived

fix(coppa): move to age input coppa #3138

Merged
merged 1 commit into from Oct 20, 2015
Merged

fix(coppa): move to age input coppa #3138

merged 1 commit into from Oct 20, 2015

Conversation

@vladikoff
Copy link
Contributor

@vladikoff vladikoff commented Oct 5, 2015

Fixes #3137
@vladikoff vladikoff added this to the FxA-0: quality milestone Oct 5, 2015
@vladikoff vladikoff force-pushed the vladikoff:remove-coppa branch from 383870c to 42d6958 Oct 5, 2015
@vladikoff vladikoff force-pushed the vladikoff:remove-coppa branch 3 times, most recently from aa14293 to 62fc051 Oct 15, 2015
@vladikoff vladikoff removed the WIP label Oct 19, 2015
app/scripts/lib/auth-errors.js
@@ -151,10 +151,6 @@ function (_, Errors) {
errno: 1011,
message: t('Valid email required')
},
YEAR_OF_BIRTH_REQUIRED: {

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

We spoke at one point about how we were going to handle deprecated client-side errors. The resolution was to comment out the errors and leave them in the code. This is usually considered bad practice with version control, but is reasonable from a documentation perspective and ensuring somebody doesn't come along and try to fill in one of the newly empty numbers.

This comment has been minimized.

Sign in to view
@vladikoff

vladikoff Oct 19, 2015
Author Contributor

sounds good!
@vladikoff
fix(coppa): move to age input coppa
2341e83 
Fixes #3137
@vladikoff vladikoff force-pushed the vladikoff:remove-coppa branch from 62fc051 to 2341e83 Oct 19, 2015
tests/functional/firefox/same_account.js
var email;

registerSuite({
name: 'about:accounts signup',

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

This suite is no longer needed?

This comment has been minimized.

Sign in to view
@vladikoff

vladikoff Oct 19, 2015
Author Contributor

Yeah we don't use it
tests/functional/lib/helpers.js
@@ -438,11 +438,12 @@ define([
.type(password)
.end()

.findByCssSelector('#fxa-age-year')
.findByCssSelector('#age')
.type('4')

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

nit: indentation.
tests/functional/oauth_preverified_sign_up.js
.findById('fxa-' + (TOO_YOUNG_YEAR - 1))
.click()
.findByCssSelector('#age')
// XXX: Bug in Selenium 2.47.1, if Firefox is out of focus it will just type 1 number,

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

ugh.

nit: indentation.
tests/functional/oauth_sign_up_verification_redirect.js
@@ -39,7 +36,7 @@ define([
return self.remote.get(require.toUrl(url + '&verification_redirect=always'));
})
.then(function () {
return FunctionalHelpers.fillOutSignUp(self, email, PASSWORD, OLD_ENOUGH_YEAR);
return FunctionalHelpers.fillOutSignUp(self, email, PASSWORD);

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

I haven't mentioned, but I am really glad you simplified the function signature.
@ryanfeeley
Copy link
Contributor

@ryanfeeley ryanfeeley commented Oct 19, 2015

@vladikoff I need to UI review. What's the flag to get this to appear?
tests/functional/sign_up.js
.findByCssSelector('#age')
// XXX: Bug in Selenium 2.47.1, if Firefox is out of focus it will just type 1 number,
// split the type commands for each character to avoid issues with the test runner
.type('1')

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

nit: indentation.

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

When you do the .type, do you have to do a .click first so the element receives the focus? I could swear that's why we do stuff like https://github.com/mozilla/fxa-content-server/pull/3138/files#diff-addb444c51a96421c45bfd482b36edd8R224

This comment has been minimized.

Sign in to view
@vladikoff

vladikoff Oct 19, 2015
Author Contributor

Tried that trick, Sadly that does not help
tests/functional/sign_up.js
var monthToSelect = now.getMonth();
var dateToSelect = now.getDate();

'coppa does not allow sign up if younger than 13 years old': function () {
return this.remote
.get(require.toUrl(PAGE_URL))
.findByCssSelector('form input.email')

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

Would FunctionalHelpers.fillOutSignUp with an optional age work here?
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Oct 19, 2015

@ryanfeeley https://latest.dev.lcip.org/signup?forceExperiment=coppaView&forceExperimentGroup=treatment
tests/functional/sign_up.js
@@ -416,7 +341,6 @@ define([

function testRepopulateFields(dest, header) {
var self = this;
var year = OLD_ENOUGH_YEAR;

return self.remote

This comment has been minimized.

Sign in to view
@shane-tomlinson

shane-tomlinson Oct 19, 2015
Member

Could fillOutSignUp with an optional age be used here as well?
@vladikoff
Copy link
Contributor Author

@vladikoff vladikoff commented Oct 19, 2015

image
@ryanfeeley
Copy link
Contributor

@ryanfeeley ryanfeeley commented Oct 19, 2015

👍
@shane-tomlinson shane-tomlinson self-assigned this Oct 20, 2015
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Oct 20, 2015

Testing now.
@shane-tomlinson shane-tomlinson merged commit 2341e83 into mozilla:master Oct 20, 2015
3 checks passed
3 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 99.039%
Details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@shane-tomlinson shane-tomlinson

Labels
None yet
Projects
None yet
Milestone
FxA-0: quality
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@vladikoff @ryanfeeley @shane-tomlinson