#169258650 User should be able to edit trip request #32

NiyoEric · 2019-11-26T16:30:57Z

What does this PR do?

This PR helps users to edit the trip request

Description of Task to be completed?

Why

Sometimes, mistakes happen and the requester needs to ensure the right information about the travel request is conveyed
As a requester, I should be able to edit a request that is still open
So that, I can immediately make modifications to it and submit for approval

How should this be manually tested?

git clone https://github.com/andela/team-odd-bn-backend.git
cd team-odd-bn-backend
git checkout -b ft-edit-travel-request-169258650
npm install
update .env file with super admin credentials
Drop existing migration npm run dropTables
Update migration npm run db-migrate
Populate the seed data npm run db-seed-dev
npm run dev-start check API in development
Pass trips request id in params
pass trips info in body
npm test testing API

Any background context you want to provide?

What are the relevant pivotal tracker stories?

#169258650

Screenshots (if appropriate)

Edit Two-way trip

Edit One-way trip

API Documentation

Questions:

ivymwende

@NiyoEric Great work done! Please take note of the few changes I have pointed out.

ivymwende · 2019-11-28T08:56:39Z

src/controllers/TripController.js

+    try {
+      const itinerary = req.body.itinerary ? req.body.itinerary : [req.body];
+      const Ids = await trips.findAll(
+        {
+          attributes: ['id'],
+          where: {
+            tripRequestId: req.params.id
+          }
+        }
+      );
+      await tripRequests.update(
+        { statusId: 1 },
+        { where: { id: req.params.id } }
+      );
+
+      itinerary.forEach(async (item, index) => {
+        const tripId = Ids[index].dataValues.id;
+        await trips.update({
+          originId: item.originId,
+          destinationId: item.destinationId,
+          reason: item.reason,
+          startDate: item.startDate,
+          returnDate: item.returnDate
+        },
+        {
+          where: { id: tripId }
+        });
+      });


@NiyoEric To avoid having a very bulky controller function I would recommend we refactor this. I'd recommend putting this in services to query the database.

ivymwende · 2019-11-28T09:01:04Z

src/routes/api/tripRoute.js

+    '/edit/:id',
+    verifyToken,
+    isUserVerified,
+    Validate.isIDInteger(),
+    checkInputDataError,
+    Validate.editRequest(),
+    checkInputDataError,
+    Conflict.isTripRequestFound,
+    IsOwnerOfTrip,
+    IsTripApproved,
+    ValidateTrip.checkIfOriginDestinationExists,
+    ValidateTrip.checkIfOriginSameAsDestination,
+    ValidateTrip.checkMultiCityForSimilarRequests,
+    ValidateTrip.checkForSimilarRequests,
+    TripController.editTrip
+  );


@NiyoEric I'd recommend sticking to one convention. If you are destructuring please ensure it is destructured through and through

hezronkimutai

Eric, please see the comments I have left.

hezronkimutai · 2019-11-28T09:16:56Z

src/tests/060-tripTest.js

@@ -3,7 +3,7 @@ import chaiHttp from 'chai-http';
 import dotenv from 'dotenv';
 import app from '../index';
 import tripMockData from './mock/tripMockData';
-import mockData from './mock/mockData';
+import mockData, { superAdminToken } from './mock/mockData';


Eric, can you help us refactor this, superAdminToken is in mockData.

hezronkimutai

You can make the format of the req.body to be uniform. Nice work.

victkarangwa

@NiyoEric you did a good job!
Just review some few comments I left.

victkarangwa · 2019-11-28T10:42:47Z

src/controllers/TripController.js

+    try {
+      const itinerary = req.body.itinerary ? req.body.itinerary : [req.body];
+      const Ids = await trips.findAll(


Nice shot @NiyoEric

victkarangwa · 2019-11-28T11:04:48Z

src/routes/api/tripRoute.js

+  .put(
+    '/edit/:id',


Can you use tripRequestId instead of id, so that we may all reflect on the same ID name?

I have changed it.

victkarangwa · 2019-11-28T11:07:11Z

src/routes/api/tripRoute.js

+ *
+ * /trips/edit/{id}:


tripRequestId instead of id

hound · 2019-11-28T17:17:42Z

src/services/TripService.js

@@ -0,0 +1,53 @@
+import database, { tripRequests, trips } from '../database/models';


Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

hound · 2019-11-28T17:17:42Z

src/services/CommonQueries.js

@@ -0,0 +1,69 @@
+
+import Customize from '../helpers/Customize';


Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

william000000

Looks good to go, keep it up! @NiyoEric

hezronkimutai

Nice work eric

hound · 2019-11-29T06:54:22Z

src/tests/060-tripTest.js

+  it('User should not edit and send the same city', (done) => {
+    chai.request(app)
+      .put(`/api/v1/trips/edit/${tripRequestMulticityId.id}`)
+      .set('token',mockData.superAdminToken)
