Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flexible pricing endpoints #1301

Merged
merged 13 commits into from
Jun 16, 2020
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ way to update this template, but currently, we follow a pattern:

## Upcoming version 2020-XX-XX

- [add] Add util functions for constructing line items in FTW backend
[#1299](https://github.com/sharetribe/ftw-daily/pull/1299)
- [add] Add local API endpoints for flexible pricing and privileged transitions
[#1301](https://github.com/sharetribe/ftw-daily/pull/1301)
- [fix] `yarn run dev-backend` was expecting NODE_ENV.
[#1303](https://github.com/sharetribe/ftw-daily/pull/1303)

Expand Down
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"compression": "^1.7.4",
"cookie-parser": "^1.4.5",
"core-js": "^3.1.4",
"cors": "^2.8.5",
"decimal.js": "10.2.0",
"dotenv": "6.2.0",
"dotenv-expand": "4.2.0",
Expand Down Expand Up @@ -51,7 +52,7 @@
"redux": "^4.0.5",
"redux-thunk": "^2.3.0",
"seedrandom": "^3.0.5",
"sharetribe-flex-sdk": "^1.9.1",
"sharetribe-flex-sdk": "1.11.0",
"sharetribe-scripts": "3.1.1",
"smoothscroll-polyfill": "^0.4.0",
"source-map-support": "^0.5.9",
Expand Down Expand Up @@ -87,8 +88,8 @@
"config": "node scripts/config.js",
"config-check": "node scripts/config.js --check",
"dev-frontend": "sharetribe-scripts start",
"dev-backend": "export NODE_ENV=development DEV_API_SERVER_PORT=3500&&nodemon server/apiServer.js",
"dev": "yarn run config-check&&concurrently --kill-others \"yarn run dev-frontend\" \"yarn run dev-backend\"",
"dev-backend": "nodemon server/apiServer.js",
"dev": "yarn run config-check&&export NODE_ENV=development REACT_APP_DEV_API_SERVER_PORT=3500&&concurrently --kill-others \"yarn run dev-frontend\" \"yarn run dev-backend\"",
"build": "sharetribe-scripts build",
"format": "prettier --write '**/*.{js,css}'",
"format-ci": "prettier --list-different '**/*.{js,css}'",
Expand Down
128 changes: 128 additions & 0 deletions server/api-util/sdk.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
const http = require('http');
const https = require('https');
const Decimal = require('decimal.js');
const sharetribeSdk = require('sharetribe-flex-sdk');

const CLIENT_ID = process.env.REACT_APP_SHARETRIBE_SDK_CLIENT_ID;
const CLIENT_SECRET = process.env.SHARETRIBE_SDK_CLIENT_SECRET;
const USING_SSL = process.env.REACT_APP_SHARETRIBE_USING_SSL === 'true';
const TRANSIT_VERBOSE = process.env.REACT_APP_SHARETRIBE_SDK_TRANSIT_VERBOSE === 'true';

// Application type handlers for JS SDK.
//
// NOTE: keep in sync with `typeHandlers` in `src/util/api.js`
const typeHandlers = [
// Use Decimal type instead of SDK's BigDecimal.
{
type: sharetribeSdk.types.BigDecimal,
customType: Decimal,
writer: v => new sharetribeSdk.types.BigDecimal(v.toString()),
reader: v => new Decimal(v.value),
},
];
exports.typeHandlers = typeHandlers;

const baseUrlMaybe = process.env.REACT_APP_SHARETRIBE_SDK_BASE_URL
? { baseUrl: process.env.REACT_APP_SHARETRIBE_SDK_BASE_URL }
: null;
const httpAgent = new http.Agent({ keepAlive: true });
const httpsAgent = new https.Agent({ keepAlive: true });

const memoryStore = token => {
const store = sharetribeSdk.tokenStore.memoryStore();
store.setToken(token);
return store;
};

// Read the user token from the request cookie
const getUserToken = req => {
const cookieTokenStore = sharetribeSdk.tokenStore.expressCookieStore({
clientId: CLIENT_ID,
req,
secure: USING_SSL,
});
return cookieTokenStore.getToken();
};

exports.serialize = data => {
return sharetribeSdk.transit.write(data, { typeHandlers, verbose: TRANSIT_VERBOSE });
};

exports.deserialize = str => {
return sharetribeSdk.transit.read(str, { typeHandlers });
};

exports.handleError = (res, error) => {
console.error(error);
if (error.status && error.statusText && error.data) {
// JS SDK error
res
.status(error.status)
.json({
status: error.status,
statusText: error.statusText,
data: error.data,
})
.end();
} else {
res
.status(500)
.json({ error: error.message })
.end();
}
};

exports.getSdk = (req, res) => {
return sharetribeSdk.createInstance({
transitVerbose: TRANSIT_VERBOSE,
clientId: CLIENT_ID,
httpAgent,
httpsAgent,
tokenStore: sharetribeSdk.tokenStore.expressCookieStore({
clientId: CLIENT_ID,
req,
res,
secure: USING_SSL,
}),
typeHandlers,
...baseUrlMaybe,
});
};

exports.getTrustedSdk = req => {
const userToken = getUserToken(req);

// Initiate an SDK instance for token exchange
const sdk = sharetribeSdk.createInstance({
transitVerbose: TRANSIT_VERBOSE,
clientId: CLIENT_ID,
clientSecret: CLIENT_SECRET,
httpAgent,
httpsAgent,
tokenStore: memoryStore(userToken),
typeHandlers,
...baseUrlMaybe,
});

// Perform a token exchange
return sdk.exchangeToken().then(response => {
// Setup a trusted sdk with the token we got from the exchange:
const trustedToken = response.data;

return sharetribeSdk.createInstance({
transitVerbose: TRANSIT_VERBOSE,

// We don't need CLIENT_SECRET here anymore
clientId: CLIENT_ID,

// Important! Do not use a cookieTokenStore here but a memoryStore
// instead so that we don't leak the token back to browser client.
tokenStore: memoryStore(trustedToken),

httpAgent,
httpsAgent,
typeHandlers,
...baseUrlMaybe,
});
});
};
54 changes: 54 additions & 0 deletions server/api/initiate-privileged.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
const { transactionLineItems } = require('../api-util/lineItems');
const { getSdk, getTrustedSdk, handleError, serialize } = require('../api-util/sdk');

module.exports = (req, res) => {
const { isSpeculative, bookingData, bodyParams, queryParams } = req.body;

const listingId = bodyParams && bodyParams.params ? bodyParams.params.listingId : null;

const sdk = getSdk(req, res);
let lineItems = null;

sdk.listings
.show({ id: listingId })
.then(listingResponse => {
const listing = listingResponse.data.data;
lineItems = transactionLineItems(listing, bookingData);

return getTrustedSdk(req);
})
.then(trustedSdk => {
const { params } = bodyParams;

// Add lineItems to the body params
const body = {
...bodyParams,
params: {
...params,
lineItems,
},
};

if (isSpeculative) {
return trustedSdk.transactions.initiateSpeculative(body, queryParams);
}
return trustedSdk.transactions.initiate(body, queryParams);
})
.then(apiResponse => {
const { status, statusText, data } = apiResponse;
res
.status(status)
.set('Content-Type', 'application/transit+json')
.send(
serialize({
status,
statusText,
data,
})
)
.end();
})
.catch(e => {
handleError(res, e);
});
};
10 changes: 2 additions & 8 deletions server/api/login-as.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const http = require('http');
const https = require('https');
const sharetribeSdk = require('sharetribe-flex-sdk');
const Decimal = require('decimal.js');
const sdkUtils = require('../api-util/sdk');

const CLIENT_ID = process.env.REACT_APP_SHARETRIBE_SDK_CLIENT_ID;
const ROOT_URL = process.env.REACT_APP_CANONICAL_ROOT_URL;
Expand Down Expand Up @@ -71,14 +72,7 @@ module.exports = (req, res) => {
httpAgent: httpAgent,
httpsAgent: httpsAgent,
tokenStore,
typeHandlers: [
{
type: sharetribeSdk.types.BigDecimal,
customType: Decimal,
writer: v => new sharetribeSdk.types.BigDecimal(v.toString()),
reader: v => new Decimal(v.value),
},
],
typeHandlers: sdkUtils.typeHandlers,
...baseUrl,
});

Expand Down
26 changes: 26 additions & 0 deletions server/api/transaction-line-items.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
const { transactionLineItems } = require('../api-util/lineItems');
const { getSdk, handleError, serialize } = require('../api-util/sdk');

module.exports = (req, res) => {
const { isOwnListing, listingId, bookingData } = req.body;

const sdk = getSdk(req, res);

const listingPromise = isOwnListing
? sdk.ownListings.show({ id: listingId })
: sdk.listings.show({ id: listingId });

listingPromise
.then(apiResponse => {
const listing = apiResponse.data.data;
const lineItems = transactionLineItems(listing, bookingData);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we are using this endpoint directly in FTW I added the lineItem validation here in estimated transaction branch (b9ba586) but now I noticed that this is also used in the other endpoints internally so I'm thinking should we add some flag (e.g. validateLineItems true/false) to the API call or do something else. Anyway, I think it might be clearer to move these changes to this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, is there any harm in always validating the line items? Then it could be inside the transactionLineItems helper.

Copy link
Contributor

@OtterleyW OtterleyW Jun 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there should be any harm but it's just extra step that is not always needed. If I understood correctly Marketplace API should anyway calculate the lineTotal even if the total is already passed in the API request and check that these values match.

Copy link
Contributor Author

@kpuputti kpuputti Jun 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, now I understand. It doesn't make sense to add the totals if the line items are sent to the Marketplace API that also adds them. It would be just confusing. But the UI needs those for the line items endpoint.

I think a separate function is cleaner than an option, so your changes in the commit you linked seem good IMO. The other places that use line items are sending those to the Marketplace API, and shouldn't add the line totals. Or am I missing something?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I checked the code again and realized that I got little confused. Somehow I thought that we are calling the transaction line items endpoint in initiate transaction endpoint too but obviously we are just using the transactionLineItems helper function there. So my bad... (but makes me wonder if we should think about the name of the function again).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I understand the confusion. Maybe you were confused as the API helper name for the endpoint is the same as the line item util function? I wouldn't rename the functions, most customizers will only have to deal with the util function...

res
.status(200)
.set('Content-Type', 'application/transit+json')
.send(serialize({ data: lineItems }))
.end();
})
.catch(e => {
handleError(res, e);
});
};
52 changes: 52 additions & 0 deletions server/api/transition-privileged.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
const { transactionLineItems } = require('../api-util/lineItems');
const { getSdk, getTrustedSdk, handleError, serialize } = require('../api-util/sdk');

module.exports = (req, res) => {
const { isSpeculative, bookingData, bodyParams, queryParams } = req.body;

const { listingId, ...restParams } = bodyParams && bodyParams.params ? bodyParams.params : {};

const sdk = getSdk(req, res);
let lineItems = null;

sdk.listings
.show({ id: listingId })
.then(listingResponse => {
const listing = listingResponse.data.data;
lineItems = transactionLineItems(listing, bookingData);

return getTrustedSdk(req);
})
.then(trustedSdk => {
// Add lineItems to the body params
const body = {
...bodyParams,
params: {
...restParams,
lineItems,
},
};

if (isSpeculative) {
return trustedSdk.transactions.transitionSpeculative(body, queryParams);
}
return trustedSdk.transactions.transition(body, queryParams);
})
.then(apiResponse => {
const { status, statusText, data } = apiResponse;
res
.status(status)
.set('Content-Type', 'application/transit+json')
.send(
serialize({
status,
statusText,
data,
})
)
.end();
})
.catch(e => {
handleError(res, e);
});
};
34 changes: 34 additions & 0 deletions server/apiRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,47 @@
*/

const express = require('express');
const bodyParser = require('body-parser');
const { deserialize } = require('./api-util/sdk');

const initiateLoginAs = require('./api/initiate-login-as');
const loginAs = require('./api/login-as');
const transactionLineItems = require('./api/transaction-line-items');
const initiatePrivileged = require('./api/initiate-privileged');
const transitionPrivileged = require('./api/transition-privileged');

const router = express.Router();

// ================ API router middleware: ================ //

// Parse Transit body first to a string
router.use(
bodyParser.text({
type: 'application/transit+json',
})
);

// Deserialize Transit body string to JS data
router.use((req, res, next) => {
if (req.get('Content-Type') === 'application/transit+json' && typeof req.body === 'string') {
try {
req.body = deserialize(req.body);
} catch (e) {
console.error('Failed to parse request body as Transit:');
console.error(e);
res.status(400).send('Invalid Transit in request body.');
return;
}
}
next();
});

// ================ API router endpoints: ================ //

router.get('/initiate-login-as', initiateLoginAs);
router.get('/login-as', loginAs);
router.post('/transaction-line-items', transactionLineItems);
router.post('/initiate-privileged', initiatePrivileged);
router.post('/transition-privileged', transitionPrivileged);

module.exports = router;
Loading