new users have the "engagement" value set to true (Seemingly), even if they didn't check it when they signed up

RESOLVED FIXED

Status

Webmaker
Login
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: pomax, Assigned: sedge)

Tracking

Details

(Whiteboard: QA)

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
If I create a new user, and sign into webmaker, then fill in my desired username, click "I agree to ToS", but NOT "you can email me", and I then check what the values are for this used in the login admin console, the "egagements" checkbox is checked, indicating I did in fact pick that option when I didn't.
(Reporter)

Comment 1

4 years ago
s/used/user
(Assignee)

Comment 2

4 years ago
The user model sets those fields to "true" by default, so if the boxes are unchecked they must explicitly pass "false" for both fields.
(Reporter)

Comment 3

4 years ago
if the boxes are unchecked, the browser will not even send them as part of the query. Only when true will they show up as query arguments, so those defaults should actually be "false".
(Reporter)

Updated

4 years ago
Whiteboard: QA
(Assignee)

Comment 4

4 years ago
I agree! The question becomes, which change is more reasonable atm? I suspect it will be easier to update client-side user creation logic to pass "false" than it will be to change the user schema and deal with the fallout of that.
Whiteboard: QA
(Assignee)

Updated

4 years ago
Whiteboard: QA
(Reporter)

Comment 5

4 years ago
doesn't a default value for a column take effect on new records only? It shouldn't try to rewrite all the existing data, so updating the schema to say "the default is now 'false'" should be pretty safe.
(Reporter)

Updated

4 years ago
Assignee: nobody → kieran.sedgwick

Comment 6

4 years ago
The default should be false on the user column, I can deal with the existing data easily enough (in another bug)
(Assignee)

Comment 7

4 years ago
Created attachment 765402 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/116

While running locally it didn't affect existing documents in the database, and new users had the new defaults.
Attachment #765402 - Flags: review?(jon)

Comment 8

4 years ago
Comment on attachment 765402 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/116

r-, you have failing tests to fix up:

https://travis-ci.org/mozilla/login.webmaker.org/jobs/8279874#L1009
Attachment #765402 - Flags: review?(jon) → review-
(Assignee)

Comment 9

4 years ago
Comment on attachment 765402 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/116

Argh.. that was just lazy testing on my part.  Sorry for wasting your time!

Haha 2 for the money!
Attachment #765402 - Flags: review- → review?(jon)
Comment on attachment 765402 [details] [review]
https://github.com/mozilla/login.webmaker.org/pull/116

Three to get ready!
Attachment #765402 - Flags: review?(jon) → review+

Comment 11

4 years ago
Commits pushed to master at https://github.com/mozilla/login.webmaker.org

https://github.com/mozilla/login.webmaker.org/commit/a4845d6967601b4ebc735bbab2dea072998850f4
[Bug 885000] Changed user defaults for notifications/engagements

https://github.com/mozilla/login.webmaker.org/commit/ac1c0d59a0611b29cd9a531145eece2f96bb055a
Merge pull request #116 from ksedge/bug885000

[Bug 885000] Changed user defaults for notifications/engagements
(Assignee)

Comment 12

4 years ago
FOUR TO LIKE, GO OR SOMETHING!
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.