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

add search travel request #33

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

t4christ
Copy link
Collaborator

@t4christ t4christ commented Sep 13, 2019

What does this PR do?

This PR allows users to search for travel request.

Description of Task to be completed

 Search for travel request based on origin and destination

How should this be manually tested?

  Using postman

Screen Shot 2019-09-13 at 1 23 10 PM

What are the relevant pivotal tracker stories?

  #167730650

}
return next();
}

Copy link

Choose a reason for hiding this comment

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

Too many blank lines at the end of file. Max of 1 allowed no-multiple-empty-lines

return errorResponse(res, statusCode.badRequest, error);
}
return next();
}
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 0 spaces but found 1 tab indent
Missing semicolon semi

});
return errorResponse(res, statusCode.badRequest, error);
}
return next();
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 2 tabs indent

error.push(err.msg);
});
return errorResponse(res, statusCode.badRequest, error);
}
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 2 tabs indent

errors.array().forEach((err) => {
error.push(err.msg);
});
return errorResponse(res, statusCode.badRequest, error);
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 12 indent

.withMessage(message.lettersAlone)
]

export const validateTravelResult = (req, res, next) => {
Copy link

Choose a reason for hiding this comment

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

Multiple spaces found before '=' no-multi-spaces

.withMessage(message.lettersAlone),
check('destination').not().isInt()
.withMessage(message.lettersAlone)
]
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 0 spaces but found 4 indent
Missing semicolon semi

check('origin').not().isInt()
.withMessage(message.lettersAlone),
check('destination').not().isInt()
.withMessage(message.lettersAlone)
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 8 indent

