From f1de946516ad1f69fef77a6e7d75fddd8565510f Mon Sep 17 00:00:00 2001 From: petruki <31597636+petruki@users.noreply.github.com> Date: Thu, 30 May 2024 19:44:11 -0700 Subject: [PATCH] Decoupled Slack tickets from Slack installation schema --- src/models/slack.js | 20 +++++----- src/models/slack_ticket.js | 25 +++++++++++- src/routers/slack.js | 8 ++-- src/services/slack.js | 39 ++++++++++++++----- tests/fixtures/db_api.js | 2 + tests/slack.test.js | 79 +++++++++++++++++++++++++++----------- 6 files changed, 125 insertions(+), 48 deletions(-) diff --git a/src/models/slack.js b/src/models/slack.js index 845a185..a0d6d3e 100644 --- a/src/models/slack.js +++ b/src/models/slack.js @@ -1,6 +1,6 @@ import mongoose from 'mongoose'; import moment from 'moment'; -import { slackTicketSchema, TicketStatusType } from './slack_ticket.js'; +import { SlackTicket } from './slack_ticket'; const slackSchema = new mongoose.Schema({ user_id: { @@ -35,11 +35,16 @@ const slackSchema = new mongoose.Schema({ type: String }] }, - tickets: [slackTicketSchema] }, { timestamps: true }); +slackSchema.virtual('slack_ticket', { + ref: 'SlackTicket', + localField: '_id', + foreignField: 'slack' +}); + slackSchema.options.toJSON = { getters: true, virtuals: true, @@ -53,14 +58,11 @@ slackSchema.options.toJSON = { } }; -slackSchema.methods.isTicketOpened = function (ticket_content) { +slackSchema.pre('deleteOne', { document: true, query: false }, async function (next) { const slack = this; - return slack.tickets.filter(ticket => - ticket.environment === ticket_content.environment && - ticket.group === ticket_content.group && - ticket.switcher === ticket_content.switcher && - ticket.ticket_status === TicketStatusType.OPENED); -}; + await SlackTicket.deleteMany({ slack: slack._id }).exec(); + next(); +}); const Slack = mongoose.model('Slack', slackSchema); diff --git a/src/models/slack_ticket.js b/src/models/slack_ticket.js index caa08ae..c728c0f 100644 --- a/src/models/slack_ticket.js +++ b/src/models/slack_ticket.js @@ -1,4 +1,5 @@ import mongoose from 'mongoose'; +import moment from 'moment'; export const SLACK_SUB = 'Switcher Slack App'; export const TicketStatusType = Object.freeze({ @@ -13,7 +14,12 @@ export const TicketValidationType = Object.freeze({ FROZEN_ENVIRONMENT: 'FROZEN_ENVIRONMENT' }); -export const slackTicketSchema = new mongoose.Schema({ +const slackTicketSchema = new mongoose.Schema({ + slack: { + type: mongoose.Schema.Types.ObjectId, + required: true, + ref: 'Slack' + }, environment: { type: String, required: true @@ -43,4 +49,19 @@ export const slackTicketSchema = new mongoose.Schema({ } }, { timestamps: true -}); \ No newline at end of file +}); + +slackTicketSchema.options.toJSON = { + getters: true, + virtuals: true, + minimize: false, + transform: function (_doc, ret) { + if (ret.updatedAt || ret.createdAt) { + ret.updatedAt = moment(ret.updatedAt).format('YYYY-MM-DD HH:mm:ss'); + ret.createdAt = moment(ret.createdAt).format('YYYY-MM-DD HH:mm:ss'); + } + return ret; + } +}; + +export const SlackTicket = mongoose.model('SlackTicket', slackTicketSchema); \ No newline at end of file diff --git a/src/routers/slack.js b/src/routers/slack.js index 7fcc83d..c6c0efd 100644 --- a/src/routers/slack.js +++ b/src/routers/slack.js @@ -174,11 +174,11 @@ router.get('/slack/v1/installation/:domain', auth, [ ], validate, async (req, res) => { try { const domain = await getDomainById(req.params.domain); - const { tickets, installation_payload, settings } = + const { slack_ticket, installation_payload, settings } = await Services.getSlackOrError({ id: domain.integrations.slack }); - const openedTickets = tickets.filter(t => t.ticket_status === TicketStatusType.OPENED).length; - const approvedTickets = tickets.filter(t => t.ticket_status === TicketStatusType.APPROVED).length; + const openedTickets = slack_ticket.filter(t => t.ticket_status === TicketStatusType.OPENED).length; + const approvedTickets = slack_ticket.filter(t => t.ticket_status === TicketStatusType.APPROVED).length; res.send({ team_id: installation_payload.team_id, @@ -189,7 +189,7 @@ router.get('/slack/v1/installation/:domain', auth, [ installed_at: installation_payload.installed_at, tickets_opened: openedTickets, tickets_approved: approvedTickets, - tickets_denied: (tickets.length - openedTickets - approvedTickets), + tickets_denied: (slack_ticket.length - openedTickets - approvedTickets), settings }); } catch (e) { diff --git a/src/services/slack.js b/src/services/slack.js index b3e3d76..7835baa 100644 --- a/src/services/slack.js +++ b/src/services/slack.js @@ -1,5 +1,5 @@ import Slack from '../models/slack.js'; -import { TicketStatusType, SLACK_SUB, TicketValidationType } from '../models/slack_ticket.js'; +import { TicketStatusType, SLACK_SUB, TicketValidationType, SlackTicket } from '../models/slack_ticket.js'; import { BadRequestError, NotFoundError, PermissionError } from '../exceptions/index.js'; import { checkSlackIntegration } from '../external/switcher-api-facade.js'; import { getConfig } from './config.js'; @@ -14,9 +14,11 @@ import { isQueryValid } from './common.js'; * Otherwise, validates if ticket content is valid */ async function canCreateTicket(slack, ticket_content) { - const existingTicket = slack.isTicketOpened(ticket_content); - if (existingTicket.length) { - return existingTicket[0]; + const slackTickets = await hasSlackTicket( + slack._id, ticket_content, TicketStatusType.OPENED); + + if (slackTickets.length) { + return slackTickets[0]; } let group, config; @@ -53,6 +55,20 @@ async function deleteSlackInstallation(slack) { return slack.deleteOne(); } +async function deleteSlackTicketsBySlackId(slackId) { + return SlackTicket.deleteMany({ slack: slackId }); +} + +async function hasSlackTicket(slackId, ticket_content, status) { + const tickets = await SlackTicket.find({ + slack: slackId, + ...ticket_content, + ticket_status: status + }); + + return tickets; +} + export async function getSlackOrError(where, newInstallation = false) { const slack = await getSlack(where, newInstallation); @@ -60,6 +76,7 @@ export async function getSlackOrError(where, newInstallation = false) { throw new NotFoundError('Slack installation not found'); } + await slack.populate({ path: 'slack_ticket' }); return slack; } @@ -146,8 +163,7 @@ export async function resetTicketHistory(enterprise_id, team_id, admin) { throw new PermissionError('Only the domain owner can reset Ticket history'); } - slack.tickets = []; - return slack.save(); + await deleteSlackTicketsBySlackId(slack._id); } export async function validateTicket(ticket_content, enterprise_id, team_id) { @@ -177,9 +193,12 @@ export async function createTicket(ticket_content, enterprise_id, team_id) { ...ticket_content }; - slack.tickets.push(slackTicket); - const { tickets } = await slack.save(); - _ticket = tickets[tickets.length - 1]; + _ticket = new SlackTicket({ + slack: slack._id, + ...slackTicket + }); + + await _ticket.save(); } return { @@ -192,7 +211,7 @@ export async function createTicket(ticket_content, enterprise_id, team_id) { export async function processTicket(enterprise_id, team_id, ticket_id, approved) { const slack = await getSlackOrError({ enterprise_id, team_id }); await getDomainById(slack.domain); - const ticket = slack.tickets.filter( + const ticket = slack.slack_ticket.filter( t => String(t.id) === String(ticket_id) && t.ticket_status === TicketStatusType.OPENED); diff --git a/tests/fixtures/db_api.js b/tests/fixtures/db_api.js index de2086c..7a5cd14 100644 --- a/tests/fixtures/db_api.js +++ b/tests/fixtures/db_api.js @@ -14,6 +14,7 @@ import { Permission, ActionTypes, RouterTypes } from '../../src/models/permissio import { Metric } from '../../src/models/metric'; import { EnvType, Environment } from '../../src/models/environment'; import { ConfigStrategy, StrategiesType, OperationsType } from '../../src/models/config-strategy'; +import { SlackTicket } from '../../src/models/slack_ticket'; import Slack from '../../src/models/slack'; import { EncryptionSalts } from '../../src/models/common'; @@ -238,6 +239,7 @@ export const setupDatabase = async () => { await Environment.deleteMany().exec(); await Component.deleteMany().exec(); await Slack.deleteMany().exec(); + await SlackTicket.deleteMany().exec(); await History.deleteMany().exec(); await Metric.deleteMany().exec(); diff --git a/tests/slack.test.js b/tests/slack.test.js index 1c0aae5..e20cefd 100644 --- a/tests/slack.test.js +++ b/tests/slack.test.js @@ -6,10 +6,11 @@ import app from '../src/app'; import * as Services from '../src/services/slack'; import { getDomainById } from '../src/services/domain'; import { getConfig } from '../src/services/config'; -import { mock1_slack_installation } from './fixtures/db_slack'; import { EnvType } from '../src/models/environment'; +import Domain from '../src/models/domain'; +import { SlackTicket, TicketValidationType } from '../src/models/slack_ticket'; import Slack from '../src/models/slack'; -import { TicketValidationType } from '../src/models/slack_ticket'; +import { mock1_slack_installation } from './fixtures/db_slack'; import { setupDatabase, slack, @@ -20,7 +21,6 @@ import { adminAccountToken, adminMasterAccountId } from './fixtures/db_api'; -import Domain from '../src/models/domain'; afterAll(async () => { await Slack.deleteMany().exec(); @@ -46,6 +46,16 @@ const buildInstallation = async (team_id, domain) => { return installation; }; +const authorizeInstallation = async (installation, domainId, token) => { + await request(app) + .post('/slack/v1/authorize') + .set('Authorization', `Bearer ${token}`) + .send({ + domain: domainId, + team_id: installation.team_id + }).expect(200); +} + const createDomain = async (name) => { const _id = new mongoose.Types.ObjectId(); const domainDocument = { @@ -60,7 +70,27 @@ const createDomain = async (name) => { return _id; }; +const createTicket = async (team_id) => { + let ticket_content = { + environment: EnvType.DEFAULT, + group: groupConfigDocument.name, + switcher: config1Document.key, + status: false + }; + + const response = await request(app) + .post('/slack/v1/ticket/create') + .set('Authorization', `Bearer ${generateToken('30s')}`) + .send({ + team_id, + ticket_content + }); + + return response.body; +}; + describe('Slack Installation', () => { + beforeAll(setupDatabase); test('SLACK_SUITE - Should save installation', async () => { @@ -337,13 +367,22 @@ describe('Slack Installation', () => { }); test('SLACK_SUITE - Should delete authorized installation', async () => { + //given + const installation = await buildInstallation('SHOULD_DELETE_AUTHORIZE_INSTALLATION', null); + await authorizeInstallation(installation, domainId, adminMasterAccountToken); + + const responseCreateTicket = await createTicket(installation.team_id); + //verify that let domain = await getDomainById(domainId); expect(domain.integrations.slack).not.toBe(null); + let slackTickets = await SlackTicket.find({ slack: responseCreateTicket.ticket.slack }).exec(); + expect(slackTickets.length).toBe(1); + //test await request(app) - .delete('/slack/v1/installation?team_id=SHOULD_AUTHORIZE_DOMAIN') + .delete(`/slack/v1/installation?team_id=${installation.team_id}`) .set('Authorization', `Bearer ${generateToken('30s')}`) .send().expect(200); @@ -352,9 +391,12 @@ describe('Slack Installation', () => { expect(domain.integrations.slack).toBe(null); const slackDb = await Services.getSlack({ - team_id: 'SHOULD_AUTHORIZE_DOMAIN' + team_id: installation.team_id }); expect(slackDb).toBe(null); + + slackTickets = await SlackTicket.find({ slack: responseCreateTicket.ticket.slack }).exec(); + expect(slackTickets.length).toBe(0); }); test('SLACK_SUITE - Should NOT delete installation - Not found', async () => { @@ -374,13 +416,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should unlink installation', async () => { //given const installation = await buildInstallation('SHOULD_UNLINK_INTEGRATION', null); - await request(app) - .post('/slack/v1/authorize') - .set('Authorization', `Bearer ${adminMasterAccountToken}`) - .send({ - domain: domainId, - team_id: installation.team_id - }).expect(200); + await authorizeInstallation(installation, domainId, adminMasterAccountToken); //verify that let domain = await getDomainById(domainId); @@ -405,13 +441,7 @@ describe('Slack Installation', () => { test('SLACK_SUITE - Should unlink single installation from Multi Slack Installation', async () => { //given - installation/authorization for Domain 1 const installation = await buildInstallation('MULTI_DOMAIN_TEAM_ID', null); - await request(app) - .post('/slack/v1/authorize') - .set('Authorization', `Bearer ${adminMasterAccountToken}`) - .send({ - domain: domainId, - team_id: installation.team_id - }).expect(200); + await authorizeInstallation(installation, domainId, adminMasterAccountToken); //given - installation/authorization for Domain 2 const domainId2 = await createDomain('Domain 2'); @@ -849,9 +879,12 @@ describe('Slack Route - Process Ticket', () => { }); test('SLACK_SUITE - Should reset installation tickets', async () => { + //given + await createTicket(); + //verify that - let slackInstallation = await Slack.findOne({ team_id: slack.team_id }).exec(); - expect(slackInstallation.tickets.length).toBeGreaterThan(0); + let slackTickets = await SlackTicket.find({ slack: slack._id }).exec(); + expect(slackTickets.length).toBeGreaterThan(0); //test await request(app) @@ -861,8 +894,8 @@ describe('Slack Route - Process Ticket', () => { team_id: slack.team_id }).expect(200); - slackInstallation = await Slack.findOne({ team_id: slack.team_id }).exec(); - expect(slackInstallation.tickets.length).toEqual(0); + slackTickets = await SlackTicket.find({ slack: slack._id }).exec(); + expect(slackTickets.length).toEqual(0); }); test('SLACK_SUITE - Should NOT reset installation tickets - Admin not owner', async () => {