Merge pull request #449 from 52cs/issue-448

issues/448

Author: jmgasper

src/common/db-helper.js

@@ -179,6 +179,43 @@ async function queryOneIssue(model, repositoryId, number, provider) {
   });
 }
 
+/**
+ * Get Issue's id and challengeUUID by repoUrl
+ * @param {String} repoUrl The repo url
+ * @returns {Promise<Object>}
+ */
+async function queryIssueIdChallengeUUIDByRepoUrl(repoUrl) {
+  return await new Promise((resolve, reject) => {
+    models.Issue.scan('repoUrl').eq(repoUrl)
+      .attributes(['id', 'challengeUUID'])
+      .exec((err, result) => {
+        if (err) {
+          return reject(err);
+        }
+        return resolve(result);
+      });
+  });
+}
+
+
+/**
+ * Get CopilotPayment's id by challengeUUID
+ * @param {String} challengeUUID The challengeUUID
+ * @returns {Promise<String>}
+ */
+async function queryPaymentIdByChallengeUUID(challengeUUID) {
+  return await new Promise((resolve, reject) => {
+    models.CopilotPayment.scan('challengeUUID').eq(challengeUUID)
+      .attributes(['id'])
+      .exec((err, result) => {
+        if (err) {
+          return reject(err);
+        }
+        return resolve(result.id);
+      });
+  });
+}
+
 /**
  * Get single data by query parameters
  * @param {Object} model The dynamoose model to query
@@ -257,7 +294,7 @@ async function queryOneUserMappingByTCUsername(model, tcusername) {
 async function queryOneActiveProject(model, repoUrl) {
   return await new Promise((resolve, reject) => {
     queryOneActiveRepository(models.Repository, repoUrl).then((repo) => {
-      if (!repo) resolve(null);
+      if (!repo || repo.length === 0) resolve(null);
       else model.queryOne('id').eq(repo.projectId).consistent()
         .exec((err, result) => {
           if (err) {
@@ -509,8 +546,8 @@ async function queryOneActiveRepository(model, url) {
   return await new Promise((resolve, reject) => {
     model.queryOne({
       url,
-      archived: 'false'
     })
+    .filter('archived').eq('false')
     .all()
     .exec((err, result) => {
       if (err) {
@@ -531,8 +568,8 @@ async function queryActiveRepositoriesExcludeByProjectId(url, projectId) {
   return await new Promise((resolve, reject) => {
     models.Repository.query({
       url,
-      archived: 'false'
     })
+    .filter('archived').eq('false')
     .filter('projectId')
     .not().eq(projectId)
     .all()
@@ -609,6 +646,8 @@ async function populateRepoUrls(projectId) {
 }
 
 module.exports = {
+  queryIssueIdChallengeUUIDByRepoUrl,
+  queryPaymentIdByChallengeUUID,
   getById,
   getByKey,
   scan,

src/services/CopilotPaymentService.js

@@ -86,7 +86,7 @@ async function _ensureEditPermissionAndGetInfo(paymentId, topcoderUser) {
   if (dbPayment.closed === true) {
     throw new Error('Closed payment can not be updated');
   }
-  if (dbProject.archived) {
+  if (dbProject.archived === 'true') {
     throw new errors.ForbiddenError('You can\'t edit this payment in an archived project');
   }
   return dbPayment;
@@ -206,7 +206,7 @@ async function create(topcoderUser, payment) {
   if (dbProject.copilot !== topcoderUser.handle && dbProject.owner !== topcoderUser.handle) {
     throw new errors.ForbiddenError('You do not have permission to edit this payment');
   }
-  if (dbProject.archived) {
+  if (dbProject.archived === 'true') {
     throw new errors.ForbiddenError('You can\'t edit this payment in an archived project');
   }
   payment.username = dbProject.copilot;

src/services/IssueService.js

@@ -117,7 +117,7 @@ async function _ensureEditPermissionAndGetInfo(projectId, currentUser) {
   ) {
     throw new errors.ForbiddenError('You don\'t have access on this project');
   }
-  if (dbProject.archived) {
+  if (dbProject.archived === 'true') {
     throw new errors.ForbiddenError('You can\'t access on this archived project');
   }
   return dbProject;

src/services/ProjectService.js

@@ -112,7 +112,7 @@ async function _ensureEditPermissionAndGetInfo(projectId, currentUser) {
   ) {
     throw new errors.ForbiddenError('You don\'t have access on this project');
   }
-  if (dbProject.archived) {
+  if (dbProject.archived === 'true') {
     throw new errors.ForbiddenError('You can\'t access on this archived project');
   }
   return dbProject;
@@ -140,33 +140,21 @@ async function _createOrMigrateRepository(repoUrl, project, currentUser) {
         or a time-sequence cornercase encountered here`);
     }
     try {
-      let oldIssues = await models.Issue.query({repoUrl: oldRepo.url});
-      let oldCopilotPaymentPromise = oldIssues.filter(issue => issue.challengeUUID)
-        .map(issue => models.CopilotPayment.query({challengeUUID: issue.challengeUUID})
-          .then(payments => {
-            if (!payments || payments.length === 0) {
-              /* eslint-disable-next-line no-console */
-              console.log(`No CopilotPayment correspond to Issue with challengeUUID ${issue.challengeUUID}.
-                           The corresponding CopilotPayment may have been removed.
-                           Or, there is bug in old version.`);
-              return null;
-            }
-            if (payments.length > 1) {
-              throw new Error(`Duplicate CopilotPayment correspond to one Issue with challengeUUID ${issue.challengeUUID}.
-                               There must be bug in old version`);
-            }
-            return payments[0];
-          }));
-      let oldCopilotPayment = await Promise.all(oldCopilotPaymentPromise).filter(payment => payment);
-
-      await models.Repository.update({id: oldRepo.id}, {projectId: project.id, archived: false});
-      await oldIssues.forEach(issue => models.Issue.update({id: issue.id}, {projectId: project.id}));
-      await oldCopilotPayment.forEach(
-        payment => models.CopilotPayment.update({id: payment.id}, {project: project.id})
+      const oldIssues = await dbHelper.queryIssueIdChallengeUUIDByRepoUrl(repoUrl);
+      const issueIds = oldIssues.map(issue => issue.id);
+      const challengeUUIDs = oldIssues.map(issue => issue.challengeUUID).filter(challengeUUID => challengeUUID);
+      const paymentIds = await Promise.all(
+        challengeUUIDs.map(challengeUUID => dbHelper.queryPaymentIdByChallengeUUID(challengeUUID))
+      );
+
+      await dbHelper.update(models.Repository, oldRepo.id, {projectId: project.id, archived: false});
+      await Promise.all(issueIds.map(issueId => dbHelper.update(models.Issue, issueId, {projectId: project.id})));
+      await Promise.all(
+        paymentIds.map(paymentId => dbHelper.update(models.CopilotPayment, paymentId, {project: project.id}))
       );
     }
     catch (err) {
-      throw new Error(`Update ProjectId for Repository, Issue, CopilotPayment failed. Repo ${repoUrl}. Internal Error: ${err.message}`);
+      throw new Error(`Update ProjectId for Repository, Issue, CopilotPayment failed. Repo ${repoUrl}. Internal Error: ${err}`);
     }
   } else {
     try {
@@ -181,7 +169,7 @@ async function _createOrMigrateRepository(repoUrl, project, currentUser) {
       await addWikiRules({projectId: project.id}, currentUser, repoUrl);
     }
     catch (err) {
-      throw new Error(`Project created. Adding the webhook, issue labels, and wiki rules failed. Repo ${repoUrl}. Internal Error: ${err.message}`);
+      throw new Error(`Project created. Adding the webhook, issue labels, and wiki rules failed. Repo ${repoUrl}. Internal Error: ${err}`);
     }
   }
 }
