From 7c6c4fa0b09830598df7989216c1303e8ab4cbb4 Mon Sep 17 00:00:00 2001 From: Hsu Zhong Jun <27919917+dcshzj@users.noreply.github.com> Date: Fri, 14 Oct 2022 16:04:47 +0800 Subject: [PATCH] feat: introduce a new site info API endpoint (#513) * ref(fixtures): convert repoInfo to typescript * ref(services): migrate SitesService to typescript * tests: update unit and integration tests for SitesService * ref(sites): migrate sites router to typescript * fix: revert back to using SessionData * fix: remove use of Bluebird and unused getSiteToken function * fix: use more accurate type * chore: remove unused variable * refactor(tests): migrate generic axios instance to __mocks__ * feat: introduce function to obtain latest commit details * feat: add function for obtaining a User by ID * feat: introduce a new site info API endpoint * tests: add partial tests for SitesService * tests: use mockAxios directly instead of preparing an instance * tests: fix SitesService unit tests to pass * chore: adjust constants to use SCREAMING_SNAKE_CASE * fix: add authorizationMiddleware to ensure user is member of site * chore: combine sessionData unpacking * fix: insert try-catch to handle errors from JSON.parse * chore: remove unnecessary check for undefined site * chore: return instead of throwing NotFoundError * fix: add assertion to ensure integrity of GitHubCommitData * fix: remove need for adding site name to sessionData * refactor: convert routes Sites.spec.js to TypeScript * refactor: redesign getUrlsOfSite to increase readability * fix: use correct endpoint to get latest commit data * test: add unit tests for GitHubService getLatestCommitOfBranch * fix: add stub for obtaining merge author details * fix: return a well-formatted response for known exceptions * test: enhance GitHubService test for all other error statuses * chore: rename isType function and return boolean directly * fix: create new siteUrls object instead of changing in-place * fix: handle case of null or undefined user email * chore: improve code style * tests: fix output of getStagingUrl --- src/fixtures/identity.ts | 24 + src/integration/Sites.spec.ts | 30 +- .../{Sites.spec.js => Sites.spec.ts} | 58 ++- src/routes/v2/authenticated/index.js | 5 +- src/routes/v2/authenticated/sites.ts | 72 +-- src/services/db/GitHubService.js | 22 + .../db/__tests__/GitHubService.spec.js | 62 +++ src/services/identity/SitesService.ts | 248 +++++++++- src/services/identity/UsersService.ts | 4 + .../identity/__tests__/SitesService.spec.ts | 456 ++++++++++++++++-- src/types/commitData.ts | 8 + src/types/configYml.ts | 4 + src/types/siteInfo.ts | 8 + 13 files changed, 899 insertions(+), 102 deletions(-) rename src/routes/v2/authenticated/__tests__/{Sites.spec.js => Sites.spec.ts} (58%) create mode 100644 src/types/commitData.ts create mode 100644 src/types/configYml.ts create mode 100644 src/types/siteInfo.ts diff --git a/src/fixtures/identity.ts b/src/fixtures/identity.ts index 8d0990547..ab1d7ae7b 100644 --- a/src/fixtures/identity.ts +++ b/src/fixtures/identity.ts @@ -2,6 +2,8 @@ import { Attributes } from "sequelize/types" import { User, SiteMember } from "@database/models" +import { mockIsomerUserId } from "./sessionData" + export const mockRecipient = "hello@world.com" export const mockSubject = "mock subject" export const mockBody = "somebody" @@ -138,3 +140,25 @@ export const mockSiteOrmResponseWithNoCollaborators = { id: 1, site_members: "", } + +export const MOCK_COMMIT_MESSAGE_ONE = "Update file: Example.md" +export const MOCK_COMMIT_FILENAME_ONE = "Example.md" +export const MOCK_GITHUB_NAME_ONE = "testuser" +export const MOCK_GITHUB_EMAIL_ADDRESS_ONE = "test@example.com" +export const MOCK_GITHUB_DATE_ONE = "2022-09-22T04:07:53Z" +export const MOCK_COMMIT_MESSAGE_OBJECT_ONE = { + message: MOCK_COMMIT_MESSAGE_ONE, + fileName: MOCK_COMMIT_FILENAME_ONE, + userId: mockIsomerUserId, +} + +export const MOCK_COMMIT_MESSAGE_TWO = "Update file: Test.md" +export const MOCK_COMMIT_FILENAME_TWO = "Test.md" +export const MOCK_GITHUB_NAME_TWO = "testuser2" +export const MOCK_GITHUB_EMAIL_ADDRESS_TWO = "test2@example.com" +export const MOCK_GITHUB_DATE_TWO = "2022-09-28T06:25:14Z" +export const MOCK_COMMIT_MESSAGE_OBJECT_TWO = { + message: MOCK_COMMIT_MESSAGE_TWO, + fileName: MOCK_COMMIT_FILENAME_TWO, + userId: mockIsomerUserId, +} diff --git a/src/integration/Sites.spec.ts b/src/integration/Sites.spec.ts index 1d5a370b1..f48873a7e 100644 --- a/src/integration/Sites.spec.ts +++ b/src/integration/Sites.spec.ts @@ -2,17 +2,26 @@ import express from "express" import mockAxios from "jest-mock-axios" import request from "supertest" -import { IsomerAdmin, Repo, Site, SiteMember, User } from "@database/models" +import { + IsomerAdmin, + Repo, + Site, + SiteMember, + User, + Whitelist, +} from "@database/models" import { generateRouter } from "@fixtures/app" import UserSessionData from "@root/classes/UserSessionData" import { mockEmail, mockIsomerUserId } from "@root/fixtures/sessionData" +import { getAuthorizationMiddleware } from "@root/middleware" import { SitesRouter as _SitesRouter } from "@root/routes/v2/authenticated/sites" import { GitHubService } from "@root/services/db/GitHubService" import { ConfigYmlService } from "@root/services/fileServices/YmlFileServices/ConfigYmlService" import IsomerAdminsService from "@root/services/identity/IsomerAdminsService" import SitesService from "@root/services/identity/SitesService" import TokenStore from "@root/services/identity/TokenStore" -import { getUsersService } from "@services/identity" +import { getIdentityAuthService, getUsersService } from "@services/identity" +import CollaboratorsService from "@services/identity/CollaboratorsService" import { sequelize } from "@tests/database" const mockSite = "mockSite" @@ -27,6 +36,7 @@ const configYmlService = new ConfigYmlService({ gitHubService }) const usersService = getUsersService(sequelize) const isomerAdminsService = new IsomerAdminsService({ repository: IsomerAdmin }) const tokenStore = new TokenStore() +const identityAuthService = getIdentityAuthService(gitHubService) const sitesService = new SitesService({ siteRepository: Site, gitHubService, @@ -35,8 +45,22 @@ const sitesService = new SitesService({ isomerAdminsService, tokenStore, }) +const collaboratorsService = new CollaboratorsService({ + siteRepository: Site, + siteMemberRepository: SiteMember, + sitesService, + usersService, + whitelist: Whitelist, +}) + +const authorizationMiddleware = getAuthorizationMiddleware({ + identityAuthService, + usersService, + isomerAdminsService, + collaboratorsService, +}) -const SitesRouter = new _SitesRouter({ sitesService }) +const SitesRouter = new _SitesRouter({ sitesService, authorizationMiddleware }) const sitesSubrouter = SitesRouter.getRouter() // Set up express with defaults and use the router under test diff --git a/src/routes/v2/authenticated/__tests__/Sites.spec.js b/src/routes/v2/authenticated/__tests__/Sites.spec.ts similarity index 58% rename from src/routes/v2/authenticated/__tests__/Sites.spec.js rename to src/routes/v2/authenticated/__tests__/Sites.spec.ts index 9bf1e6a65..894760a28 100644 --- a/src/routes/v2/authenticated/__tests__/Sites.spec.js +++ b/src/routes/v2/authenticated/__tests__/Sites.spec.ts @@ -1,37 +1,40 @@ -const express = require("express") -const request = require("supertest") +import express from "express" +import request from "supertest" -const { attachReadRouteHandlerWrapper } = require("@middleware/routeHandler") +import type { AuthorizationMiddleware } from "@middleware/authorization" +import { attachReadRouteHandlerWrapper } from "@middleware/routeHandler" -const { generateRouter } = require("@fixtures/app") -const { +import { generateRouter } from "@fixtures/app" +import { mockSiteName, mockUserSessionData, mockUserWithSiteSessionData, -} = require("@fixtures/sessionData") +} from "@fixtures/sessionData" +import type SitesService from "@services/identity/SitesService" -const { SitesRouter } = require("../sites") +import { SitesRouter } from "../sites" describe("Sites Router", () => { const mockSitesService = { getSites: jest.fn(), - checkHasAccess: jest.fn(), getLastUpdated: jest.fn(), getStagingUrl: jest.fn(), + getSiteInfo: jest.fn(), + } + + const mockAuthorizationMiddleware = { + verifySiteMember: jest.fn(), } const router = new SitesRouter({ - sitesService: mockSitesService, + sitesService: (mockSitesService as unknown) as SitesService, + authorizationMiddleware: (mockAuthorizationMiddleware as unknown) as AuthorizationMiddleware, }) const subrouter = express() // We can use read route handler here because we don't need to lock the repo subrouter.get("/", attachReadRouteHandlerWrapper(router.getSites)) - subrouter.get( - "/:siteName", - attachReadRouteHandlerWrapper(router.checkHasAccess) - ) subrouter.get( "/:siteName/lastUpdated", attachReadRouteHandlerWrapper(router.getLastUpdated) @@ -40,6 +43,10 @@ describe("Sites Router", () => { "/:siteName/stagingUrl", attachReadRouteHandlerWrapper(router.getStagingUrl) ) + subrouter.get( + "/:siteName/info", + attachReadRouteHandlerWrapper(router.getSiteInfo) + ) const app = generateRouter(subrouter) beforeEach(() => { @@ -77,7 +84,7 @@ describe("Sites Router", () => { }) describe("getStagingUrl", () => { - it("returns the last updated time", async () => { + it("returns the site's staging URL", async () => { const stagingUrl = "staging-url" mockSitesService.getStagingUrl.mockResolvedValueOnce(stagingUrl) @@ -85,10 +92,31 @@ describe("Sites Router", () => { .get(`/${mockSiteName}/stagingUrl`) .expect(200) - expect(resp.body).toStrictEqual({ stagingUrl }) + expect(resp.body).toStrictEqual({ possibleStagingUrl: stagingUrl }) expect(mockSitesService.getStagingUrl).toHaveBeenCalledWith( mockUserWithSiteSessionData ) }) }) + + describe("getSiteInfo", () => { + it("returns the site's info", async () => { + const siteInfo = { + savedAt: 12345678, + savedBy: "test@example.com", + publishedAt: 23456789, + publishedBy: "test2@example.com", + stagingUrl: "staging-url", + siteUrl: "prod-url", + } + mockSitesService.getSiteInfo.mockResolvedValueOnce(siteInfo) + + const resp = await request(app).get(`/${mockSiteName}/info`).expect(200) + + expect(resp.body).toStrictEqual(siteInfo) + expect(mockSitesService.getSiteInfo).toHaveBeenCalledWith( + mockUserWithSiteSessionData + ) + }) + }) }) diff --git a/src/routes/v2/authenticated/index.js b/src/routes/v2/authenticated/index.js index 5f9c81378..90ef72c4b 100644 --- a/src/routes/v2/authenticated/index.js +++ b/src/routes/v2/authenticated/index.js @@ -18,7 +18,10 @@ const getAuthenticatedSubrouter = ({ }) => { const netlifyTomlService = new NetlifyTomlService() - const sitesV2Router = new SitesRouter({ sitesService }) + const sitesV2Router = new SitesRouter({ + sitesService, + authorizationMiddleware, + }) const collaboratorsRouter = new CollaboratorsRouter({ collaboratorsService, authorizationMiddleware, diff --git a/src/routes/v2/authenticated/sites.ts b/src/routes/v2/authenticated/sites.ts index 0b1b2d9c6..d8e18e857 100644 --- a/src/routes/v2/authenticated/sites.ts +++ b/src/routes/v2/authenticated/sites.ts @@ -1,45 +1,40 @@ import autoBind from "auto-bind" import express from "express" +import type { AuthorizationMiddleware } from "@middleware/authorization" import { attachReadRouteHandlerWrapper } from "@middleware/routeHandler" import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" import type UserSessionData from "@root/classes/UserSessionData" +import { BaseIsomerError } from "@root/errors/BaseError" import { attachSiteHandler } from "@root/middleware" import type { RequestHandler } from "@root/types" import type SitesService from "@services/identity/SitesService" type SitesRouterProps = { sitesService: SitesService + authorizationMiddleware: AuthorizationMiddleware } export class SitesRouter { private readonly sitesService - constructor({ sitesService }: SitesRouterProps) { + private readonly authorizationMiddleware + + constructor({ sitesService, authorizationMiddleware }: SitesRouterProps) { this.sitesService = sitesService + this.authorizationMiddleware = authorizationMiddleware // We need to bind all methods because we don't invoke them from the class directly autoBind(this) } - addSiteNameToSessionData(userSessionData: UserSessionData, siteName: string) { - const { githubId, accessToken, isomerUserId, email } = userSessionData - return new UserWithSiteSessionData({ - githubId, - accessToken, - isomerUserId, - email, - siteName, - }) - } - getSites: RequestHandler< never, unknown, never, never, - { userSessionData: UserWithSiteSessionData } + { userSessionData: UserSessionData } > = async (req, res) => { const { userSessionData } = res.locals const siteNames = await this.sitesService.getSites(userSessionData) @@ -51,14 +46,9 @@ export class SitesRouter { unknown, never, never, - { userSessionData: UserWithSiteSessionData } + { userWithSiteSessionData: UserWithSiteSessionData } > = async (req, res) => { - const { userSessionData } = res.locals - const { siteName } = req.params - const userWithSiteSessionData = this.addSiteNameToSessionData( - userSessionData, - siteName - ) + const { userWithSiteSessionData } = res.locals const lastUpdated = await this.sitesService.getLastUpdated( userWithSiteSessionData ) @@ -70,18 +60,38 @@ export class SitesRouter { unknown, never, never, - { userSessionData: UserWithSiteSessionData } + { userWithSiteSessionData: UserWithSiteSessionData } > = async (req, res) => { - const { userSessionData } = res.locals - const { siteName } = req.params - const userWithSiteSessionData = this.addSiteNameToSessionData( - userSessionData, - siteName + const { userWithSiteSessionData } = res.locals + const possibleStagingUrl = await this.sitesService.getStagingUrl( + userWithSiteSessionData ) - const stagingUrl = await this.sitesService.getStagingUrl( + + // Check for error and throw + if (possibleStagingUrl instanceof BaseIsomerError) { + return res.status(404).json({ message: possibleStagingUrl.message }) + } + return res.status(200).json({ possibleStagingUrl }) + } + + getSiteInfo: RequestHandler< + { siteName: string }, + unknown, + never, + never, + { userWithSiteSessionData: UserWithSiteSessionData } + > = async (req, res) => { + const { userWithSiteSessionData } = res.locals + + const possibleSiteInfo = await this.sitesService.getSiteInfo( userWithSiteSessionData ) - return res.status(200).json({ stagingUrl }) + + // Check for error and throw + if (possibleSiteInfo instanceof BaseIsomerError) { + return res.status(400).json({ message: possibleSiteInfo.message }) + } + return res.status(200).json(possibleSiteInfo) } getRouter() { @@ -98,6 +108,12 @@ export class SitesRouter { attachSiteHandler, attachReadRouteHandlerWrapper(this.getStagingUrl) ) + router.get( + "/:siteName/info", + attachSiteHandler, + this.authorizationMiddleware.verifySiteMember, + attachReadRouteHandlerWrapper(this.getSiteInfo) + ) return router } diff --git a/src/services/db/GitHubService.js b/src/services/db/GitHubService.js index 394332995..567326178 100644 --- a/src/services/db/GitHubService.js +++ b/src/services/db/GitHubService.js @@ -9,6 +9,7 @@ const { inputNameConflictErrorMsg, } = require("@errors/ConflictError") const { NotFoundError } = require("@errors/NotFoundError") +const { UnprocessableError } = require("@errors/UnprocessableError") class GitHubService { constructor({ axiosInstance }) { @@ -284,6 +285,27 @@ class GitHubService { return { treeSha, currentCommitSha } } + async getLatestCommitOfBranch(sessionData, branch) { + const { accessToken, siteName } = sessionData + const endpoint = `${siteName}/commits/${branch}` + const headers = { + Authorization: `token ${accessToken}`, + } + // Get the commits of the repo + try { + const { data: latestCommit } = await this.axiosInstance.get(endpoint, { + headers, + }) + const { commit: latestCommitMeta } = latestCommit + return latestCommitMeta + } catch (err) { + const { status } = err.response + if (status === 422) + throw new UnprocessableError(`Branch ${branch} does not exist`) + throw err + } + } + async getTree(sessionData, githubSessionData, { isRecursive }) { const { accessToken } = sessionData const { siteName } = sessionData diff --git a/src/services/db/__tests__/GitHubService.spec.js b/src/services/db/__tests__/GitHubService.spec.js index b89868003..65846c1d7 100644 --- a/src/services/db/__tests__/GitHubService.spec.js +++ b/src/services/db/__tests__/GitHubService.spec.js @@ -1,5 +1,6 @@ const { ConflictError } = require("@errors/ConflictError") const { NotFoundError } = require("@errors/NotFoundError") +const { UnprocessableError } = require("@errors/UnprocessableError") const validateStatus = require("@utils/axios-utils") @@ -557,6 +558,67 @@ describe("Github Service", () => { }) }) + describe("getLatestCommitOfBranch", () => { + const endpoint = `${siteName}/commits/staging` + const headers = { + Authorization: `token ${accessToken}`, + } + + it("Getting the latest commit of branch works correctly", async () => { + const expected = { + author: { + name: "test", + }, + } + const resp = { + data: { + commit: expected, + }, + } + mockAxiosInstance.get.mockResolvedValueOnce(resp) + const actual = await service.getLatestCommitOfBranch( + sessionData, + "staging" + ) + expect(actual).toEqual(expected) + expect(mockAxiosInstance.get).toHaveBeenCalledWith(endpoint, { + headers, + }) + }) + + it("Getting an invalid branch should throw UnprocessableError", async () => { + mockAxiosInstance.get.mockImplementationOnce(() => { + const err = new Error() + err.response = { + status: 422, + } + throw err + }) + await expect( + service.getLatestCommitOfBranch(sessionData, "staging") + ).rejects.toThrowError(UnprocessableError) + expect(mockAxiosInstance.get).toHaveBeenCalledWith(endpoint, { + headers, + }) + }) + + it("Getting other kinds of errors should throw the original error", async () => { + mockAxiosInstance.get.mockImplementationOnce(() => { + const err = new Error() + err.response = { + status: 418, + } + throw err + }) + await expect( + service.getLatestCommitOfBranch(sessionData, "staging") + ).rejects.toThrowError() + expect(mockAxiosInstance.get).toHaveBeenCalledWith(endpoint, { + headers, + }) + }) + }) + describe("GetTree", () => { const url = `${siteName}/git/trees/${treeSha}` diff --git a/src/services/identity/SitesService.ts b/src/services/identity/SitesService.ts index 0817c7db0..d2d59717b 100644 --- a/src/services/identity/SitesService.ts +++ b/src/services/identity/SitesService.ts @@ -1,7 +1,7 @@ import _ from "lodash" import { ModelStatic } from "sequelize" -import { Site } from "@database/models" +import { Deployment, Site } from "@database/models" import type UserSessionData from "@root/classes/UserSessionData" import type UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData" import { @@ -11,8 +11,12 @@ import { ISOMER_ADMIN_REPOS, } from "@root/constants" import { NotFoundError } from "@root/errors/NotFoundError" +import { UnprocessableError } from "@root/errors/UnprocessableError" import { genericGitHubAxiosInstance } from "@root/services/api/AxiosInstance" +import { GitHubCommitData } from "@root/types/commitData" +import { ConfigYmlData } from "@root/types/configYml" import type { GitHubRepositoryData, RepositoryData } from "@root/types/repoInfo" +import { SiteInfo } from "@root/types/siteInfo" import { GitHubService } from "@services/db/GitHubService" import { ConfigYmlService } from "@services/fileServices/YmlFileServices/ConfigYmlService" import IsomerAdminsService from "@services/identity/IsomerAdminsService" @@ -29,6 +33,9 @@ interface SitesServiceProps { tokenStore: TokenStore } +type SiteUrlTypes = "staging" | "prod" +type SiteUrls = { [key in SiteUrlTypes]: string } + class SitesService { // NOTE: Explicitly specifying using keyed properties to ensure // that the types are synced. @@ -60,6 +67,85 @@ class SitesService { this.tokenStore = tokenStore } + isGitHubCommitData(commit: any): commit is GitHubCommitData { + return ( + commit && + (commit as GitHubCommitData).author !== undefined && + (commit as GitHubCommitData).author.name !== undefined && + (commit as GitHubCommitData).author.date !== undefined && + (commit as GitHubCommitData).author.email !== undefined && + (commit as GitHubCommitData).message !== undefined + ) + } + + async insertUrlsFromConfigYml( + siteUrls: SiteUrls, + sessionData: UserWithSiteSessionData + ): Promise { + if (siteUrls.staging && siteUrls.prod) { + // We call ConfigYmlService only when necessary + return siteUrls + } + + const { + content: configYmlData, + }: { content: ConfigYmlData } = await this.configYmlService.read( + sessionData + ) + + // Only replace the urls if they are not already present + const newSiteUrls: SiteUrls = { + staging: + configYmlData.staging && !siteUrls.staging + ? configYmlData.staging + : siteUrls.staging, + prod: + configYmlData.prod && !siteUrls.prod + ? configYmlData.prod + : siteUrls.prod, + } + + return newSiteUrls + } + + async insertUrlsFromGitHubDescription( + siteUrls: SiteUrls, + sessionData: UserWithSiteSessionData + ): Promise { + if (siteUrls.staging && siteUrls.prod) { + // We call GitHubService only when necessary + return siteUrls + } + + const { + description, + }: { description: string } = await this.gitHubService.getRepoInfo( + sessionData + ) + + // Retrieve the url from the description + // repo descriptions have varying formats, so we look for the first link + const repoDescTokens = description.replace("/;/g", " ").split(" ") + + const stagingUrlFromDesc = repoDescTokens.find( + (token) => token.includes("http") && token.includes("staging") + ) + const prodUrlFromDesc = repoDescTokens.find( + (token) => token.includes("http") && token.includes("prod") + ) + + // Only replace the urls if they are not already present + const newSiteUrls: SiteUrls = { + staging: + stagingUrlFromDesc && !siteUrls.staging + ? stagingUrlFromDesc + : siteUrls.staging, + prod: prodUrlFromDesc && !siteUrls.prod ? prodUrlFromDesc : siteUrls.prod, + } + + return newSiteUrls + } + async getBySiteName(siteName: string): Promise { const site = await this.siteRepository.findOne({ where: { name: siteName }, @@ -78,6 +164,89 @@ class SitesService { return user.site_members.map((site) => site.repo?.name) } + async getCommitAuthorEmail(commit: GitHubCommitData) { + const { message } = commit + + // Commit message created as part of phase 2 identity + if (message.startsWith("{") && message.endsWith("}")) { + try { + const { userId }: { userId: string } = JSON.parse(message) + const user = await this.usersService.findById(userId) + + if (user && user.email) { + return user.email + } + } catch (e) { + // Do nothing + } + } + + // Legacy style of commits, or if the user is not found + const { + author: { email: authorEmail }, + } = commit + return authorEmail + } + + async getMergeAuthorEmail(commit: GitHubCommitData) { + const { + author: { name: authorName }, + } = commit + + // Commit was made by our common identity GitHub user + if (authorName.startsWith("isomergithub")) { + // TODO: Retrieve email address of user from review request table + } + + // Legacy style of using pull requests, or if the user is not found + const { + author: { email: authorEmail }, + } = commit + return authorEmail + } + + async getUrlsOfSite( + sessionData: UserWithSiteSessionData + ): Promise { + // Tries to get the site urls in the following order: + // 1. From the deployments database table + // 2. From the config.yml file + // 3. From the GitHub repository description + // Otherwise, returns a NotFoundError + const { siteName } = sessionData + + const site = await this.siteRepository.findOne({ + where: { name: siteName }, + include: { + model: Deployment, + as: "deployment", + }, + }) + + // Note: site may be null if the site does not exist + const siteUrls: SiteUrls = { + staging: site?.deployment?.stagingUrl ?? "", + prod: site?.deployment?.productionUrl ?? "", + } + + _.assign( + siteUrls, + await this.insertUrlsFromConfigYml(siteUrls, sessionData) + ) + _.assign( + siteUrls, + await this.insertUrlsFromGitHubDescription(siteUrls, sessionData) + ) + + if (!siteUrls.staging && !siteUrls.prod) { + return new NotFoundError( + `The site ${siteName} does not have a staging or production url` + ) + } + + return siteUrls + } + async getSites(sessionData: UserSessionData): Promise { const isEmailUser = sessionData.isEmailUser() const { isomerUserId: userId } = sessionData @@ -153,30 +322,19 @@ class SitesService { return updatedAt } - async getStagingUrl(sessionData: UserWithSiteSessionData): Promise { - // Check config.yml for staging url if it exists, and github site description otherwise - const { content: configData } = await this.configYmlService.read( - sessionData - ) - if ("staging" in configData) return configData.staging - - const { - description, - }: { description: string } = await this.gitHubService.getRepoInfo( - sessionData - ) - - if (description) { - // Retrieve the url from the description - repo descriptions have varying formats, so we look for the first link - const descTokens = description.replace("/;/g", " ").split(" ") - // Staging urls also contain staging in their url - const stagingUrl = descTokens.find( - (token) => token.includes("http") && token.includes("staging") + async getStagingUrl( + sessionData: UserWithSiteSessionData + ): Promise { + const siteUrls = await this.getUrlsOfSite(sessionData) + if (siteUrls instanceof NotFoundError) { + return new NotFoundError( + `${sessionData.siteName} does not have a staging url` ) - if (stagingUrl) return stagingUrl } - throw new NotFoundError(`${sessionData.siteName} has no staging url`) + const { staging } = siteUrls + + return staging } async create( @@ -194,6 +352,52 @@ class SitesService { where: { id: updateParams.id }, }) } + + async getSiteInfo( + sessionData: UserWithSiteSessionData + ): Promise { + const siteUrls = await this.getUrlsOfSite(sessionData) + if (siteUrls instanceof NotFoundError) { + return new UnprocessableError("Unable to retrieve site info") + } + const { staging: stagingUrl, prod: prodUrl } = siteUrls + + const stagingCommit = await this.gitHubService.getLatestCommitOfBranch( + sessionData, + "staging" + ) + + const prodCommit = await this.gitHubService.getLatestCommitOfBranch( + sessionData, + "master" + ) + + if ( + !this.isGitHubCommitData(stagingCommit) || + !this.isGitHubCommitData(prodCommit) + ) { + return new UnprocessableError("Unable to retrieve GitHub commit info") + } + + const { + author: { date: stagingDate }, + } = stagingCommit + const { + author: { date: prodDate }, + } = prodCommit + + const stagingAuthor = await this.getCommitAuthorEmail(stagingCommit) + const prodAuthor = await this.getMergeAuthorEmail(prodCommit) + + return { + savedAt: new Date(stagingDate).getTime() || 0, + savedBy: stagingAuthor || "Unknown Author", + publishedAt: new Date(prodDate).getTime() || 0, + publishedBy: prodAuthor || "Unknown Author", + stagingUrl: stagingUrl || "", + siteUrl: prodUrl || "", + } + } } export default SitesService diff --git a/src/services/identity/UsersService.ts b/src/services/identity/UsersService.ts index e06c0e1c7..1790c3ee5 100644 --- a/src/services/identity/UsersService.ts +++ b/src/services/identity/UsersService.ts @@ -47,6 +47,10 @@ class UsersService { this.whitelist = whitelist } + async findById(id: string) { + return this.repository.findOne({ where: { id } }) + } + async findByEmail(email: string) { return this.repository.findOne({ where: { email } }) } diff --git a/src/services/identity/__tests__/SitesService.spec.ts b/src/services/identity/__tests__/SitesService.spec.ts index 37becf0a6..acf8eed6b 100644 --- a/src/services/identity/__tests__/SitesService.spec.ts +++ b/src/services/identity/__tests__/SitesService.spec.ts @@ -1,6 +1,18 @@ import { ModelStatic } from "sequelize" -import { Site } from "@database/models" +import { Deployment, Site, User } from "@database/models" +import { + MOCK_COMMIT_MESSAGE_OBJECT_ONE, + MOCK_COMMIT_MESSAGE_OBJECT_TWO, + MOCK_GITHUB_NAME_ONE, + MOCK_GITHUB_NAME_TWO, + MOCK_GITHUB_EMAIL_ADDRESS_ONE, + MOCK_GITHUB_EMAIL_ADDRESS_TWO, + MOCK_GITHUB_DATE_ONE, + MOCK_GITHUB_DATE_TWO, + MOCK_COMMIT_MESSAGE_ONE, + MOCK_COMMIT_MESSAGE_TWO, +} from "@fixtures/identity" import { repoInfo, repoInfo2, @@ -11,10 +23,16 @@ import { mockUserWithSiteSessionData, mockSessionDataEmailUser, mockIsomerUserId, + mockEmail, + mockSessionDataEmailUserWithSite, } from "@fixtures/sessionData" import mockAxios from "@mocks/axios" import { NotFoundError } from "@root/errors/NotFoundError" +import { UnprocessableError } from "@root/errors/UnprocessableError" +import { GitHubCommitData } from "@root/types/commitData" +import { ConfigYmlData } from "@root/types/configYml" import type { RepositoryData } from "@root/types/repoInfo" +import { SiteInfo } from "@root/types/siteInfo" import { GitHubService } from "@services/db/GitHubService" import { ConfigYmlService } from "@services/fileServices/YmlFileServices/ConfigYmlService" import IsomerAdminsService from "@services/identity/IsomerAdminsService" @@ -28,6 +46,7 @@ const MockRepository = { const MockGithubService = { checkHasAccess: jest.fn(), + getLatestCommitOfBranch: jest.fn(), getRepoInfo: jest.fn(), } @@ -36,6 +55,7 @@ const MockConfigYmlService = { } const MockUsersService = { + findById: jest.fn(), findSitesByUserId: jest.fn(), } @@ -71,7 +91,7 @@ describe("SitesService", () => { it("should call the findOne method of the db model to get the siteName", async () => { // Arrange const expected = mockSite - MockRepository.findOne.mockResolvedValue(mockSite) + MockRepository.findOne.mockResolvedValueOnce(mockSite) // Act const actual = await SitesService.getBySiteName(mockSiteName) @@ -86,6 +106,194 @@ describe("SitesService", () => { }) }) + describe("getCommitAuthorEmail", () => { + it("should return the email of the commit author who is an email login user", async () => { + // Arrange + const expected = mockEmail + const commit: GitHubCommitData = { + author: { + name: MOCK_GITHUB_NAME_ONE, + email: MOCK_GITHUB_EMAIL_ADDRESS_ONE, + date: MOCK_GITHUB_DATE_ONE, + }, + message: JSON.stringify(MOCK_COMMIT_MESSAGE_OBJECT_ONE), + } + MockUsersService.findById.mockResolvedValueOnce(mockSessionDataEmailUser) + + // Act + const actual = await SitesService.getCommitAuthorEmail(commit) + + // Assert + expect(actual).toBe(expected) + expect(MockUsersService.findById).toHaveBeenCalledWith(mockIsomerUserId) + }) + + it("should return the email of the commit author who is a GitHub login user", async () => { + // Arrange + const expected = MOCK_GITHUB_EMAIL_ADDRESS_ONE + const commit: GitHubCommitData = { + author: { + name: MOCK_GITHUB_NAME_ONE, + email: MOCK_GITHUB_EMAIL_ADDRESS_ONE, + date: MOCK_GITHUB_DATE_ONE, + }, + message: MOCK_COMMIT_MESSAGE_ONE, + } + + // Act + const actual = await SitesService.getCommitAuthorEmail(commit) + + // Assert + expect(actual).toBe(expected) + expect(MockUsersService.findById).not.toHaveBeenCalled() + }) + }) + + describe("getUrlsOfSite", () => { + const deployment: Partial = { + stagingUrl: "https://repo-deployment-staging.netlify.app", + productionUrl: "https://repo-deployment-prod.netlify.app", + } + const emptyDeployment: Partial = { + stagingUrl: "", + productionUrl: "", + } + const configYmlData: Partial = { + staging: "https://repo-configyml-staging.netlify.app", + prod: "https://repo-configyml-prod.netlify.app", + } + const emptyConfigYmlData: Partial = { + staging: "", + prod: "", + } + const gitHubUrls = { + staging: "https://repo-repoinfo-staging.netlify.app", + prod: "https://repo-repoinfo-prod.netlify.app", + } + const repoInfo: { description: string } = { + description: `Staging: ${gitHubUrls.staging} | Production: ${gitHubUrls.prod}`, + } + + it("should return the urls of the site from the deployments table", async () => { + // Arrange + const expected = { + staging: deployment.stagingUrl, + prod: deployment.productionUrl, + } + const mockSiteWithDeployment = { + ...mockSite, + deployment, + } + + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithDeployment) + + // Act + const actual = await SitesService.getUrlsOfSite( + mockSessionDataEmailUserWithSite + ) + + // Assert + expect(actual).toEqual(expected) + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockConfigYmlService.read).not.toHaveBeenCalled() + expect(MockGithubService.getRepoInfo).not.toHaveBeenCalled() + }) + + it("should return the urls of the site from the _config.yml file", async () => { + // Arrange + const expected = { + staging: configYmlData.staging, + prod: configYmlData.prod, + } + const mockSiteWithNullDeployment = { + ...mockSite, + deployment: { + ...emptyDeployment, + }, + } + + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithNullDeployment) + MockConfigYmlService.read.mockResolvedValueOnce({ + content: configYmlData, + }) + + // Act + const actual = await SitesService.getUrlsOfSite( + mockSessionDataEmailUserWithSite + ) + + // Assert + expect(actual).toEqual(expected) + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockConfigYmlService.read).toHaveBeenCalled() + expect(MockGithubService.getRepoInfo).not.toHaveBeenCalled() + }) + + it("should return the urls of the site from the GitHub repo description", async () => { + // Arrange + const expected = { + staging: gitHubUrls.staging, + prod: gitHubUrls.prod, + } + const mockSiteWithNullDeployment = { + ...mockSite, + deployment: { + ...emptyDeployment, + }, + } + + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithNullDeployment) + MockConfigYmlService.read.mockResolvedValueOnce({ + content: { + ...emptyConfigYmlData, + }, + }) + MockGithubService.getRepoInfo.mockResolvedValueOnce(repoInfo) + + // Act + const actual = await SitesService.getUrlsOfSite( + mockSessionDataEmailUserWithSite + ) + + // Assert + expect(actual).toEqual(expected) + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockConfigYmlService.read).toHaveBeenCalled() + expect(MockGithubService.getRepoInfo).toHaveBeenCalled() + }) + + it("should return a NotFoundError if all fails", async () => { + // Arrange + const mockSiteWithNullDeployment = { + ...mockSite, + deployment: { + ...emptyDeployment, + }, + } + + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithNullDeployment) + MockConfigYmlService.read.mockResolvedValueOnce({ + content: { + ...emptyConfigYmlData, + }, + }) + MockGithubService.getRepoInfo.mockResolvedValueOnce({ + description: "", + }) + + // Act + const actual = await SitesService.getUrlsOfSite( + mockSessionDataEmailUserWithSite + ) + + // Assert + expect(actual).toBeInstanceOf(NotFoundError) + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockConfigYmlService.read).toHaveBeenCalled() + expect(MockGithubService.getRepoInfo).toHaveBeenCalled() + }) + }) + describe("getSites", () => { it("Filters accessible sites for github user correctly", async () => { // Store the API key and set it later so that other tests are not affected @@ -243,7 +451,7 @@ describe("SitesService", () => { describe("getLastUpdated", () => { it("Checks when site was last updated", async () => { - MockGithubService.getRepoInfo.mockResolvedValue(repoInfo) + MockGithubService.getRepoInfo.mockResolvedValueOnce(repoInfo) await expect( SitesService.getLastUpdated(mockUserWithSiteSessionData) @@ -257,57 +465,239 @@ describe("SitesService", () => { describe("getStagingUrl", () => { const stagingUrl = "https://repo-staging.netlify.app" - it("Retrieves the staging url for a site from config if available with higher priority over the description", async () => { - MockConfigYmlService.read.mockResolvedValue({ - content: { - staging: stagingUrl, - }, - }) - MockGithubService.getRepoInfo.mockResolvedValue(repoInfo2) + const productionUrl = "https://repo-prod.netlify.app" - await expect( - SitesService.getStagingUrl(mockUserWithSiteSessionData) - ).resolves.toEqual(stagingUrl) + it("should return the staging URL if it is available", async () => { + // Arrange + const mockSiteWithDeployment = { + ...mockSite, + deployment: { stagingUrl, productionUrl }, + } - expect(MockConfigYmlService.read).toHaveBeenCalledWith( - mockUserWithSiteSessionData + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithDeployment) + + // Act + const actual = await SitesService.getStagingUrl( + mockSessionDataEmailUserWithSite ) + + // Assert + expect(actual).toEqual(stagingUrl) + expect(MockRepository.findOne).toHaveBeenCalled() }) - it("Retrieves the staging url for a site from repo info otherwise", async () => { - MockConfigYmlService.read.mockResolvedValue({ + + it("should return an error when the staging url for a repo is not found", async () => { + // Arrange + MockRepository.findOne.mockResolvedValueOnce(null) + MockConfigYmlService.read.mockResolvedValueOnce({ content: {}, }) - MockGithubService.getRepoInfo.mockResolvedValue(repoInfo) + MockGithubService.getRepoInfo.mockResolvedValueOnce({ + description: "", + }) + // Act await expect( SitesService.getStagingUrl(mockUserWithSiteSessionData) - ).resolves.toEqual(stagingUrl) + ).resolves.toBeInstanceOf(NotFoundError) - expect(MockConfigYmlService.read).toHaveBeenCalledWith( - mockUserWithSiteSessionData + // Assert + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockConfigYmlService.read).toHaveBeenCalled() + expect(MockGithubService.getRepoInfo).toHaveBeenCalled() + }) + }) + + describe("getSiteInfo", () => { + const stagingUrl = "https://repo-staging.netlify.app" + const productionUrl = "https://repo-prod.netlify.app" + const mockSiteWithDeployment = { + ...mockSite, + deployment: { + stagingUrl, + productionUrl, + }, + } + + it("should return the site info if authors are email login users", async () => { + // Arrange + const mockStagingCommit: GitHubCommitData = { + author: { + name: MOCK_GITHUB_NAME_ONE, + email: MOCK_GITHUB_EMAIL_ADDRESS_ONE, + date: MOCK_GITHUB_DATE_ONE, + }, + message: JSON.stringify(MOCK_COMMIT_MESSAGE_OBJECT_ONE), + } + const mockStagingCommitAuthor: Partial = { + email: MOCK_GITHUB_EMAIL_ADDRESS_ONE, + } + const mockProductionCommit: GitHubCommitData = { + author: { + name: MOCK_GITHUB_NAME_TWO, + email: MOCK_GITHUB_EMAIL_ADDRESS_TWO, + date: MOCK_GITHUB_DATE_TWO, + }, + message: JSON.stringify(MOCK_COMMIT_MESSAGE_OBJECT_TWO), + } + const mockProductionCommitAuthor: Partial = { + email: MOCK_GITHUB_EMAIL_ADDRESS_TWO, + } + const expected: SiteInfo = { + savedAt: new Date(MOCK_GITHUB_DATE_ONE).getTime(), + savedBy: MOCK_GITHUB_EMAIL_ADDRESS_ONE, + publishedAt: new Date(MOCK_GITHUB_DATE_TWO).getTime(), + publishedBy: MOCK_GITHUB_EMAIL_ADDRESS_TWO, + stagingUrl, + siteUrl: productionUrl, + } + + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithDeployment) + MockGithubService.getLatestCommitOfBranch.mockResolvedValueOnce( + mockStagingCommit ) - expect(MockGithubService.getRepoInfo).toHaveBeenCalledWith( - mockUserWithSiteSessionData + MockGithubService.getLatestCommitOfBranch.mockResolvedValueOnce( + mockProductionCommit ) + MockUsersService.findById.mockResolvedValueOnce(mockStagingCommitAuthor) + MockUsersService.findById.mockResolvedValueOnce( + mockProductionCommitAuthor + ) + + // Act + const actual = await SitesService.getSiteInfo( + mockSessionDataEmailUserWithSite + ) + + // Assert + expect(actual).toEqual(expected) + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockGithubService.getLatestCommitOfBranch).toHaveBeenCalledTimes(2) + expect(MockUsersService.findById).toHaveBeenCalled() }) - it("throws an error when the staging url for a repo is not found", async () => { - MockConfigYmlService.read.mockResolvedValue({ + + it("should return the site info if authors are GitHub login users", async () => { + // Arrange + const mockStagingCommit: GitHubCommitData = { + author: { + name: MOCK_GITHUB_NAME_ONE, + email: MOCK_GITHUB_EMAIL_ADDRESS_ONE, + date: MOCK_GITHUB_DATE_ONE, + }, + message: MOCK_COMMIT_MESSAGE_ONE, + } + const mockProductionCommit: GitHubCommitData = { + author: { + name: MOCK_GITHUB_NAME_TWO, + email: MOCK_GITHUB_EMAIL_ADDRESS_TWO, + date: MOCK_GITHUB_DATE_TWO, + }, + message: MOCK_COMMIT_MESSAGE_TWO, + } + const expected: SiteInfo = { + savedAt: new Date(MOCK_GITHUB_DATE_ONE).getTime(), + savedBy: MOCK_GITHUB_EMAIL_ADDRESS_ONE, + publishedAt: new Date(MOCK_GITHUB_DATE_TWO).getTime(), + publishedBy: MOCK_GITHUB_EMAIL_ADDRESS_TWO, + stagingUrl, + siteUrl: productionUrl, + } + + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithDeployment) + MockGithubService.getLatestCommitOfBranch.mockResolvedValueOnce( + mockStagingCommit + ) + MockGithubService.getLatestCommitOfBranch.mockResolvedValueOnce( + mockProductionCommit + ) + + // Act + const actual = await SitesService.getSiteInfo( + mockSessionDataEmailUserWithSite + ) + + // Assert + expect(actual).toEqual(expected) + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockGithubService.getLatestCommitOfBranch).toHaveBeenCalledTimes(2) + expect(MockUsersService.findById).not.toHaveBeenCalled() + }) + + it("should return UnprocessableError when the site is not found", async () => { + // Arrange + MockRepository.findOne.mockResolvedValueOnce(null) + MockConfigYmlService.read.mockResolvedValueOnce({ content: {}, }) - MockGithubService.getRepoInfo.mockResolvedValue({ - description: "edited description", + MockGithubService.getRepoInfo.mockResolvedValueOnce({ + description: "", }) + // Act await expect( - SitesService.getStagingUrl(mockUserWithSiteSessionData) - ).rejects.toThrowError(NotFoundError) + SitesService.getSiteInfo(mockSessionDataEmailUserWithSite) + ).resolves.toBeInstanceOf(UnprocessableError) - expect(MockConfigYmlService.read).toHaveBeenCalledWith( - mockUserWithSiteSessionData + // Assert + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockUsersService.findById).not.toHaveBeenCalled() + }) + + it("should return UnprocessableError when the GitHub commit is not found", async () => { + // Arrange + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithDeployment) + MockGithubService.getLatestCommitOfBranch.mockResolvedValueOnce(null) + MockGithubService.getLatestCommitOfBranch.mockResolvedValueOnce(null) + + // Act + await expect( + SitesService.getSiteInfo(mockSessionDataEmailUserWithSite) + ).resolves.toBeInstanceOf(UnprocessableError) + + // Assert + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockGithubService.getLatestCommitOfBranch).toHaveBeenCalledTimes(2) + expect(MockUsersService.findById).not.toHaveBeenCalled() + }) + + it("should return with unknown author when the GitHub commit is empty", async () => { + // Arrange + const expected: SiteInfo = { + savedAt: 0, + savedBy: "Unknown Author", + publishedAt: 0, + publishedBy: "Unknown Author", + stagingUrl, + siteUrl: productionUrl, + } + + const mockEmptyCommit: GitHubCommitData = { + author: { + name: "", + email: "", + date: "", + }, + message: "", + } + + MockRepository.findOne.mockResolvedValueOnce(mockSiteWithDeployment) + MockGithubService.getLatestCommitOfBranch.mockResolvedValueOnce( + mockEmptyCommit ) - expect(MockGithubService.getRepoInfo).toHaveBeenCalledWith( - mockUserWithSiteSessionData + MockGithubService.getLatestCommitOfBranch.mockResolvedValueOnce( + mockEmptyCommit + ) + + // Act + const actual = await SitesService.getSiteInfo( + mockSessionDataEmailUserWithSite ) + + // Assert + expect(actual).toEqual(expected) + expect(MockRepository.findOne).toHaveBeenCalled() + expect(MockGithubService.getLatestCommitOfBranch).toHaveBeenCalledTimes(2) + expect(MockUsersService.findById).not.toHaveBeenCalled() }) }) }) diff --git a/src/types/commitData.ts b/src/types/commitData.ts new file mode 100644 index 000000000..be07cc1a5 --- /dev/null +++ b/src/types/commitData.ts @@ -0,0 +1,8 @@ +export type GitHubCommitData = { + author: { + name: string + email: string + date: string + } + message: string +} diff --git a/src/types/configYml.ts b/src/types/configYml.ts new file mode 100644 index 000000000..3f48cfa76 --- /dev/null +++ b/src/types/configYml.ts @@ -0,0 +1,4 @@ +export type ConfigYmlData = { + staging?: string + prod?: string +} diff --git a/src/types/siteInfo.ts b/src/types/siteInfo.ts new file mode 100644 index 000000000..c0dadba6f --- /dev/null +++ b/src/types/siteInfo.ts @@ -0,0 +1,8 @@ +export type SiteInfo = { + savedAt: number + savedBy: string + publishedAt: number + publishedBy: string + stagingUrl: string + siteUrl: string +}