Skip to content

feat(PM-1168): Added API to assign an opportunity with an applicant #805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
May 15, 2025
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ workflows:
context : org-global
filters:
branches:
only: ['develop', 'migration-setup']
only: ['develop', 'migration-setup', 'pm-1168']
- deployProd:
context : org-global
filters:
Expand Down
54 changes: 53 additions & 1 deletion docs/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,37 @@ paths:
description: "Internal Server Error"
schema:
$ref: "#/definitions/ErrorModel"
"/projects/copilots/opportunity/{copilotOpportunityId}/assign":
post:
tags:
- assign project copilot opportunity
operationId: assignCopilotOpportunity
security:
- Bearer: []
description: "Assign a copilot opportunity with copilot."
parameters:
- $ref: "#/parameters/copilotOpportunityIdParam"
- in: body
name: body
schema:
$ref: "#/definitions/AssignCopilotOpportunity"
responses:
"200":
description: "The response after assigning an copilot opportunity"
schema:
$ref: "#/definitions/CopilotOpportunityAssignResponse"
"401":
description: "Unauthorized"
schema:
$ref: "#/definitions/ErrorModel"
"403":
description: "Forbidden - User does not have permission"
schema:
$ref: "#/definitions/ErrorModel"
"500":
description: "Internal Server Error"
schema:
$ref: "#/definitions/ErrorModel"
"/projects/{projectId}/attachments":
get:
tags:
Expand Down Expand Up @@ -6081,6 +6112,13 @@ definitions:
notes:
description: notes regarding the application
type: string
status:
description: status of the application
type: string
enum:
- pending
- accepted
example: pending
opportunityId:
description: copilot request id
type: integer
Expand Down Expand Up @@ -6111,6 +6149,13 @@ definitions:
format: int64
description: READ-ONLY. User that deleted this task
readOnly: true
CopilotOpportunityAssignResponse:
type: object
properties:
id:
description: unique identifier
type: integer
format: int64
Project:
type: object
properties:
Expand Down Expand Up @@ -6321,12 +6366,19 @@ definitions:
- manager
- copilot
ApplyCopilotOpportunity:
title: Apply copilot CopilotOpportunity
title: Apply Copilot Opportunity
type: object
properties:
notes:
type: string
description: notes about applying copilot opportunity
AssignCopilotOpportunity:
title: Assign Copilot Opportunity
type: object
properties:
applicationId:
type: string
description: The ID of the application to be accepted for the copilot opportunity.
NewProjectAttachment:
title: Project attachment request
type: object
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
'use strict';