@@ -213,6 +201,8 @@ async function create(project, currentUser) {
   project.copilot = project.copilot ? project.copilot.toLowerCase() : null;
   project.id = helper.generateIdentifier();
 
+  const createdProject = await dbHelper.create(models.Project, project);
+
   // TODO: The following db operation should/could be moved into one transaction
   for (const repoUrl of repoUrls) { // eslint-disable-line no-restricted-syntax
     try {
@@ -222,7 +212,6 @@ async function create(project, currentUser) {
       throw new Error(`Create or migrate repository failed. Repo ${repoUrl}. Internal Error: ${err.message}`);
     }
   }
-  const createdProject = await dbHelper.create(models.Project, project);
 
   return createdProject;
 }
@@ -267,12 +256,12 @@ async function update(project, currentUser) {
   });
 
   // TODO: move the following logic into one dynamoose transaction
-  const repoUrl2Repo = await dbHelper.queryRepositoriesByProjectId(dbProject.id)
-    .map(repo => { return {[repo.url]: repo}; });
+  const repos = await dbHelper.queryRepositoriesByProjectId(dbProject.id);
 
   for (const repoUrl of repoUrls) { // eslint-disable-line no-restricted-syntax
-    if (repoUrl in repoUrl2Repo) {
-      await models.Repository.update({id: repoUrl2Repo[repoUrl].id}, {archived: project.archived});
+    if (repos.find(repo => repo.url === repoUrl)) {
+      const repoId = repos.find(repo => repo.url === repoUrl).id
+      await dbHelper.update(models.Repository, repoId, {archived: project.archived});
     } else {
       try {
         await _createOrMigrateRepository(repoUrl, project, currentUser);
@@ -319,7 +308,6 @@ async function getAll(query, currentUser) {
       query.lastKey = parseInt(query.lastKey, 10);
     }
     const slicedProjects = _.slice(projects, query.lastKey, query.lastKey + query.perPage);
-    // console.log(projects);
     for (const project of slicedProjects) { // eslint-disable-line
       project.repoUrls = await dbHelper.populateRepoUrls(project.id);
     }