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

Null basket server #2485

Merged
merged 2 commits into from May 28, 2015
Merged

Null basket server #2485

merged 2 commits into from May 28, 2015

Conversation

@zaach
Copy link
Contributor

@zaach zaach commented May 28, 2015

WIP, untested

Will help us test and develop locally. Meant to be a minimal copy of the Basket API.

/cc @shane-tomlinson

@zaach zaach force-pushed the issue-2443-basket-proxy branch from a3da6da to fa5aa01 May 28, 2015
@rfk
Copy link
Member

@rfk rfk commented May 28, 2015

This just-proxy-it-to-basket business is turning out to be a lot more work than I was hoping we'd get away with...@zaach @shane-tomlinson @vladikoff I salute your commitment to not shipping code that hasn't been properly tested.

function initApp() {
var app = express();
app.use(bodyParser.json());
app.use(verifyApiKey());

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

verifyApiKey is the actual middleware, no need for the ()

userData[email] = {
email: email,
token: token,
newsletters: params.newsletters.slipt(',')

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

slipt=>split.


app.post('/unsubscribe/:token/', function (req, res) {
var user = tokenToUser[req.body.token];
var newsletters = req.params.newsletters.slit(',');

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

slit=>split


function initApp() {
var app = express();
app.use(bodyParser.json());

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

We send the data to basket using form data instead of JSON.

} else {
user.newsletters = user.newsletters.concat(params.newsletters.split(','));
}
res.send(200, { status: 'ok' });

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

Express says this form is deprecated:

express deprecated res.send(status, body): Use res.status(status).send(body) instead null-basket-server.js:102:9
app.get('/lookup-user/', function (req, res) {
var email = req.params.email;
if (! userData[email]) {
res.status(400).json(errorResponse('unknown-email', BASKET_ERRORS.UNKNOWN_EMAIL));

This comment has been minimized.

res.status(400).json(errorResponse('unknown-email', BASKET_ERRORS.UNKNOWN_EMAIL));
return;
}
res.json(userData[email]);

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

This also needs to send a status: "ok" as part of the returned data.

app.use(verifyApiKey);

app.get('/lookup-user/', function (req, res) {
var email = req.params.email;

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

email comes over query parameters, use req.query here.

});

app.post('/unsubscribe/:token/', function (req, res) {
var user = tokenToUser[req.body.token];

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

the token comes from req.params, the newsletters from req.body.

@shane-tomlinson shane-tomlinson force-pushed the null-basket-server branch from 276995d to d305e94 May 28, 2015
zaach and others added 2 commits May 28, 2015
* `verifyApiKey` is already middleware, do not make a function call when setting it up as middleware.
* Fetch the parameters from the right place.
@shane-tomlinson shane-tomlinson force-pushed the null-basket-server branch from d305e94 to c708a13 May 28, 2015
}

return target;
}

This comment has been minimized.

@pdehaan

pdehaan May 28, 2015
Contributor

Curious, but why not just something like Underscore's _.extend()?

This comment has been minimized.

@shane-tomlinson

shane-tomlinson May 28, 2015
Member

underscore isn't an npm dependency, and I thought since it was just a single function that I need, I'll just hack it in.

This comment has been minimized.

@zaach

zaach May 28, 2015
Author Contributor

👍

@zaach zaach removed the WIP label May 28, 2015
@zaach zaach changed the title [WIP] Null basket server Null basket server May 28, 2015
zaach added a commit that referenced this pull request May 28, 2015
tests(basket): Null basket server r=shane-tomlinson
@zaach zaach merged commit 5aa3fe2 into issue-2443-basket-proxy May 28, 2015
3 checks passed
3 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 98.25%
Details
@zaach zaach deleted the null-basket-server branch May 28, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants