#169817560 add search functionality #52

gadishimwe · 2020-01-16T08:37:00Z

What does this PR do?

Add search functionality feature

Description of Task to be completed?

Add search controller
Add search service
Add search tests
Add search api documentation

How should this be manually tested?

$ git clone https://github.com/andela/the_spinners-backend.git
$ cd the_spinners-backend
$ git checkout  ft-search-functionality-169817560

then
$ npm run test

or you can use Postman to send a get request to /api/search?term={query} and replace the query with your search choice

What are the relevant pivotal tracker stories?

#169817560

Screenshots

src/services/search.service.js

erickyvand · 2020-01-16T10:03:28Z

src/swagger/search.swagger.js

+ *     tags:
+ *       - Search
+ *     name: Search the requests
+ *     parameters:


https://github.com/andela/the_spinners-backend/pull/48/files#r367259603

src/swagger/search.swagger.js

adesege · 2020-01-16T12:20:33Z

src/controllers/search.controller.js

+
+    const pagination = {
+      limit,
+      offset: Number.isNaN(offset) ? undefined : offset


Offset should be zero by default. Also, line 22 cannot return a NaN value so the check here is unnecessary.

@adesege Because pagination in this search is optional, I was thinking if the user searches without providing the page value or limit value, he should get the data without pagination. In that situation, line 22 would return NaN and that's why I was checking first. If you say that offset should be zero by default, there would be no pagination. Can you give me your suggestions? because for me without setting the default values for page and limit which would be pagination, I'm not finding any other option except checking.

adesege · 2020-01-16T12:21:21Z

src/controllers/search.controller.js

+      limit,
+      offset: Number.isNaN(offset) ? undefined : offset
+    };
+    const results = await SearchService.searchAll(userData, term, pagination);


Always pass an object as a single param to a function as opposed to setting multiple params

adesege · 2020-01-16T12:24:07Z

src/services/search.service.js

+ * @returns {response} @memberof SearchService
+ */
+  static getStatusQuery(term) {
+    if (term === 'approved' || term === 'rejected' || term === 'pending') {


Use the .includes() instead

adesege · 2020-01-20T10:38:36Z

src/controllers/search.controller.js

+      offset
+    };
+    let results;
+    if (location) {


What happens when the user searches by both location and name, for instance?

@adesege It returns results for both of them, see below image, it returns results for both location and status and they are different requests as you see

I don't think so. Run a search for approved status and location. Show the location property in the return object. Also, remove the pagination queries in the request url

Yh I get it. let me fix it

adesege · 2020-01-20T10:39:53Z

src/controllers/search.controller.js

+ *
+ * @class searchController
+ */
+class searchController {


Class names must be in pascal case

adesege · 2020-01-20T10:41:01Z

src/tests/fixtures/trip.fixture.js

 const newTripComment = {
  id: faker.random.number({ min: 20, max: 50 }),
  userId: loggedInUser.id,
+  tripId: faker.random.number(),
  tripType: 'one-way',
-  departure: faker.address.city(),
-  destination: faker.address.city(),
-  travelDate: faker.date.future(),
+  originId: faker.random.number({ min: 1, max: 2 }),
+  destinationId: faker.random.number({ min: 1, max: 2 }),
+  departureDate: faker.date.future(),
  travelReasons: faker.lorem.sentence(),
-  accommodation: faker.lorem.sentence(),
+  accommodationId: faker.random.number()
 };

 const tripComment = {
  userId: loggedInUser.id,
  tripType: 'one-way',
-  departure: faker.address.city(),
-  destination: faker.address.city(),
-  travelDate: faker.date.future(),
+  originId: faker.random.number({ min: 1, max: 2 }),
+  destinationId: faker.random.number({ min: 1, max: 2 }),
+  departureDate: faker.date.future(),
  travelReasons: faker.lorem.sentence(),
-  accommodation: faker.lorem.sentence(),
+  accommodationId: faker.random.number()
 };


Use spread operator to copy common object properties

adesege · 2020-01-20T11:08:09Z

src/tests/search/search.test.js

+chai.use(chaiHttp);
+
+describe('Test searching requests:', () => {
+  let departuredate;


A variable with multiple words should be in camel case

adesege · 2020-01-20T11:08:49Z

src/tests/search/search.test.js

+        done();
+      });
+  });
+  it('Should return status code of 201 on successful trip creation', (done) => {


Why are you creating a trip here?

I was creating a trip that later needs to be searched for. But maybe I can use the fixture instead

adesege · 2020-01-20T11:09:06Z

src/tests/search/search.test.js

+    const fakerDate = new Date(departuredate);
+    const month = (`0${fakerDate.getMonth() + 1}`).slice(-2);
+    const date = (`0${fakerDate.getDate()}`).slice(-2);
+    const formattedDate = `${fakerDate.getFullYear()}-${month}-${date}`;


Move this to a function

adesege · 2020-01-20T11:10:27Z

src/tests/search/search.test.js

+  });
+  it('Should return status code of 200 on with search results when search by status and with pagination', (done) => {
+    chai.request(app)
+      .get(`/api/search?status=pending&page=${faker.random.number({ min: 1, max: 1 })}&limit=${faker.random.number()}`)


There's no point using faker to generate the page and limit values.

- Add search controller - Add search service - Add search tests - Add search api documentation [stars #169817560]

gadishimwe requested review from victor-abz, erickyvand and higustave-ops January 16, 2020 08:37

gadishimwe added the needs review label Jan 16, 2020

erickyvand reviewed Jan 16, 2020

View reviewed changes

src/services/search.service.js Outdated Show resolved Hide resolved

erickyvand suggested changes Jan 16, 2020

View reviewed changes

gadishimwe added Work In Progress Work not yet finished and removed needs review labels Jan 16, 2020

gadishimwe force-pushed the ft-search-functionality-169817560 branch from aa759ee to 29127a2 Compare January 16, 2020 10:59

gadishimwe added needs review and removed Work In Progress Work not yet finished labels Jan 16, 2020

adesege reviewed Jan 16, 2020

View reviewed changes

gadishimwe added Work In Progress Work not yet finished and removed needs review labels Jan 16, 2020

adesege reviewed Jan 16, 2020

View reviewed changes

gadishimwe force-pushed the ft-search-functionality-169817560 branch from 09dfc7c to 3e6b3ed Compare January 17, 2020 13:16

erickyvand approved these changes Jan 17, 2020

View reviewed changes

gadishimwe added needs review and removed Work In Progress Work not yet finished labels Jan 17, 2020

adesege suggested changes Jan 20, 2020

View reviewed changes

adesege added Work In Progress Work not yet finished and removed needs review labels Jan 20, 2020

gadishimwe added 2 commits January 24, 2020 11:34

Feature(search): add search functionality

8b538bb

- Add search controller - Add search service - Add search tests - Add search api documentation [stars #169817560]

Apply feedback

c06f7d2

gadishimwe force-pushed the ft-search-functionality-169817560 branch 2 times, most recently from 1ea764d to fc647d5 Compare January 24, 2020 14:17

apply feedback

3351620

gadishimwe force-pushed the ft-search-functionality-169817560 branch from fc647d5 to 3351620 Compare January 24, 2020 14:27

gadishimwe removed the Work In Progress Work not yet finished label Jan 26, 2020

gadishimwe added the needs review label Jan 26, 2020

bdushimi merged commit 5976365 into develop Jan 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#169817560 add search functionality #52

#169817560 add search functionality #52

gadishimwe commented Jan 16, 2020 •

edited

Loading

erickyvand Jan 16, 2020

adesege Jan 16, 2020

gadishimwe Jan 16, 2020

adesege Jan 16, 2020

adesege Jan 16, 2020

adesege Jan 20, 2020

gadishimwe Jan 20, 2020

adesege Jan 20, 2020

gadishimwe Jan 20, 2020

adesege Jan 20, 2020

adesege Jan 20, 2020

adesege Jan 20, 2020

adesege Jan 20, 2020

gadishimwe Jan 20, 2020

adesege Jan 20, 2020

adesege Jan 20, 2020

#169817560 add search functionality #52

#169817560 add search functionality #52

Conversation

gadishimwe commented Jan 16, 2020 • edited Loading

What does this PR do?

Description of Task to be completed?

How should this be manually tested?

What are the relevant pivotal tracker stories?

Screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gadishimwe commented Jan 16, 2020 •

edited

Loading