export const validateTravelSearch = [
check('origin').not().isInt()
.withMessage(message.lettersAlone),
check('destination').not().isInt()
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 2 spaces but found 8 indent


export const validateTravelSearch = [
check('origin').not().isInt()
.withMessage(message.lettersAlone),
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 8 indent

.not()
.isInt()
.withMessage(message.lettersAlone),
check("destination")
Copy link

Choose a reason for hiding this comment

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

Strings must use singlequote quotes

};

export const validateTravelSearch = [
check("origin")
Copy link

Choose a reason for hiding this comment

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

Strings must use singlequote quotes

.not()
.isEmpty()
.withMessage(message.emptyDepartureDate),
check("travel_purpose")
Copy link

Choose a reason for hiding this comment

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

Strings must use singlequote quotes

.withMessage(message.emptyDestination)
.isAlpha()
.withMessage(message.lettersAlone),
check('departure_date').not().isEmpty().withMessage(message.emptyDepartureDate),
check('travel_purpose').not().isEmpty().withMessage(message.emptyTravelPurpose),
check("departure_date")
Copy link

Choose a reason for hiding this comment

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

Strings must use singlequote quotes

.withMessage(message.emptyOrigin)
.isAlpha()
.withMessage(message.lettersAlone),
check('destination').not().isEmpty()
check("destination")
Copy link

Choose a reason for hiding this comment

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

Strings must use singlequote quotes

const searchTravelRequest = {
"origin":"NewYork, USA",
"destination":"Paris, France"
}
Copy link

Choose a reason for hiding this comment

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

Expected indentation of 4 spaces but found 6 indent
Missing semicolon semi

it('should return search result based on origin and destination', (done) => {
const searchTravelRequest = {
"origin":"NewYork, USA",
"destination":"Paris, France"
Copy link

Choose a reason for hiding this comment

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

Missing space before value for key 'destination' key-spacing
Strings must use singlequote quotes
Unnecessarily quoted property 'destination' found quote-props


it('should return search result based on origin and destination', (done) => {
const searchTravelRequest = {
"origin":"NewYork, USA",
Copy link

Choose a reason for hiding this comment

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

Missing space before value for key 'origin' key-spacing
Strings must use singlequote quotes
Unnecessarily quoted property 'origin' found quote-props

return {
origin: body[key]
};
case "destination":
Copy link

Choose a reason for hiding this comment

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

Strings must use singlequote quotes

const { limit, offset } = computeLimitAndOffset(page, perPage);
const searchValue = Object.keys(body).map(key => {
switch (key) {
case "origin":
Copy link

Choose a reason for hiding this comment

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

Strings must use singlequote quotes

Copy link
Collaborator

@captainamiedi captainamiedi left a comment

Choose a reason for hiding this comment

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

nice work. i can see alot of hard work here but i can see console.log on your controller file

@@ -13,6 +14,7 @@ route.post('/onewaytrip', getToken, verifyToken, validateTravelRequest, validate

// handles manager pending req approvals route
route.get('/requests/pending/:manager', getToken, verifyToken, pendingManagerApproval);
route.get('/search/travels',validateTravelSearch, validateTravelResult,searchTravels);
Copy link

Choose a reason for hiding this comment

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

A space is required after ',' comma-spacing

import { getToken, verifyToken } from '../middlewares/tokenMiddleware';
import travelValidator,{validateTravelSearch, validateTravelResult} from '../validation/travelValidation';
Copy link

Choose a reason for hiding this comment

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

A space is required after ',' comma-spacing
A space is required after '{' object-curly-spacing
A space is required before '}' object-curly-spacing

@@ -1,8 +1,9 @@
import { Router } from 'express';

import travelControllers from '../controllers/travelControllers';
import travelValidator from '../validation/travelValidation';
import travelControllers, {searchTravels} from '../controllers/travelControllers';
Copy link

Choose a reason for hiding this comment

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

A space is required after '{' object-curly-spacing
A space is required before '}' object-curly-spacing

const meta = paginate(page, perPage, count, rows);
return res.status(200).json({ success: { requests: rows, meta } });
} catch (error) {
return res.status(404).json({ error: error });
Copy link

Choose a reason for hiding this comment

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

Expected property shorthand object-shorthand

Copy link
Collaborator

Choose a reason for hiding this comment

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

@t4christ Excellent

There is a defined error handler in the utils folder, instead of res.status(404)..... and also a success response too... instead of res.status(200)....

} catch (error) {

Copy link

Choose a reason for hiding this comment

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

Trailing spaces not allowed no-trailing-spaces

} else {
if (role === 'admin') {
return errorResponse(res, statusCode.unauthorized, message.unauthorized);
} else {
Copy link

Choose a reason for hiding this comment

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

Unnecessary 'else' after 'return' no-else-return

const filteredRequests = requestsPending.filter(request => request['user.department.line_manager'] !== null);
const filteredRequests = requestsPending.filter(
request => request['user.department.line_manager'] !== null
);
Copy link

Choose a reason for hiding this comment

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

Unexpected newline before ')' function-paren-newline

@@ -46,29 +56,60 @@ export default {
try {
const requestsPending = await showManagerPendingAppr(manager);

const filteredRequests = requestsPending.filter(request => request['user.department.line_manager'] !== null);
const filteredRequests = requestsPending.filter(
Copy link

Choose a reason for hiding this comment

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

Unexpected newline after '(' function-paren-newline

Copy link
Collaborator

@captainamiedi captainamiedi left a comment

Choose a reason for hiding this comment

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

I'm having difficulty using the GET verb to search using Post man.

@t4christ
Copy link
Collaborator Author

I'm having difficulty using the GET verb to search using Post man.

You have to add origin and destination in the body not as a parameter. When using a get request

Copy link
Collaborator

@Marshalheri Marshalheri left a comment

Choose a reason for hiding this comment

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

Nice work team mate.
Please do the following:

  • remove the bearer string you appended to your token value in your test file the token generator now appends it as we discussed.
  • also just like everything that involved authentication started with /auth make all that involves the travel to start with /travel for consistency

yeah. i was able to create a return trip. i have approve as well.

Copy link
Collaborator

@chuksjoe chuksjoe left a comment

Choose a reason for hiding this comment

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

Great work bro @t4christ, but I will like to bring to your attention that the users should be able to search based on the column headers which are: request ID, owner, destination, origin, duration, start date, request status, etc.
I will like to know if your implementation supports the above.

I already thought of it and i wanted to pass it accross the house that i think the only thing a user needs to search for is the origin and destination which will return the search result containing other components. A user will mostly be interested in the origin and destination of a travel

Copy link

@iakhator iakhator left a comment

Choose a reason for hiding this comment

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

still coding style everywhere

I don't understand what you mean sir

@chuksjoe
Copy link
Collaborator

Great work bro @t4christ, but I will like to bring to your attention that the users should be able to search based on the column headers which are: request ID, owner, destination, origin, duration, start date, request status, etc.
I will like to know if your implementation supports the above.

I already thought of it and i wanted to pass it accross the house that i think the only thing a user needs to search for is the origin and destination which will return the search result containing other components. A user will mostly be interested in the origin and destination of a travel

You can go through the PT story description to get the full details of the story so you don't miss the full picture of the task. As a user, I might want to get a list of my approved request or the likes.

Copy link
Collaborator

@OmoloyinA OmoloyinA left a comment

Choose a reason for hiding this comment

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

Nice work you've done here bro. However, I will like you to resolve conflicts and I am good to go with your PR.

Copy link
Collaborator

@Kaytbode Kaytbode left a comment

Choose a reason for hiding this comment

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

@t4christ Appreciate what you did here

Left some comments for you while going through your code. Please attend to them, and get hound to stop dancing..

Copy link
Collaborator

@Marshalheri Marshalheri left a comment

Choose a reason for hiding this comment

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

Nice work team mate.
Please do effect the changes kayode recommended, so i can approve.
Your work is ok. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants