Webmaker RID script incorrectly parses values with hyphens

RESOLVED FIXED

Status

Webmaker
Metrics
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: adamlofting, Assigned: adamlofting)

Tracking

Details

(Whiteboard: [landingpages][metrics][June27])

Attachments

(4 attachments)

(Assignee)

Description

4 years ago
I'm filing this bug in case anyone has time to look at this before me, but if not I'll pick it up later. It should be really quick but I'm now in meetings for a few hours.

RE: https://github.com/mozilla/webmaker-rid

for sample ref codes like "makerparty2014-facebook-firefox"

The cookie gets set to "makerparty2014" (everything after the hyphen gets chopped)

For the snippet test that's about to start I've used an underscore in the RID so this isn't an immediate blocker, but I think hyphens should be acceptable in RIDs. And currently all the RIDs planned for MP are using the format makerparty2015-morespecific-thing
(Assignee)

Updated

4 years ago
Whiteboard: [landingpages][metrics][June27]
if we change the regular expression to /ref=((\w|-)+)/ it will capture hyphens.
(Assignee)

Comment 2

4 years ago
Created attachment 8445251 [details] [review]
https://github.com/mozilla/webmaker-rid/pull/6

Thanks Cade. I've dropped that in place.
Attachment #8445251 - Flags: review?(cade)
(Assignee)

Updated

4 years ago
Assignee: jon → adam
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED

Updated

4 years ago
Attachment #8445251 - Flags: review?(cade) → review+
We need to add that same regex to webmaker-auth-client

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED

Updated

4 years ago
Attachment #8445266 - Flags: review?(cade) → review+
we can close this now, or add PR's to update webmaker-auth-client on:

webmakerorg
thimble
popcorn
goggles
events
profile
appmaker
...login?
(Assignee)

Comment 8

4 years ago
Created attachment 8445750 [details] [review]
https://github.com/mozilla/webmaker-auth-client/pull/29

Is this the correct way to bump the version number before updating elsewhere?
Attachment #8445750 - Flags: review?(cade)

Comment 9

4 years ago
Comment on attachment 8445750 [details] [review]
https://github.com/mozilla/webmaker-auth-client/pull/29

r-. To bump the version number we land whatever patches need to be landed, then use mversion to bump package.json and bower.json versions:

Install mversion: npm install -g mversion
Use mversion: mversion patch -m
Push it up: git push mozilla master --tags
Attachment #8445750 - Flags: review?(cade) → review-
(Assignee)

Comment 10

4 years ago
Thanks, the version number has now been bumped with mversion and pushed to master.

Comment 11

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

https://github.com/mozilla/webmaker.org/commit/6c5fb95afab959c2f4bb1316b11cf5223d5c4916
Merge pull request #833 from adamlofting/bug1029536

update webmaker-auth-client

Comment 12

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

https://github.com/mozilla/thimble.webmaker.org/commit/8d3d823a919b8176b311d4aef7fee860ae3d7360
Merge pull request #433 from adamlofting/bug1029536

update webmaker-auth-client and webmaker-analytics

Comment 13

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

https://github.com/mozilla/popcorn.webmaker.org/commit/c434ce614d4b56b4373dbf92e197d19990f8f33f
Merge pull request #554 from adamlofting/bug1029536

update webmaker-auth-client and webmaker-analytics

Comment 14

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

https://github.com/mozilla/goggles.webmaker.org/commit/56e22cbd882a746d7047f75828a21b69987c950b
Merge pull request #138 from adamlofting/bug1029536

update webmaker-auth-client and webmaker-analytics

Comment 15

4 years ago
Commit pushed to master at https://github.com/mozilla/webmaker-events-2

https://github.com/mozilla/webmaker-events-2/commit/d0a2b2d868ef6dcdc87e982bf7b07b2b04ae767c
Merge pull request #109 from adamlofting/bug1029536

update webmaker-auth-client and webmaker-analytics

Comment 16

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

https://github.com/mozilla/login.webmaker.org/commit/06d92ea3925fd1e8d1de6585c467cf81644fcf2b
Merge pull request #281 from adamlofting/bug1029536

update webmaker-auth-client and webmaker-analytics

Comment 17

4 years ago
Commit pushed to master at https://github.com/mozilla/webmaker-profile-2

https://github.com/mozilla/webmaker-profile-2/commit/06c39a83a976603bfad0e377f83eef3fe4aa9143
Merge pull request #50 from adamlofting/bug1029536

update webmaker-auth-client
(Assignee)

Comment 18

4 years ago
Created attachment 8447236 [details] [review]
https://github.com/mozilla/webmaker-auth-client/pull/30

Just spotted we didn't do this after the regex fix.
Attachment #8447236 - Flags: review?(jon)

Updated

4 years ago
Attachment #8447236 - Flags: review?(jon) → review+
You need to log in before you can comment on or make changes to this bug.