1. BIG server clean & API response schema standardization (#266)

:(

* Update API response schema to always include 'status' and 'data'

* Lots of schema changes and db migration to remove splash phot

* Profile schema standardize select

* Remove console log

* Add ability to delete last photo

* Remove last photo deletion status code

* Fix delete photo changes to tests

* Splash photo id nullable true on down migration

* rollback last commit

* splash photo nullable on down migration

* fix weird flowtype issue in get-photo

* add flowtype error comment

* Update status with return flowtypes

* Add comment about flowfixme issue

* Update comments in api/users utils for select profile and settings

* update async-handler comments

src/server/api/auth/get-token-utln.js

@@ -13,8 +13,7 @@ const getTokenUtln = async (utln: string) => {
   // If the token is invalid, it will get caught upstream in the `authorized`
   // middleware. If it is valid, the request should include a `user` property
   // including the user's utln, which we will return here.
-  return apiUtils.status(200).json({
-    status: codes.AUTHORIZED,
+  return apiUtils.status(codes.AUTHORIZED).data({
     utln,
   });
 };

src/server/api/auth/send-verification-email.js

@@ -55,8 +55,7 @@ const sendVerificationEmail = async (utln: string, forceResend: boolean) => {
     // respond that the email has already been sent
     if (forceResend !== true && !oldCodeExpired) {
       logger.info(`Already sent code: ${code.code}`);
-      return apiUtils.status(200).json({
-        status: codes.SEND_VERIFICATION_EMAIL__EMAIL_ALREADY_SENT,
+      return apiUtils.status(codes.SEND_VERIFICATION_EMAIL__EMAIL_ALREADY_SENT).data({
         email: code.email,
       });
     }
@@ -67,31 +66,25 @@ const sendVerificationEmail = async (utln: string, forceResend: boolean) => {
 
   //  If the member info is null (not found), error that it was not found.
   if (!memberInfo) {
-    return apiUtils.status(400).json({
-      status: codes.SEND_VERIFICATION_EMAIL__UTLN_NOT_FOUND,
-    });
+    return apiUtils.status(codes.SEND_VERIFICATION_EMAIL__UTLN_NOT_FOUND).noData();
   }
 
   // Ensure the member is a student
   if (!memberInfo.classYear) {
-    return apiUtils.status(400).json({
-      status: codes.SEND_VERIFICATION_EMAIL__UTLN_NOT_STUDENT,
-    });
+    return apiUtils.status(codes.SEND_VERIFICATION_EMAIL__UTLN_NOT_STUDENT).noData();
   }
 
   // Check that the student is in A&S or E
   if (!_.includes(['A&S', 'E'], memberInfo.college)) {
-    return apiUtils.status(400).json({
-      status: codes.SEND_VERIFICATION_EMAIL__UTLN_NOT_UNDERGRAD,
+    return apiUtils.status(codes.SEND_VERIFICATION_EMAIL__UTLN_NOT_UNDERGRAD).data({
       college: memberInfo.college,
       classYear: memberInfo.classYear,
     });
   }
 
   // Ensure user is in the Class of 2019
   if (memberInfo.classYear !== '19') {
-    return apiUtils.status(400).json({
-      status: codes.SEND_VERIFICATION_EMAIL__UTLN_NOT_2019,
+    return apiUtils.status(codes.SEND_VERIFICATION_EMAIL__UTLN_NOT_2019).data({
       classYear: memberInfo.classYear,
     });
   }
@@ -132,8 +125,7 @@ const sendVerificationEmail = async (utln: string, forceResend: boolean) => {
   slack.postVerificationCode(verificationCode, utln, memberInfo.email);
 
   // Send a success response to the client
-  return apiUtils.status(200).json({
-    status: codes.SEND_VERIFICATION_EMAIL__SUCCESS,
+  return apiUtils.status(codes.SEND_VERIFICATION_EMAIL__SUCCESS).data({
     email: memberInfo.email,
   });
 };

src/server/api/auth/verify.js

@@ -42,26 +42,20 @@ const verify = async (utln: string, code: number) => {
 
   // No email has been sent for this utln
   if (result.rowCount === 0) {
-    return apiUtils.status(401).json({
-      status: codes.VERIFY__NO_EMAIL_SENT,
-    });
+    return apiUtils.status(codes.VERIFY__NO_EMAIL_SENT).noData();
   }
 
   const verification = result.rows[0];
   const expired = new Date(verification.expiration).getTime() < new Date().getTime();
 
   // Check if the code is expired
   if (verification.attempts > 3 || expired) {
-    return apiUtils.status(400).json({
-      status: codes.VERIFY__EXPIRED_CODE,
-    });
+    return apiUtils.status(codes.VERIFY__EXPIRED_CODE).noData();
   }
 
   // Check if the code is valid. If not, send a bad code message
   if (verification.code !== code) {
-    return apiUtils.status(400).json({
-      status: codes.VERIFY__BAD_CODE,
-    });
+    return apiUtils.status(codes.VERIFY__BAD_CODE).noData();
   }
 
   // Success! The code is verified!
@@ -97,8 +91,7 @@ const verify = async (utln: string, code: number) => {
   });
 
   // Send the response back!
-  return apiUtils.status(200).json({
-    status: codes.VERIFY__SUCCESS,
+  return apiUtils.status(codes.VERIFY__SUCCESS).data({
     token,
   });
 };

src/server/api/photos/confirm-upload.js

@@ -29,9 +29,7 @@ const confirmUpload = async (userId: number) => {
   `, [userId]);
 
   if (unconfirmedPhotoRes.rowCount === 0) {
-    return apiUtils.status(400).json({
-      status: codes.CONFIRM_UPLOAD__NO_UNCONFIRMED_PHOTO,
-    });
+    return apiUtils.status(codes.CONFIRM_UPLOAD__NO_UNCONFIRMED_PHOTO).noData();
   }
 
   const [{ uuid }] = unconfirmedPhotoRes.rows;
@@ -47,9 +45,7 @@ const confirmUpload = async (userId: number) => {
   try {
     await s3.headObject(s3Params).promise();
   } catch (error) {
-    return apiUtils.status(400).json({
-      status: codes.CONFIRM_UPLOAD__NO_UPLOAD_FOUND,
-    });
+    return apiUtils.status(codes.CONFIRM_UPLOAD__NO_UPLOAD_FOUND).noData();
   }
 
   // TRANSACTION: Insert the photo and delete its "unconfirmed" counterpart
@@ -71,20 +67,33 @@ const confirmUpload = async (userId: number) => {
     // Ensure there are only 3 or fewer photos
     const [{ photoCount }] = photosRes.rows;
     if (photoCount > 3) {
-      return apiUtils.status(400).json({
-        status: codes.CONFIRM_UPLOAD__NO_AVAILABLE_SLOT,
-      });
+      return apiUtils.status(codes.CONFIRM_UPLOAD__NO_AVAILABLE_SLOT).noData();
     }
 
     // Insert the photo in the `photos` table, giving it the "next" index.
     const insertRes = await client.query(`
-      INSERT INTO photos
-      (user_id, index, uuid)
-      VALUES ($1, $2, $3)
-      RETURNING id
-    `, [userId, photoCount + 1, uuid]);
-
-    const photoId = insertRes.rows[0].id;
+      WITH inserted AS (
+        INSERT INTO photos
+        (user_id, index, uuid)
+        VALUES ($1, $2, $3)
+        RETURNING id
+      )
+      SELECT
+        array_cat(
+          ARRAY(
+            SELECT id
+            FROM photos
+            WHERE user_id = $4
+            ORDER BY index
+          ),
+          ARRAY(
+            SELECT id
+            FROM inserted
+          )
+        ) AS "photoIds"
+    `, [userId, photoCount + 1, uuid, userId]);
+
+    const [{ photoIds }] = insertRes.rows;
 
     // Delete the unconfirmed photo
     await client.query(`
@@ -96,10 +105,7 @@ const confirmUpload = async (userId: number) => {
     await client.query('COMMIT');
     client.release();
 
-    return apiUtils.status(200).json({
-      status: codes.CONFIRM_UPLOAD__SUCCESS,
-      photoId,
-    });
+    return apiUtils.status(codes.CONFIRM_UPLOAD__SUCCESS).data(photoIds);
   } catch (err) {
     // Rollback the transaction and release the client
     await client.query('ROLLBACK');

src/server/api/photos/delete-photo.js

@@ -20,7 +20,7 @@ const bucket = config.get('s3_bucket');
  * @api {delete} /api/photos/:photoId
  *
  */
-const deletePhoto = async (photoId: number, userId: number, userHasProfile: boolean) => {
+const deletePhoto = async (photoId: number, userId: number) => {
   // On error, return a server error.
   const photosRes = await db.query(`
     SELECT id, uuid, index
@@ -34,15 +34,7 @@ const deletePhoto = async (photoId: number, userId: number, userHasProfile: bool
   const photos = photosRes.rows;
   const [photoToDelete] = _.remove(photos, photo => photo.id === photoId);
   if (photoToDelete === undefined) {
-    return apiUtils.status(400).json({
-      status: codes.DELETE_PHOTO__NOT_FOUND,
-    });
-  }
-
-  if (photos.length === 0) {
-    return apiUtils.status(409).json({
-      status: codes.DELETE_PHOTO__CANNOT_DELETE_LAST_PHOTO,
-    });
+    return apiUtils.status(codes.DELETE_PHOTO__NOT_FOUND).noData();
   }
 
   // Transaction to delete the photo:
@@ -51,37 +43,30 @@ const deletePhoto = async (photoId: number, userId: number, userHasProfile: bool
     // 0. Begin the transaction
     await client.query('BEGIN');
 
-    // If we are deleting the splash photo, update the user's splash photo
-    // Only do this if the user already has a profile
-    if (userHasProfile && photoToDelete.index === 1) {
-      await client.query(`
-        UPDATE profiles
-        SET splash_photo_id = $1
-        WHERE user_id = $2
-      `, [photos[0].id, userId]);
-    }
-
     // 1. Remove the photo from our database
     await client.query(`
       DELETE FROM photos
-      WHERE id = $1
+      WHERE
+        id = $1
     `, [photoToDelete.id]);
 
     // Get an updated list of photos for the requesting user
     const updatedPhotos = _.map(photos, (photo, index) => {
       return `(${photo.id}, ${index + 1})`;
     });
 
-    // 2. Update the photos for the requesting user
-    await client.query(`
-      UPDATE photos
-      SET index = updated_photos.index
-      FROM
-        (VALUES
-          ${updatedPhotos.join(',')}
-        ) AS updated_photos (id, index)
-      WHERE photos.id = updated_photos.id
-    `);
+    // 2. Update the photos for the requesting user. Only do if some photos exist
+    if (photos.length > 0) {
+      await client.query(`
+        UPDATE photos
+        SET index = updated_photos.index
+        FROM
+          (VALUES
+            ${updatedPhotos.join(',')}
+          ) AS updated_photos (id, index)
+        WHERE photos.id = updated_photos.id
+      `);
+    }
 
     // 3. Delete the photo from S3
     const params = {
@@ -92,29 +77,21 @@ const deletePhoto = async (photoId: number, userId: number, userHasProfile: bool
 
     // 4. Commit the transaction!
     await client.query('COMMIT');
+
+    return apiUtils.status(codes.DELETE_PHOTO__SUCCESS).data(
+      _.map(photos, photo => photo.id),
+    );
   } catch (err) {
     await client.query('ROLLBACK');
     throw err;
   } finally {
     client.release();
   }
-
-  const newOrderRes = await db.query(`
-    SELECT id
-    FROM photos
-    WHERE user_id = $1
-    ORDER BY index
-  `, [userId]);
-
-  return apiUtils.status(200).json({
-    status: codes.DELETE_PHOTO__SUCCESS,
-    photos: _.map(newOrderRes.rows, row => row.id),
-  });
 };
 
 const handler = [
   apiUtils.asyncHandler(async (req: $Request) => {
-    return deletePhoto(Number.parseInt(req.params.photoId, 10), req.user.id, req.user.hasProfile);
+    return deletePhoto(Number.parseInt(req.params.photoId, 10), req.user.id);
   }),
 ];
 

src/server/api/photos/get-photo.js

@@ -15,7 +15,7 @@ const NODE_ENV = serverUtils.getNodeEnv();
 const s3 = new aws.S3({ region: 'us-east-1', signatureVersion: 'v4' });
 const bucket = config.get('s3_bucket');
 
-const getSignedUrl = async (params) => {
+const getSignedUrl = async (params): Promise<string> => {
   return new Promise((resolve, reject) => {
     s3.getSignedUrl('getObject', params, (err, url) => {
       if (err) return reject(err);
@@ -38,9 +38,11 @@ const getPhoto = async (photoId: number) => {
 
   // If it does not exist, error.
   if (photoRes.rowCount === 0) {
-    return apiUtils.status(400).json({
-      status: codes.GET_PHOTO__NOT_FOUND,
-    });
+    // Weird flowtype issue requires us to specifically define return type
+    // Same bug as https://github.com/facebook/flow/issues/5294. Not resolve.
+    // Should look into this more: Max made a trello ticket 2/4/19
+    // $FlowFixMe
+    return apiUtils.status(codes.GET_PHOTO__NOT_FOUND).noData();
   }
 
   // Sign a url for the photo and redirect the request to it
@@ -49,9 +51,9 @@ const getPhoto = async (photoId: number) => {
     Bucket: bucket,
     Key: `photos/${NODE_ENV}/${uuid}`,
   };
+
   const url = await getSignedUrl(params);
-  return apiUtils.status(200).json({
-    status: codes.GET_PHOTO__SUCCESS,
+  return apiUtils.status(codes.GET_PHOTO__SUCCESS).data({
     url,
   });
 };
@@ -62,10 +64,15 @@ const handler = [
   async (req: $Request, res: $Response, next: $Next) => {
     try {
       const photoRes = await getPhoto(req.params.photoId);
-      if (photoRes.body.status === codes.GET_PHOTO__SUCCESS) {
-        return res.redirect(photoRes.body.url);
+
+      // If the photo was succesfully retrieved, redirect to it!
+      // NOTE: This is "abnormal" by the standards of the API
+      if (photoRes.body.status === codes.GET_PHOTO__SUCCESS.status) {
+        return res.redirect(photoRes.body.data.url);
       }
-      return res.status(photoRes.status).json(photoRes.body);
+
+      // On failure, respond 'normally'
+      return res.status(photoRes.statusCode).json(photoRes.body);
     } catch (err) {
       return next(err);
     }

src/server/api/photos/reorder-photos.js

@@ -25,7 +25,7 @@ const schema = {
  * @api {patch} /api/photos/reorder
  *
  */
-const reorderPhotos = async (newOrder: number[], userId: number, userHasProfile: boolean) => {
+const reorderPhotos = async (newOrder: number[], userId: number) => {
   // No worry about SQL Injection here: newOrder is verified to be an
   // array of integers/numbers.
   const result = await db.query(`
@@ -41,9 +41,7 @@ const reorderPhotos = async (newOrder: number[], userId: number, userHasProfile:
 
   // If there are photo id mismatches, error
   if (mismatchCount > 0) {
-    return apiUtils.status(400).json({
-      status: codes.REORDER_PHOTOS__MISMATCHED_IDS,
-    });
+    return apiUtils.status(codes.REORDER_PHOTOS__MISMATCHED_IDS).noData();
   }
 
   // Get an updated list of photos for the requesting user
@@ -62,25 +60,13 @@ const reorderPhotos = async (newOrder: number[], userId: number, userHasProfile:
     WHERE photos.id = updated_photos.id
   `);
 
-  // If the user has a profile, set the new first photo to be the splash photo
-  if (userHasProfile) {
-    await db.query(`
-      UPDATE profiles
-      SET splash_photo_id = $1
-      WHERE user_id = $2
-    `, [newOrder[0], userId]);
-  }
-
-  return apiUtils.status(200).json({
-    status: codes.REORDER_PHOTOS__SUCCESS,
-    photos: newOrder,
-  });
+  return apiUtils.status(codes.REORDER_PHOTOS__SUCCESS).data(newOrder);
 };
 
 const handler = [
   apiUtils.validate(schema),
   apiUtils.asyncHandler(async (req: $Request) => {
-    return reorderPhotos(req.body, req.user.id, req.user.hasProfile);
+    return reorderPhotos(req.body, req.user.id);
   }),
 ];