Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions backend/src/routes/v3/response/getLatestResponseForReview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,28 @@ export const getLatestResponseSchema = z.object({
}),
})

registerPath({
method: 'get',
path: '/api/v3/review/{reviewId}/responses/latest',
tags: ['response'],
description: 'Get the latest response for a review',
schema: getLatestResponseSchema,
responses: {
200: {
description: 'A review response.',
content: {
'application/json': {
schema: z.object({
response: responseInterfaceSchema,
}),
registerPath(
{
method: 'get',
path: '/api/v3/review/{reviewId}/responses/latest',
tags: ['response'],
description: 'Get the latest response for a review',
schema: getLatestResponseSchema,
responses: {
200: {
description: 'A review response.',
content: {
'application/json': {
schema: z.object({
response: responseInterfaceSchema,
}),
},
},
},
},
},
})
'v3',
)
Comment on lines +17 to +38

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

While debugging. I noticed this route was on the v2 swagger docs.


interface GetLatestResponseResponse {
response: ResponseInterface
Expand Down
56 changes: 51 additions & 5 deletions backend/src/services/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import ResponseModel, {
ResponseKind,
ResponseReaction,
} from '../models/Response.js'
import { ReviewDoc } from '../models/Review.js'
import ReviewModel, { ReviewDoc } from '../models/Review.js'
import { UserInterface } from '../models/User.js'
import { WebhookEvent } from '../models/Webhook.js'
import { ReviewKind, ReviewKindKeys } from '../types/enums.js'
Expand All @@ -18,7 +18,7 @@ import { getAccessRequestById } from './accessRequest.js'
import log from './log.js'
import { getReleaseBySemver } from './release.js'
import { findReviewForResponse, findReviews, findReviewsForAccessRequests } from './review.js'
import { notifyReviewResponseForAccess, notifyReviewResponseForRelease } from './smtp/smtp.js'
import { notifyReleaseOnApproval, notifyReviewResponseForAccess, notifyReviewResponseForRelease } from './smtp/smtp.js'
import { dispatchWebhooks } from './webhook.js'

export async function findResponseById(responseId: string) {
Expand Down Expand Up @@ -122,7 +122,10 @@ export async function respondToReview(
reviewId: string,
): Promise<ResponseInterface> {
const review = await findReviewForResponse(user, modelId, role, kind, reviewId)

let isApproved = false
if (kind === ReviewKind.Release && review.semver) {
isApproved = await checkReleaseApproved(modelId, review.semver)
}
// Store the response
const reviewResponse = new ResponseModel({
entity: toEntity('user', user.dn),
Expand All @@ -133,7 +136,7 @@ export async function respondToReview(
})

await reviewResponse.save()
await sendReviewResponseNotification(review, reviewResponse, user)
await sendReviewResponseNotification(review, reviewResponse, user, isApproved)

dispatchWebhooks(
review.modelId,
Expand All @@ -149,6 +152,7 @@ export async function sendReviewResponseNotification(
review: ReviewDoc,
reviewResponse: ResponseInterface,
user: UserInterface,
isApproved?: boolean,
) {
switch (review.kind) {
case ReviewKind.Access: {
Expand All @@ -168,11 +172,15 @@ export async function sendReviewResponseNotification(
log.error({ review }, 'Unable to send notification for review response. Cannot find semver.')
return
}

const release = await getReleaseBySemver(user, review.modelId, review.semver)
notifyReviewResponseForRelease(reviewResponse, release).catch((error) =>
log.warn({ error }, 'Error when notifying collaborators about review response.'),
)
if (!isApproved && (await checkReleaseApproved(release.modelId, release.semver))) {
notifyReleaseOnApproval(release.modelId, release).catch((error) =>
log.warn({ error }, 'Error when notifying release approval.'),
)
}
break
}
case ReviewKind.Lifecycle: {
Expand All @@ -197,6 +205,44 @@ export async function checkAccessRequestsApproved(accessRequestIds: string[]) {
return approvals.length > 0
}

export async function checkReleaseApproved(modelId: string, semver: string) {
const reviewsWithoutApproval = await ReviewModel.aggregate([
{ $match: { semver, modelId } },
{
$lookup: {
from: 'v2_responses',
localField: '_id',
foreignField: 'parentId',
as: 'responses',
},
},
{
$addFields: {
latestResponse: {
$arrayElemAt: [
{
$sortArray: {
input: '$responses',
sortBy: { createdAt: -1 },
},
},
0,
],
},
},
},
{
$match: {
$or: [{ latestResponse: { $exists: false } }, { 'latestResponse.decision': { $ne: Decision.Approve } }],
},
},
])

const totalReviews = await ReviewModel.countDocuments({ semver })

return totalReviews > 0 && reviewsWithoutApproval.length === 0
}

export async function removeResponsesByParentIds(parentIds: string[], session: ClientSession | undefined) {
const objectIds = parentIds.map((id) => new Types.ObjectId(id))
const responses = await ResponseModel.find({
Expand Down
39 changes: 39 additions & 0 deletions backend/src/services/smtp/smtp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import nodemailer, { Transporter } from 'nodemailer'
import { createSesTransporter } from '../../clients/ses.js'
import authentication from '../../connectors/authentication/index.js'
import AccessRequestModel, { AccessRequestDoc } from '../../models/AccessRequest.js'
import { SystemRoles } from '../../models/Model.js'
import ReleaseModel, { ReleaseDoc } from '../../models/Release.js'
import { ResponseInterface } from '../../models/Response.js'
import { ReviewDoc, ReviewInterface } from '../../models/Review.js'
Expand All @@ -14,6 +15,7 @@ import { BadReq, NotFound } from '../../utils/error.js'
import { resolveKindToUrl, toTitleCase } from '../../utils/string.js'
import log from '../log.js'
import { getModelByIdNoAuth, getRoleEntities } from '../model.js'
import { checkAccessRequestsApproved } from '../response.js'
import { buildEmail, EmailContent, Info } from './emailBuilder.js'

const appBaseUrl = `${config.app.protocol}://${config.app.host}:${config.app.port}`
Expand Down Expand Up @@ -79,6 +81,17 @@ async function dispatchEmail(entities: string[], emailContent: EmailContent) {
}
}

export async function getApprovedAccessRequests(modelId: string) {
const accessRequests = await AccessRequestModel.find({
modelId,
})
const approvalResults = await Promise.all(
accessRequests.map((accessRequest) => checkAccessRequestsApproved([accessRequest.id])),
)
const approvedAccessRequests = accessRequests.filter((_, index) => approvalResults[index])
return approvedAccessRequests.flatMap((accessRequest) => accessRequest.metadata.overview.entities)
}

export async function requestReviewForRelease(entities: string[], review: ReviewDoc, release: ReleaseDoc) {
if (!config.smtp.enabled) {
log.info('Not sending email due to SMTP disabled')
Expand Down Expand Up @@ -330,6 +343,32 @@ export async function startImportNotification(modelId: string) {
await dispatchEmail(ownerEntities, await emailContent)
}

export async function notifyReleaseOnApproval(modelId: string, release: ReleaseDoc) {
if (!config.smtp.enabled) {
log.info('Not sending email due to SMTP disabled')
return
}

const emailContent = buildEmail(
`A new release has been approved`,
[
{ title: 'Model ID', data: release.modelId },
{ title: 'Semver', data: release.semver },
],
[
{ name: 'See Model', url: `${appBaseUrl}/model/${modelId}` },
{ name: 'See Release', url: getReleaseUrl(release) },
],
)
const model = await getModelByIdNoAuth(modelId)
const entries = Object.values(
getRoleEntities([SystemRoles.Consumer, SystemRoles.Owner, SystemRoles.Contributor], model.collaborators),
)
.flat()
.concat(await getApprovedAccessRequests(modelId))
await dispatchEmail(entries, await emailContent)
}

export async function transferCompleteNotification(
modelId: string,
failed: boolean,
Expand Down
94 changes: 94 additions & 0 deletions backend/test/services/response.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import { describe, expect, test, vi } from 'vitest'
import { Decision, ReactionKind } from '../../src/models/Response.js'
import {
checkAccessRequestsApproved,
checkReleaseApproved,
findResponseById,
getResponsesByParentIds,
respondToReview,
sendReviewResponseNotification,
updateResponse,
updateResponseReaction,
} from '../../src/services/response.js'
Expand All @@ -24,6 +26,7 @@ vi.mock('../../src/connectors/authentication/index.js', () => ({
}))

const ResponseModelMock = getTypedModelMock('ResponseModel')
const ReviewModelMock = getTypedModelMock('ReviewModel')

const smtpMock = vi.hoisted(() => ({
notifyReviewResponseForAccess: vi.fn(function () {
Expand All @@ -32,6 +35,7 @@ const smtpMock = vi.hoisted(() => ({
notifyReviewResponseForRelease: vi.fn(function () {
return Promise.resolve()
}),
notifyReleaseOnApproval: vi.fn(() => Promise.resolve()),
requestReviewForRelease: vi.fn(function () {
return Promise.resolve()
}),
Expand Down Expand Up @@ -229,6 +233,10 @@ describe('services > response', () => {
})

test('respondToReview > release successful', async () => {
releaseServiceMock.getReleaseBySemver.mockResolvedValueOnce({
modelId: 'abc',
semver: '3.0.3',
})
await respondToReview(
user,
'modelId',
Expand Down Expand Up @@ -312,6 +320,10 @@ describe('services > response', () => {

test('respondToReview > log when failed to send release response notification', async () => {
smtpMock.notifyReviewResponseForRelease.mockRejectedValueOnce('failed to send')
releaseServiceMock.getReleaseBySemver.mockResolvedValueOnce({
modelId: 'abc',
semver: '3.0.3',
})
await respondToReview(
user,
'modelId',
Expand Down Expand Up @@ -392,4 +404,86 @@ describe('services > response', () => {
expect(result).toBe(false)
expect(reviewMock.findReviewsForAccessRequests.mock.calls).toMatchSnapshot()
})

describe('checkReleaseApproved', () => {
test('returns true when all reviews have approved latest response', async () => {
ReviewModelMock.aggregate.mockResolvedValueOnce([])
ReviewModelMock.countDocuments.mockResolvedValueOnce(2)

const result = await checkReleaseApproved('modelId', '1.0.0')

expect(result).toBe(true)
})

test('returns false when some reviews lack approval', async () => {
ReviewModelMock.aggregate.mockResolvedValueOnce([{ _id: 'review1' }])
ReviewModelMock.countDocuments.mockResolvedValueOnce(2)

const result = await checkReleaseApproved('modelId', '1.0.0')

expect(result).toBe(false)
})

test('returns false when there are no reviews', async () => {
ReviewModelMock.aggregate.mockResolvedValueOnce([])
ReviewModelMock.countDocuments.mockResolvedValueOnce(0)

const result = await checkReleaseApproved('modelId', '1.0.0')

expect(result).toBe(false)
})
})

describe('sendReviewResponseNotification', () => {
test('calls notifyReleaseOnApproval when release becomes newly approved', async () => {
const review = { ...testReleaseReview, kind: ReviewKind.Release } as any
const reviewResponse = { entity: 'user:test', decision: 'approve', role: 'msro' } as any

releaseServiceMock.getReleaseBySemver.mockResolvedValueOnce({
modelId: 'abc',
semver: '3.0.3',
})
ReviewModelMock.aggregate.mockResolvedValueOnce([])
ReviewModelMock.countDocuments.mockResolvedValueOnce(1)

await sendReviewResponseNotification(review, reviewResponse, user, false)

expect(smtpMock.notifyReviewResponseForRelease).toHaveBeenCalledOnce()
expect(smtpMock.notifyReleaseOnApproval).toHaveBeenCalledOnce()
})

test('does not call notifyReleaseOnApproval when release was already approved', async () => {
const review = { ...testReleaseReview, kind: ReviewKind.Release } as any
const reviewResponse = { entity: 'user:test', decision: 'approve', role: 'msro' } as any

releaseServiceMock.getReleaseBySemver.mockResolvedValueOnce({
modelId: 'abc',
semver: '3.0.3',
})
ReviewModelMock.aggregate.mockResolvedValueOnce([])
ReviewModelMock.countDocuments.mockResolvedValueOnce(1)

await sendReviewResponseNotification(review, reviewResponse, user, true)

expect(smtpMock.notifyReviewResponseForRelease).toHaveBeenCalledOnce()
expect(smtpMock.notifyReleaseOnApproval).not.toHaveBeenCalled()
})

test('does not call notifyReleaseOnApproval when release is not fully approved', async () => {
const review = { ...testReleaseReview, kind: ReviewKind.Release } as any
const reviewResponse = { entity: 'user:test', decision: 'approve', role: 'msro' } as any

releaseServiceMock.getReleaseBySemver.mockResolvedValueOnce({
modelId: 'abc',
semver: '3.0.3',
})
ReviewModelMock.aggregate.mockResolvedValueOnce([{ _id: 'unapproved' }])
ReviewModelMock.countDocuments.mockResolvedValueOnce(2)

await sendReviewResponseNotification(review, reviewResponse, user, false)

expect(smtpMock.notifyReviewResponseForRelease).toHaveBeenCalledOnce()
expect(smtpMock.notifyReleaseOnApproval).not.toHaveBeenCalled()
})
})
})
18 changes: 18 additions & 0 deletions backend/test/services/smtp/smtp.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ReviewInterface } from '../../../src/models/Review.js'
import { UserInterface } from '../../../src/models/User.js'
import {
notifyLifeCycleReview,
notifyReleaseOnApproval,
notifyReviewResponseForAccess,
notifyReviewResponseForRelease,
notifyReviewRoleOfAdditionalReview,
Expand Down Expand Up @@ -109,9 +110,15 @@ const responseService = vi.hoisted(() => ({
kind: 'review',
}
}),
checkAccessRequestsApproved: vi.fn(() => true),
}))
vi.mock('../../../src/services/response.js', async () => responseService)

const AccessRequestModelMock = vi.hoisted(() => ({
find: vi.fn(() => [] as any[]),
}))
vi.mock('../../../src/models/AccessRequest.js', () => ({ default: AccessRequestModelMock }))

const getModelByIdMock = vi.hoisted(() =>
vi.fn(function () {
return {
Expand Down Expand Up @@ -307,4 +314,15 @@ describe('services > smtp > smtp', () => {
await notifyReviewResponseForRelease(testReviewResponse as any, release)
expect(transporterMock.sendMail).toHaveBeenCalledTimes(1)
})

test('that an email is sent to all stakeholders on release', async () => {
getModelByIdMock.mockReturnValue({
id: 'modelId',
name: 'Test Model',
kind: 'model',
collaborators: [{ entity: 'user:user', roles: ['owner'] }],
} as any)
await notifyReleaseOnApproval('modelId', release)
expect(transporterMock.sendMail).toHaveBeenCalledTimes(1)
})
})
Loading