Skip to content

fix: for issue 51, add support sorting by 'GET /resources' endpoint #54

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Dec 21, 2020
Merged

fix: for issue 51, add support sorting by 'GET /resources' endpoint #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/default.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ module.exports = {
// AWS_SECRET_ACCESS_KEY: process.env.AWS_FAKE_KEY,
AWS_REGION: process.env.AWS_REGION || 'us-east-1',
IS_LOCAL_DB: process.env.IS_LOCAL_DB ? process.env.IS_LOCAL_DB === 'true' : true,
URL: process.env.DYNAMODB_URL || 'http://localhost:8000',
DYNAMODB_URL: process.env.DYNAMODB_URL || 'http://localhost:8000',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what this change is for. Did you change all the uses of this value throughout the app?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In all places it tries to get Dynamo DB url using "DYNAMODB.DYNAMODB_URL" path and I need to change only this file.

AWS_READ_UNITS: process.env.AWS_READ_UNITS || 4,
AWS_WRITE_UNITS: process.env.AWS_WRITE_UNITS || 2,
TIMEOUT: process.env.DYNAMODB_TIMEOUT || 10000
Expand Down
10 changes: 10 additions & 0 deletions docs/swagger.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ paths:
format: UUID
in: query
required: false
- name: sortBy
type: string
description: Sort the results by the field.
in: query
required: false
- name: sortOrder
type: string
description: Order the results by the asc/desc.
in: query
required: false
responses:
'200':
description: OK - the request was successful
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/ResourceController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const helper = require('../common/helper')
* @param {Object} res the response
*/
async function getResources (req, res) {
const result = await service.getResources(req.authUser, req.query.challengeId, req.query.roleId, req.query.page, req.query.perPage)
const result = await service.getResources(req.authUser, req.query.challengeId, req.query.roleId, req.query.page, req.query.perPage, req.query.sortBy, req.query.sortOrder)
helper.setResHeaders(req, res, result)
res.send(result.data)
}
Expand Down
6 changes: 5 additions & 1 deletion src/init-es.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ const initES = async () => {
const body = { mappings: {} }
body.mappings[config.get('ES.ES_TYPE')] = {
properties: {
id: { type: 'keyword' }
id: { type: 'keyword' },
memberHandle: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this change mean we have to do anything to the existing Dynamo DB? I don't see any change to the model. Just making sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to change any thing with Dynamo DB, because search is making in Elasticsearch.

type: 'text',
fielddata: true
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/scripts/create-tables.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const logger = require('../common/logger')
logger.info('Requesting to create tables...')

const promises = []
const skipModels = ['DynamoDB', 'MemberStats', 'MemberProfile']
const skipModels = ['DynamoDB', 'MemberProfile']

Object.keys(models).forEach(modelName => {
if (!includes(skipModels, modelName)) {
Expand Down
9 changes: 6 additions & 3 deletions src/services/ResourceService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async function checkAccess (currentUser, resources) {
* @param {Number} perPage The number of items to list per page
* @returns {Array} the search result
*/
async function getResources (currentUser, challengeId, roleId, page, perPage) {
async function getResources (currentUser, challengeId, roleId, page, perPage, sortBy, sortOrder) {
if (!validateUUID(challengeId)) {
throw new errors.BadRequestError(`Challenge ID ${challengeId} must be a valid v5 Challenge Id (UUID)`)
}
Expand Down Expand Up @@ -118,7 +118,8 @@ async function getResources (currentUser, challengeId, roleId, page, perPage) {
bool: {
must: mustQuery
}
}
},
sort: [{ [sortBy]: { 'order': sortOrder } }]
}
}
const esClient = await helper.getESClient()
Expand Down Expand Up @@ -180,7 +181,9 @@ getResources.schema = {
challengeId: Joi.id(),
roleId: Joi.optionalId(),
page: Joi.page(),
perPage: Joi.perPage()
perPage: Joi.perPage(),
sortBy: Joi.string().valid('memberHandle', 'created').required(),
sortOrder: Joi.string().valid('desc', 'asc').required()
}

/**
Expand Down