module.exports = {
up: async (queryInterface, Sequelize) => {
Expand Down Expand Up @@ -56,5 +56,5 @@ module.exports = {

down: async (queryInterface) => {
await queryInterface.dropTable('copilot_applications');
}
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module.exports = {
up: async (queryInterface, Sequelize) => {
await queryInterface.addColumn('copilot_applications', 'status', {
type: Sequelize.STRING(16),
allowNull: true,
});

await queryInterface.sequelize.query(
'UPDATE copilot_applications SET status = \'pending\' WHERE status IS NULL',
);

await queryInterface.changeColumn('copilot_applications', 'status', {
type: Sequelize.STRING(16),
allowNull: false,
});
},

down: async (queryInterface) => {
await queryInterface.removeColumn('copilot_applications', 'status');
},
};
5 changes: 5 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ export const COPILOT_REQUEST_STATUS = {
FULFILLED: 'fulfiled',
};

export const COPILOT_APPLICATION_STATUS = {
PENDING: 'pending',
ACCEPTED: 'accepted',
};

export const COPILOT_OPPORTUNITY_STATUS = {
ACTIVE: 'active',
COMPLETED: 'completed',
Expand Down
15 changes: 12 additions & 3 deletions src/models/copilotApplication.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import _ from 'lodash';
import { COPILOT_APPLICATION_STATUS } from '../constants';

module.exports = function defineCopilotOpportunity(sequelize, DataTypes) {
const CopilotApplication = sequelize.define('CopilotApplication', {
Expand All @@ -8,14 +9,22 @@ module.exports = function defineCopilotOpportunity(sequelize, DataTypes) {
allowNull: false,
references: {
model: 'copilot_opportunities',
key: 'id'
key: 'id',
},
onUpdate: 'CASCADE',
onDelete: 'CASCADE'
onDelete: 'CASCADE',
},
notes: {
type: DataTypes.TEXT,
allowNull: true
allowNull: true,
},
status: {
type: DataTypes.STRING(16),
defaultValue: 'pending',
validate: {
isIn: [_.values(COPILOT_APPLICATION_STATUS)],
},
allowNull: false,
},
userId: { type: DataTypes.BIGINT, allowNull: false },
deletedAt: { type: DataTypes.DATE, allowNull: true },
Expand Down
12 changes: 12 additions & 0 deletions src/permissions/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,18 @@ export const PERMISSION = { // eslint-disable-line import/prefer-default-export
],
scopes: SCOPES_PROJECTS_WRITE,
},
ASSIGN_COPILOT_OPPORTUNITY: {
meta: {
title: 'Assign copilot to opportunity',
group: 'Assign Copilot',
description: 'Who can assign for copilot opportunity.',
},
topcoderRoles: [
USER_ROLE.PROJECT_MANAGER,
USER_ROLE.TOPCODER_ADMIN,
],
scopes: SCOPES_PROJECTS_WRITE,
},

LIST_COPILOT_OPPORTUNITY: {
meta: {
Expand Down
94 changes: 94 additions & 0 deletions src/routes/copilotOpportunity/assign.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import _ from 'lodash';
import validate from 'express-validation';
import Joi from 'joi';

import models from '../../models';
import util from '../../util';
import { PERMISSION } from '../../permissions/constants';
import { COPILOT_APPLICATION_STATUS, COPILOT_OPPORTUNITY_STATUS, COPILOT_REQUEST_STATUS } from '../../constants';

const assignCopilotOpportunityValidations = {
body: Joi.object().keys({
applicationId: Joi.string(),
}),
};

module.exports = [
validate(assignCopilotOpportunityValidations),
async (req, res, next) => {
const { applicationId } = req.body;
const copilotOpportunityId = _.parseInt(req.params.id);
if (!util.hasPermissionByReq(PERMISSION.ASSIGN_COPILOT_OPPORTUNITY, req)) {
const err = new Error('Unable to assign copilot opportunity');
_.assign(err, {
details: JSON.stringify({ message: 'You do not have permission to assign a copilot opportunity' }),
status: 403,
});
return next(err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is next a promise? Returning a callback function won't revert the transaction, what do you think?

}

return models.sequelize.transaction(async (t) => {
const opportunity = await models.CopilotOpportunity.findOne({
where: { id: copilotOpportunityId },

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding error handling for the database transaction itself to ensure that any issues during the transaction are properly caught and handled.

transaction: t,
});

if (!opportunity) {
const err = new Error('No opportunity found');
err.status = 404;
throw err;
}

if (opportunity.status !== COPILOT_OPPORTUNITY_STATUS.ACTIVE) {
const err = new Error('Opportunity is not active');
err.status = 400;
throw err;
}

const application = await models.CopilotApplication.findOne({
where: { id: applicationId },

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The transaction: t option is correctly added to the findOne method, but ensure that all database operations within the transaction are consistently using this option to maintain atomicity.

transaction: t,
});

if (!application) {
const err = new Error('No such application available');
err.status = 400;
throw err;
}

if (application.status === COPILOT_APPLICATION_STATUS.ACCEPTED) {
const err = new Error('Application already accepted');
err.status = 400;
throw err;
}

const projectId = opportunity.projectId;
const userId = application.userId;
const activeMembers = await models.ProjectMember.getActiveProjectMembers(projectId);

const existingUser = activeMembers.find(item => item.userId === userId);
if (existingUser && existingUser.role === 'copilot') {
const err = new Error(`User is already a copilot of this project`);
err.status = 400;
throw err;
}

await models.CopilotRequest.update(
{ status: COPILOT_REQUEST_STATUS.FULFILLED },

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure that the transaction: t option is used consistently across all update operations to maintain atomicity within the transaction.

{ where: { id: opportunity.copilotRequestId }, transaction: t },
);

await opportunity.update(
{ status: COPILOT_OPPORTUNITY_STATUS.COMPLETED },
{ transaction: t },
);

await models.CopilotApplication.update(
{ status: COPILOT_APPLICATION_STATUS.ACCEPTED },
{ where: { id: applicationId }, transaction: t },
);

res.status(200).send({ id: applicationId });

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a final catch block after the transaction to handle any errors that might occur during the transaction and ensure a proper rollback if necessary.

}).catch(err => next(err));
},
];
8 changes: 4 additions & 4 deletions src/routes/copilotOpportunityApply/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ module.exports = [
res.status(200).json(existingApplication);
return Promise.resolve();
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove unnecessary whitespace change to keep the code clean and consistent.

return models.CopilotApplication.create(data)
.then((result) => {
res.status(201).json(result);
return Promise.resolve();
})
.catch((err) => {
util.handleError('Error creating copilot application', err, req, next);
return next(err);
});
util.handleError('Error creating copilot application', err, req, next);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure consistent indentation for the .catch block to improve readability.

return next(err);
});
}).catch((e) => {
util.handleError('Error applying for copilot opportunity', e, req, next);
});
Expand Down
3 changes: 1 addition & 2 deletions src/routes/copilotOpportunityApply/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ 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 opportunityId = _.parseInt(req.params.id);
Expand All @@ -29,7 +28,7 @@ module.exports = [
const whereCondition = _.assign({
opportunityId,
},
canAccessAllApplications ? {} : { createdBy: userId },
canAccessAllApplications ? {} : { createdBy: userId },
);

return models.CopilotApplication.findAll({
Expand Down
4 changes: 4 additions & 0 deletions src/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@ router.route('/v5/projects/copilots/opportunity/:id(\\d+)/apply')
router.route('/v5/projects/copilots/opportunity/:id(\\d+)/applications')
.get(require('./copilotOpportunityApply/list'));

// Copilot opportunity assign
router.route('/v5/projects/copilots/opportunity/:id(\\d+)/assign')
.post(require('./copilotOpportunity/assign'));

// Project Estimation Items
router.route('/v5/projects/:projectId(\\d+)/estimations/:estimationId(\\d+)/items')
.get(require('./projectEstimationItems/list'));
Expand Down
13 changes: 6 additions & 7 deletions src/routes/projectMemberInvites/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ const buildCreateInvitePromises = (req, inviteEmails, inviteUserIds, invites, da
};

const sendInviteEmail = (req, projectId, invite) => {
req.log.debug(`Sending invite email: ${JSON.stringify(req.body)}, ${projectId}, ${JSON.stringify(invite)}`)
req.log.debug(`Sending invite email: ${JSON.stringify(req.body)}, ${projectId}, ${JSON.stringify(invite)}`);
req.log.debug(req.authUser);
const emailEventType = CONNECT_NOTIFICATION_EVENT.PROJECT_MEMBER_EMAIL_INVITE_CREATED;
const promises = [
Expand Down Expand Up @@ -295,13 +295,12 @@ module.exports = [
// whom we are inviting, because Member Service has a loose search logic and may return
// users with handles whom we didn't search for
.then((foundUsers) => {
if(invite.handles) {
if (invite.handles) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a space between if and the opening parenthesis for consistency with the rest of the code style.

const lowerCaseHandles = invite.handles.map(handle => handle.toLowerCase());
return foundUsers.filter(foundUser => _.includes(lowerCaseHandles, foundUser.handleLower));
}
else {
return []
}

return [];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure consistent use of semicolons. The semicolon at the end of the return []; statement is correct, but make sure this style is consistent throughout the codebase.

})
.then((inviteUsers) => {
const members = req.context.currentProjectMembers;
Expand Down Expand Up @@ -414,7 +413,7 @@ module.exports = [
RESOURCES.PROJECT_MEMBER_INVITE,
v.toJSON());

req.log.debug(`V: ${JSON.stringify(v)}`)
req.log.debug(`V: ${JSON.stringify(v)}`);
// send email invite (async)
if (v.email && !v.userId && v.status === INVITE_STATUS.PENDING) {
sendInviteEmail(req, projectId, v);
Expand Down Expand Up @@ -443,7 +442,7 @@ module.exports = [
}
});
}).catch((err) => {
console.log(err)
console.log(err);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using console.log for error handling is not recommended in production code. Consider using a proper logging mechanism or error handling strategy.

if (failed.length) {
res.status(403).json(_.assign({}, { success: [] }, { failed }));
} else next(err);
Expand Down