If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

move the "user/:id" route definition into webmaker-loginapi so it gets set up automatically

RESOLVED FIXED

Status

Webmaker
Login
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: pomax, Assigned: sedge)

Tracking

Details

(Whiteboard: u=dev c=login p=1 s=2013w22)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Right now we're telling people to manually set up a route for handling the call to the local /user/:id location, which then calls loginAPI, it would be much nicer if the loginAPI set up that route as a dedicate route the same way express-persona sets up the persona routes so you don't have to worry about them.
(Assignee)

Updated

4 years ago
Assignee: nobody → kieran.sedgwick
Assignee: kieran.sedgwick → ali
Status: NEW → ASSIGNED
Created attachment 751762 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/10

https://github.com/mozilla/node-webmaker-loginapi/pull/10

I have added the code in to the module but have to alter the way how it's return and how the function is being declare, so that the code that I included can access the functions.
Attachment #751762 - Flags: review?(kieran.sedgwick)
Attachment #751762 - Flags: review?(david.humphrey)
(Assignee)

Comment 2

4 years ago
Comment on attachment 751762 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/10

A few minor touch-ups in the PR that need to be fixed, and you must enable travis for this PR - ask if you need help with that!
Attachment #751762 - Flags: review?(kieran.sedgwick) → review-
Comment on attachment 751762 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/10

This has failed on Travis because it needs to pass the parameters and one that needs to be pass is the express, so without that being pass it will fail because it can't find the methods from the function.

https://travis-ci.org/alicoding/node-webmaker-loginapi/jobs/7380860
Attachment #751762 - Attachment is obsolete: true
Attachment #751762 - Flags: review?(david.humphrey)
(Assignee)

Updated

4 years ago
Assignee: ali → kieran.sedgwick
(Assignee)

Updated

4 years ago
Depends on: 876841
(Assignee)

Updated

4 years ago
Blocks: 876852
(Assignee)

Comment 4

4 years ago
Created attachment 755486 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/11

https://github.com/mozilla/node-webmaker-loginapi/pull/11
Attachment #755486 - Flags: review?(david.humphrey)
Comment on attachment 755486 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/11

Comments in PR.  Also, it looks like your tests aren't passing the express app object, as you're failing travis:

/home/travis/build/ksedge/node-webmaker-loginapi/index.js:86

app.get( "/user/:userid", function( req, res ) {

^

TypeError: Cannot call method 'get' of undefined

at module.exports (/home/travis/build/ksedge/node-webmaker-loginapi/index.js:86:7)

at Object.<anonymous> (/home/travis/build/ksedge/node-webmaker-loginapi/test/test.js:21:38)

at Module._compile (module.js:449:26)

at Object.Module._extensions..js (module.js:467:10)

at Module.load (module.js:356:32)

at Function.Module._load (module.js:312:12)

at Module.require (module.js:362:17)

at require (module.js:378:17)

at Mocha.loadFiles (/home/travis/build/ksedge/node-webmaker-loginapi/node_modules/mocha/lib/mocha.js:152:27)

at Array.forEach (native)

at Mocha.loadFiles (/home/travis/build/ksedge/node-webmaker-loginapi/node_modules/mocha/lib/mocha.js:149:14)

at Mocha.run (/home/travis/build/ksedge/node-webmaker-loginapi/node_modules/mocha/lib/mocha.js:305:31)

at Object.<anonymous> (/home/travis/build/ksedge/node-webmaker-loginapi/node_modules/mocha/bin/_mocha:327:7)

at Module._compile (module.js:449:26)

at Object.Module._extensions..js (module.js:467:10)

at Module.load (module.js:356:32)

at Function.Module._load (module.js:312:12)

at Module.runMain (module.js:492:10)

at process.startup.processNextTick.process._tickCallback (node.js:245:9)
Attachment #755486 - Flags: review?(david.humphrey) → review-
(Assignee)

Comment 6

4 years ago
Comment on attachment 755486 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/11

Okay, second pass. As for the travis tests, do you have a suggestion? Is it worth doing a shim?
Attachment #755486 - Flags: review?(pomax)
Attachment #755486 - Flags: review?(david.humphrey)
Attachment #755486 - Flags: review-
(Assignee)

Comment 7

4 years ago
UPDATE: I did a shim :P
Comment on attachment 755486 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/11

I'll let Pomax sign off on it.  One question in PR.
Attachment #755486 - Flags: review?(david.humphrey) → review+
(Reporter)

Updated

4 years ago
Attachment #755486 - Attachment description: Pull request for this patch → https://github.com/mozilla/node-webmaker-loginapi/pull/11
(Reporter)

Comment 9

4 years ago
Comment on attachment 755486 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/11

I'd really like to see the README to not show the internals (people can view source for that) but instead give the practical information only. And an arg ordering nit. Those are the only things I got.
Attachment #755486 - Flags: review?(pomax) → review-
(Assignee)

Comment 10

4 years ago
Comment on attachment 755486 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/11

AGAIN!

https://github.com/mozilla/node-webmaker-loginapi/pull/11
Attachment #755486 - Flags: review- → review?(pomax)
(Reporter)

Comment 11

4 years ago
Comment on attachment 755486 [details] [review]
https://github.com/mozilla/node-webmaker-loginapi/pull/11

R+, landing time.
Attachment #755486 - Flags: review?(pomax) → review+
(Assignee)

Comment 12

4 years ago
https://github.com/mozilla/node-webmaker-loginapi/commit/4ddc98412c8f90ded8d8c06d4cb298c1a661118d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: c=login → u=dev c=login p=1 s=2013w22
(Assignee)

Updated

4 years ago
No longer blocks: 876852
Attachment mime type: text/plain → text/x-github-pull-request
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.