HOTFIX - better handling of large uploads

.circleci/config.yml

@@ -149,7 +149,7 @@ workflows:
           context : org-global
           filters:
             branches:
-              only: ['develop', 'migration-setup', 'pm-1611_1', 'pm-1836_prod']
+              only: ['develop', 'migration-setup', 'PM-1612', 'fix-project-exposing']
       - deployProd:
           context : org-global
           filters:

src/routes/attachments/create.js

@@ -39,7 +39,7 @@ module.exports = [
    * Add project attachment
    * In development mode we have to mock the ec2 file transfer and file service calls
    */
-  (req, res, next) => {
+  async (req, res, next) => {
     const data = req.body;
     // default values
     const projectId = req.params.projectId;
@@ -53,64 +53,22 @@ module.exports = [
     // extract file name
     const fileName = Path.parse(data.path).base;
     // create file path
-    const path = _.join([
+    const attachmentPath = _.join([
       config.get('projectAttachmentPathPrefix'),
       data.projectId,
       config.get('projectAttachmentPathPrefix'),
       fileName,
     ], '/');
-    let newAttachment = null;
 
     const sourceBucket = data.s3Bucket;
     const sourceKey = data.path;
     const destBucket = config.get('attachmentsS3Bucket');
-    const destKey = path;
+    const destKey = attachmentPath;
 
-    if (data.type === ATTACHMENT_TYPES.LINK) {
-      // We create the record in the db and return (i.e. no need to handle transferring file between S3 buckets)
-      Promise.resolve(models.ProjectAttachment.create({
-        projectId,
-        allowedUsers,
-        createdBy: req.authUser.userId,
-        updatedBy: req.authUser.userId,
-        title: data.title,
-        size: data.size,
-        category: data.category || null,
-        description: data.description,
-        contentType: data.contentType,
-        path: data.path,
-        type: data.type,
-        tags: data.tags,
-      })).then((_link) => {
-        const link = _link.get({ plain: true });
-        req.log.debug('New Link Attachment record: ', link);
-
-        // emit the Kafka event
-        util.sendResourceToKafkaBus(
-          req,
-          EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_ADDED,
-          RESOURCES.ATTACHMENT,
-          link);
-
-        res.status(201).json(link);
-        return Promise.resolve();
-      })
-        .catch((error) => {
-          req.log.error('Error adding link attachment', error);
-          const rerr = error;
-          rerr.status = rerr.status || 500;
-          next(rerr);
-        });
-    } else {
-    // don't actually transfer file in development mode if file uploading is disabled, so we can test this endpoint
-      const fileTransferPromise = (process.env.NODE_ENV !== 'development' || config.get('enableFileUpload') === 'true')
-        ? util.s3FileTransfer(req, sourceBucket, sourceKey, destBucket, destKey)
-        : Promise.resolve();
-
-      fileTransferPromise.then(() => {
-        // file copied to final destination, create DB record
-        req.log.debug('creating db file record');
-        return models.ProjectAttachment.create({
+    try {
+      if (data.type === ATTACHMENT_TYPES.LINK) {
+        // Create the record and return immediately (no file transfer needed)
+        const linkInstance = await models.ProjectAttachment.create({
           projectId,
           allowedUsers,
           createdBy: req.authUser.userId,
@@ -120,60 +78,84 @@ module.exports = [
           category: data.category || null,
           description: data.description,
           contentType: data.contentType,
-          path,
+          path: data.path,
           type: data.type,
           tags: data.tags,
         });
-      }).then((_newAttachment) => {
-        newAttachment = _newAttachment.get({ plain: true });
-        req.log.debug('New Attachment record: ', newAttachment);
-        if (process.env.NODE_ENV !== 'development' || config.get('enableFileUpload') === 'true') {
-          // retrieve download url for the response
-          req.log.debug('retrieving download url');
-          return getDownloadUrl(destBucket, path);
-        }
-        return Promise.resolve();
-      }).then((url) => {
-        if (
-          process.env.NODE_ENV !== 'development' ||
-          config.get('enableFileUpload') === 'true'
-        ) {
-          req.log.debug('Retreiving Presigned Url resp: ', url);
-          let response = _.cloneDeep(newAttachment);
-          response = _.omit(response, ['path', 'deletedAt']);
-
-          response.downloadUrl = url;
-
-          // emit the event
-          util.sendResourceToKafkaBus(
-            req,
-            EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_ADDED,
-            RESOURCES.ATTACHMENT,
-            newAttachment,
-          );
-          res.status(201).json(response);
-          return Promise.resolve();
-        }
-        let response = _.cloneDeep(newAttachment);
-        response = _.omit(response, ['path', 'deletedAt']);
-        // only in development mode
-        response.downloadUrl = path;
-        // emit the event
+        const link = linkInstance.get({ plain: true });
+        req.log.debug('New Link Attachment record: ', link);
+
         util.sendResourceToKafkaBus(
           req,
           EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_ADDED,
           RESOURCES.ATTACHMENT,
-          newAttachment);
-
-        res.status(201).json(response);
-        return Promise.resolve();
-      })
-        .catch((error) => {
-          req.log.error('Error adding file attachment', error);
-          const rerr = error;
-          rerr.status = rerr.status || 500;
-          next(rerr);
-        });
+          link,
+        );
+
+        res.status(201).json(link);
+        return;
+      }
+
+      const shouldTransfer = process.env.NODE_ENV !== 'development' || config.get('enableFileUpload') === 'true';
+      const downloadUrlPromise = shouldTransfer
+        ? getDownloadUrl(destBucket, destKey)
+        : Promise.resolve(destKey);
+
+      req.log.debug('creating db file record');
+      const attachmentInstance = await models.ProjectAttachment.create({
+        projectId,
+        allowedUsers,
+        createdBy: req.authUser.userId,
+        updatedBy: req.authUser.userId,
+        title: data.title,
+        size: data.size,
+        category: data.category || null,
+        description: data.description,
+        contentType: data.contentType,
+        path: destKey,
+        type: data.type,
+        tags: data.tags,
+      });
+
+      const newAttachment = attachmentInstance.get({ plain: true });
+      req.log.debug('New Attachment record: ', newAttachment);
+
+      const downloadUrl = await downloadUrlPromise;
+      req.log.debug('Retrieved presigned url for new attachment');
+
+      let response = _.cloneDeep(newAttachment);
+      response = _.omit(response, ['path', 'deletedAt']);
+      response.downloadUrl = downloadUrl;
+
+      util.sendResourceToKafkaBus(
+        req,
+        EVENT.ROUTING_KEY.PROJECT_ATTACHMENT_ADDED,
+        RESOURCES.ATTACHMENT,
+        newAttachment,
+      );
+
+      res.status(201).json(response);
+
+      if (shouldTransfer) {
+        util.s3FileTransfer(req, sourceBucket, sourceKey, destBucket, destKey)
+          .then(() => {
+            req.log.debug('File attachment copied asynchronously', { attachmentId: newAttachment.id });
+          })
+          .catch((error) => {
+            req.log.error('Async S3 file transfer failed', {
+              error: error.message,
+              stack: error.stack,
+              attachmentId: newAttachment.id,
+              source: `${sourceBucket}/${sourceKey}`,
+              destination: `${destBucket}/${destKey}`,
+            });
+          });
+      }
+    } catch (error) {
+      req.log.error('Error adding attachment', error);
+      const rerr = error;
+      rerr.status = rerr.status || 500;
+      next(rerr);
     }
   },
 ];

src/routes/copilotOpportunity/get.js

@@ -10,6 +10,7 @@ module.exports = [
     }
 
     const isAdminOrManager = util.hasRoles(req, [USER_ROLE.CONNECT_ADMIN, USER_ROLE.TOPCODER_ADMIN, USER_ROLE.PROJECT_MANAGER]);
+
     return models.CopilotOpportunity.findOne({
       where: { id },
       include: isAdminOrManager ? [
@@ -43,6 +44,7 @@ module.exports = [
         if (req.authUser) {
           canApplyAsCopilot = !memberIds.includes(req.authUser.userId)
         }
+
         if (plainOpportunity.project) {
           // This shouldn't be exposed to the clientside
           delete plainOpportunity.project.members;

src/routes/copilotOpportunity/list.js

@@ -43,7 +43,7 @@ module.exports = [
     baseOrder.push([sortParams[0], sortParams[1]]);
 
     return models.CopilotOpportunity.findAll({
-      include: isAdminOrManager ? [
+      include: isAdminOrManager ?[
         {
           model: models.CopilotRequest,
           as: 'copilotRequest',
@@ -57,7 +57,7 @@ module.exports = [
         {
           model: models.CopilotRequest,
           as: 'copilotRequest',
-        },
+        }
       ],
       order: baseOrder,
       limit,

src/routes/copilotOpportunityApply/list.js

@@ -1,17 +1,12 @@
 import _ from 'lodash';
-import { middleware as tcMiddleware } from 'tc-core-library-js';
 
 import models from '../../models';
 import { ADMIN_ROLES } from '../../constants';
 import util from '../../util';
 
-const permissions = tcMiddleware.permissions;
-
 module.exports = [
-  permissions('copilotApplications.view'),
   (req, res, next) => {
-    const canAccessAllApplications = util.hasRoles(req, ADMIN_ROLES) || util.hasProjectManagerRole(req);
-    const userId = req.authUser.userId;
+    const isAdminOrPM = util.hasRoles(req, ADMIN_ROLES) || util.hasProjectManagerRole(req);
     const opportunityId = _.parseInt(req.params.id);
 
     let sort = req.query.sort ? decodeURIComponent(req.query.sort) : 'createdAt desc';
@@ -24,17 +19,15 @@ module.exports = [
     }
     const sortParams = sort.split(' ');
 
-    // Admin can see all requests and the PM can only see requests created by them
     const whereCondition = _.assign({
       opportunityId,
     },
-    canAccessAllApplications ? {} : { createdBy: userId },
     );
 
     return models.CopilotOpportunity.findOne({
       where: {
         id: opportunityId,
-      }
+      },
     }).then((opportunity) => {
       if (!opportunity) {
         const err = new Error('No opportunity found');
@@ -51,13 +44,13 @@ module.exports = [
         ],
         order: [[sortParams[0], sortParams[1]]],
       })
-        .then(copilotApplications => {
+        .then((copilotApplications) => {
           req.log.debug(`CopilotApplications ${JSON.stringify(copilotApplications)}`);
           return models.ProjectMember.getActiveProjectMembers(opportunity.projectId).then((members) => {
             req.log.debug(`Fetched existing active members ${JSON.stringify(members)}`);
             req.log.debug(`Applications ${JSON.stringify(copilotApplications)}`);
-            const enrichedApplications = copilotApplications.map(application => {
-              const m = members.find(m => m.userId === application.userId);
+            const enrichedApplications = copilotApplications.map((application) => {
+              const member = members.find(memberItem => memberItem.userId === application.userId);
 
               // Using spread operator fails in lint check
               // While Object.assign fails silently during run time
@@ -77,22 +70,30 @@ module.exports = [
                 copilotOpportunity: application.copilotOpportunity,
               };
 
-              if (m) {
-                enriched.existingMembership = m;
+              if (member) {
+                enriched.existingMembership = member;
               }
 
               req.log.debug(`Existing member to application ${JSON.stringify(enriched)}`);
 
               return enriched;
             });
 
+            const response = isAdminOrPM
+              ? enrichedApplications
+              : enrichedApplications.map(app => ({
+                userId: app.userId,
+                status: app.status,
+                createdAt: app.createdAt,
+              }));
+
             req.log.debug(`Enriched Applications ${JSON.stringify(enrichedApplications)}`);
-            res.status(200).send(enrichedApplications);
+            res.status(200).send(response);
           });
-        })
+        });
     })
-    .catch((err) => {
-      util.handleError('Error fetching copilot applications', err, req, next);
-    });
+      .catch((err) => {
+        util.handleError('Error fetching copilot applications', err, req, next);
+      });
   },
 ];