diff --git a/src/errors/ConfigParseError.ts b/src/errors/ConfigParseError.ts new file mode 100644 index 000000000..1175ea9b1 --- /dev/null +++ b/src/errors/ConfigParseError.ts @@ -0,0 +1,7 @@ +import { BaseIsomerError } from "@root/errors/BaseError" + +export default class ConfigParseError extends BaseIsomerError { + constructor(fileName: string) { + super(500, `The given file: ${fileName} was not a config file!`) + } +} diff --git a/src/errors/NetworkError.ts b/src/errors/NetworkError.ts new file mode 100644 index 000000000..9a1f569cd --- /dev/null +++ b/src/errors/NetworkError.ts @@ -0,0 +1,9 @@ +import { BaseIsomerError } from "@root/errors/BaseError" + +export default class NetworkError extends BaseIsomerError { + constructor( + message = "An error occurred with the network whilst processing your request. Please try again later." + ) { + super(500, message) + } +} diff --git a/src/errors/PageParseError.ts b/src/errors/PageParseError.ts new file mode 100644 index 000000000..aa2febf30 --- /dev/null +++ b/src/errors/PageParseError.ts @@ -0,0 +1,7 @@ +import { BaseIsomerError } from "@root/errors/BaseError" + +export default class PageParseError extends BaseIsomerError { + constructor(fileName: string) { + super(500, `The given file: ${fileName} was not a page!`) + } +} diff --git a/src/integration/NotificationOnEditHandler.spec.ts b/src/integration/NotificationOnEditHandler.spec.ts index 54fae472c..986181289 100644 --- a/src/integration/NotificationOnEditHandler.spec.ts +++ b/src/integration/NotificationOnEditHandler.spec.ts @@ -33,6 +33,7 @@ import { ResourcePageService } from "@root/services/fileServices/MdPageServices/ import { SubcollectionPageService } from "@root/services/fileServices/MdPageServices/SubcollectionPageService" import { UnlinkedPageService } from "@root/services/fileServices/MdPageServices/UnlinkedPageService" import { CollectionYmlService } from "@root/services/fileServices/YmlFileServices/CollectionYmlService" +import { ConfigService } from "@root/services/fileServices/YmlFileServices/ConfigService" import { FooterYmlService } from "@root/services/fileServices/YmlFileServices/FooterYmlService" import { GitHubService } from "@services/db/GitHubService" import * as ReviewApi from "@services/db/review" @@ -100,6 +101,7 @@ const pageService = new PageService({ unlinkedPageService, resourceRoomDirectoryService, }) +const configService = new ConfigService() const reviewRequestService = new ReviewRequestService( (mockGithubService as unknown) as typeof ReviewApi, User, @@ -107,7 +109,8 @@ const reviewRequestService = new ReviewRequestService( Reviewer, ReviewMeta, ReviewRequestView, - pageService + pageService, + configService ) const sitesService = new SitesService({ siteRepository: Site, diff --git a/src/integration/Notifications.spec.ts b/src/integration/Notifications.spec.ts index 74307e938..82e7be7da 100644 --- a/src/integration/Notifications.spec.ts +++ b/src/integration/Notifications.spec.ts @@ -31,7 +31,6 @@ import { } from "@root/fixtures/sessionData" import { getAuthorizationMiddleware } from "@root/middleware" import { NotificationsRouter as _NotificationsRouter } from "@root/routes/v2/authenticated/notifications" -import { SitesRouter as _SitesRouter } from "@root/routes/v2/authenticated/sites" import { genericGitHubAxiosInstance } from "@root/services/api/AxiosInstance" import { GitHubService } from "@root/services/db/GitHubService" import { BaseDirectoryService } from "@root/services/directoryServices/BaseDirectoryService" @@ -44,6 +43,7 @@ import { ResourcePageService } from "@root/services/fileServices/MdPageServices/ import { SubcollectionPageService } from "@root/services/fileServices/MdPageServices/SubcollectionPageService" import { UnlinkedPageService } from "@root/services/fileServices/MdPageServices/UnlinkedPageService" import { CollectionYmlService } from "@root/services/fileServices/YmlFileServices/CollectionYmlService" +import { ConfigService } from "@root/services/fileServices/YmlFileServices/ConfigService" import { ConfigYmlService } from "@root/services/fileServices/YmlFileServices/ConfigYmlService" import { FooterYmlService } from "@root/services/fileServices/YmlFileServices/FooterYmlService" import CollaboratorsService from "@root/services/identity/CollaboratorsService" @@ -101,6 +101,7 @@ const pageService = new PageService({ unlinkedPageService, resourceRoomDirectoryService, }) +const configService = new ConfigService() const reviewRequestService = new ReviewRequestService( (gitHubService as unknown) as typeof ReviewApi, User, @@ -108,7 +109,8 @@ const reviewRequestService = new ReviewRequestService( Reviewer, ReviewMeta, ReviewRequestView, - pageService + pageService, + configService ) const sitesService = new SitesService({ siteRepository: Site, diff --git a/src/integration/Reviews.spec.ts b/src/integration/Reviews.spec.ts index 4cb47120f..b2d1833cf 100644 --- a/src/integration/Reviews.spec.ts +++ b/src/integration/Reviews.spec.ts @@ -83,6 +83,7 @@ import { ResourcePageService } from "@root/services/fileServices/MdPageServices/ import { SubcollectionPageService } from "@root/services/fileServices/MdPageServices/SubcollectionPageService" import { UnlinkedPageService } from "@root/services/fileServices/MdPageServices/UnlinkedPageService" import { CollectionYmlService } from "@root/services/fileServices/YmlFileServices/CollectionYmlService" +import { ConfigService } from "@root/services/fileServices/YmlFileServices/ConfigService" import { FooterYmlService } from "@root/services/fileServices/YmlFileServices/FooterYmlService" import { ReviewRequestDto } from "@root/types/dto/review" import { GitHubService } from "@services/db/GitHubService" @@ -132,6 +133,7 @@ const pageService = new PageService({ unlinkedPageService, resourceRoomDirectoryService, }) +const configService = new ConfigService() const reviewRequestService = new ReviewRequestService( (gitHubService as unknown) as typeof ReviewApi, User, @@ -139,7 +141,8 @@ const reviewRequestService = new ReviewRequestService( Reviewer, ReviewMeta, ReviewRequestView, - pageService + pageService, + configService ) const sitesService = new SitesService({ siteRepository: Site, @@ -263,20 +266,20 @@ describe("Review Requests Integration Tests", () => { const expected = { items: [ { - type: ["page"], + type: "page", name: MOCK_GITHUB_FILENAME_ALPHA_ONE, path: [], stagingUrl: `${MOCK_DEPLOYMENT_DBENTRY_ONE.stagingUrl}${MOCK_PAGE_PERMALINK}`, - fileUrl: `${FRONTEND_URL}/sites/${MOCK_REPO_NAME_ONE}/homepage`, - lastEditedBy: MOCK_USER_EMAIL_TWO, // TODO: This should be MOCK_USER_EMAIL_ONE + cmsFileUrl: `${FRONTEND_URL}/sites/${MOCK_REPO_NAME_ONE}/homepage`, + lastEditedBy: MOCK_USER_EMAIL_TWO, lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), }, { - type: ["page"], + type: "page", name: MOCK_GITHUB_FILENAME_ALPHA_TWO, path: MOCK_GITHUB_FILEPATH_ALPHA_TWO.split("/").filter((x) => x), stagingUrl: `${MOCK_DEPLOYMENT_DBENTRY_ONE.stagingUrl}${MOCK_PAGE_PERMALINK}`, - fileUrl: `${FRONTEND_URL}/sites/${MOCK_REPO_NAME_ONE}/editPage/${MOCK_GITHUB_FILENAME_ALPHA_TWO}`, + cmsFileUrl: `${FRONTEND_URL}/sites/${MOCK_REPO_NAME_ONE}/editPage/${MOCK_GITHUB_FILENAME_ALPHA_TWO}`, lastEditedBy: MOCK_USER_EMAIL_TWO, lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), }, @@ -750,22 +753,22 @@ describe("Review Requests Integration Tests", () => { reviewRequestedTime: new Date(MOCK_GITHUB_DATE_ONE).getTime(), changedItems: [ { - type: ["page"], + type: "page", name: MOCK_GITHUB_FILENAME_ALPHA_ONE, path: [], - stagingUrl: `${MOCK_DEPLOYMENT_DBENTRY_ONE.stagingUrl}${MOCK_PAGE_PERMALINK}`, - fileUrl: `${FRONTEND_URL}/sites/${MOCK_REPO_NAME_ONE}/homepage`, - lastEditedBy: MOCK_USER_EMAIL_TWO, // TODO: This should be MOCK_USER_EMAIL_ONE + lastEditedBy: MOCK_USER_EMAIL_TWO, lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), + stagingUrl: `${MOCK_DEPLOYMENT_DBENTRY_ONE.stagingUrl}${MOCK_PAGE_PERMALINK}`, + cmsFileUrl: `${FRONTEND_URL}/sites/${MOCK_REPO_NAME_ONE}/homepage`, }, { - type: ["page"], + type: "page", name: MOCK_GITHUB_FILENAME_ALPHA_TWO, path: MOCK_GITHUB_FILEPATH_ALPHA_TWO.split("/").filter((x) => x), - stagingUrl: `${MOCK_DEPLOYMENT_DBENTRY_ONE.stagingUrl}${MOCK_PAGE_PERMALINK}`, - fileUrl: `${FRONTEND_URL}/sites/${MOCK_REPO_NAME_ONE}/editPage/${MOCK_GITHUB_FILENAME_ALPHA_TWO}`, lastEditedBy: MOCK_USER_EMAIL_TWO, lastEditedTime: new Date(MOCK_GITHUB_COMMIT_DATE_THREE).getTime(), + stagingUrl: `${MOCK_DEPLOYMENT_DBENTRY_ONE.stagingUrl}${MOCK_PAGE_PERMALINK}`, + cmsFileUrl: `${FRONTEND_URL}/sites/${MOCK_REPO_NAME_ONE}/editPage/${MOCK_GITHUB_FILENAME_ALPHA_TWO}`, }, ], } @@ -1480,7 +1483,6 @@ describe("Review Requests Integration Tests", () => { const actual = await request(app).post( `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` ) - console.log(actual.error) // Assert expect(actual.statusCode).toEqual(403) @@ -1498,7 +1500,6 @@ describe("Review Requests Integration Tests", () => { const actual = await request(app).post( `/${MOCK_REPO_NAME_ONE}/${MOCK_GITHUB_PULL_REQUEST_NUMBER}/approve` ) - console.log(actual.error) // Assert expect(actual.statusCode).toEqual(403) diff --git a/src/integration/Sites.spec.ts b/src/integration/Sites.spec.ts index f92795fcd..9252e47a8 100644 --- a/src/integration/Sites.spec.ts +++ b/src/integration/Sites.spec.ts @@ -34,6 +34,7 @@ import { ResourcePageService } from "@root/services/fileServices/MdPageServices/ import { SubcollectionPageService } from "@root/services/fileServices/MdPageServices/SubcollectionPageService" import { UnlinkedPageService } from "@root/services/fileServices/MdPageServices/UnlinkedPageService" import { CollectionYmlService } from "@root/services/fileServices/YmlFileServices/CollectionYmlService" +import { ConfigService } from "@root/services/fileServices/YmlFileServices/ConfigService" import { ConfigYmlService } from "@root/services/fileServices/YmlFileServices/ConfigYmlService" import { FooterYmlService } from "@root/services/fileServices/YmlFileServices/FooterYmlService" import IsomerAdminsService from "@root/services/identity/IsomerAdminsService" @@ -89,6 +90,7 @@ const pageService = new PageService({ unlinkedPageService, resourceRoomDirectoryService, }) +const configService = new ConfigService() const reviewRequestService = new ReviewRequestService( gitHubService, User, @@ -96,7 +98,8 @@ const reviewRequestService = new ReviewRequestService( Reviewer, ReviewMeta, ReviewRequestView, - pageService + pageService, + configService ) const sitesService = new SitesService({ siteRepository: Site, diff --git a/src/routes/v2/authenticated/__tests__/Sites.spec.ts b/src/routes/v2/authenticated/__tests__/Sites.spec.ts index c78675a1f..a29a1d189 100644 --- a/src/routes/v2/authenticated/__tests__/Sites.spec.ts +++ b/src/routes/v2/authenticated/__tests__/Sites.spec.ts @@ -1,5 +1,5 @@ import express from "express" -import { ok, okAsync } from "neverthrow" +import { okAsync } from "neverthrow" import request from "supertest" import type { AuthorizationMiddleware } from "@middleware/authorization" @@ -99,7 +99,7 @@ describe("Sites Router", () => { describe("getStagingUrl", () => { it("returns the site's staging URL", async () => { const stagingUrl = "staging-url" - mockSitesService.getStagingUrl.mockResolvedValueOnce(ok(stagingUrl)) + mockSitesService.getStagingUrl.mockReturnValueOnce(okAsync(stagingUrl)) const resp = await request(app) .get(`/${mockSiteName}/stagingUrl`) @@ -138,7 +138,7 @@ describe("Sites Router", () => { stagingUrl: "staging-url", siteUrl: "prod-url", } - mockSitesService.getSiteInfo.mockResolvedValueOnce(ok(siteInfo)) + mockSitesService.getSiteInfo.mockReturnValueOnce(okAsync(siteInfo)) const resp = await request(app).get(`/${mockSiteName}/info`).expect(200) diff --git a/src/routes/v2/authenticated/__tests__/review.spec.ts b/src/routes/v2/authenticated/__tests__/review.spec.ts index 1ec13d14a..82a35e0f9 100644 --- a/src/routes/v2/authenticated/__tests__/review.spec.ts +++ b/src/routes/v2/authenticated/__tests__/review.spec.ts @@ -148,10 +148,12 @@ describe("Review Requests Router", () => { // Arrange const mockFilesChanged = ["file1", "file2"] mockIdentityUsersService.getSiteMember.mockResolvedValueOnce("user") - mockReviewRequestService.compareDiff.mockResolvedValueOnce( - mockFilesChanged + mockReviewRequestService.compareDiff.mockReturnValueOnce( + okAsync(mockFilesChanged) ) mockSitesService.getBySiteName.mockResolvedValueOnce(ok(true)) + mockSitesService.getStagingUrl.mockReturnValueOnce(okAsync("staging-url")) + mockGithubService.getRepoInfo.mockResolvedValueOnce(true) // Act const response = await request(app).get("/mockSite/review/compare") diff --git a/src/routes/v2/authenticated/review.ts b/src/routes/v2/authenticated/review.ts index 53bb06397..e47ac34e2 100644 --- a/src/routes/v2/authenticated/review.ts +++ b/src/routes/v2/authenticated/review.ts @@ -1,7 +1,6 @@ import autoBind from "auto-bind" import express from "express" import _ from "lodash" -import { ResultAsync } from "neverthrow" import logger from "@logger/logger" @@ -15,6 +14,8 @@ import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" import { CollaboratorRoles, ReviewRequestStatus } from "@root/constants" import { SiteMember, User } from "@root/database/models" +import MissingSiteError from "@root/errors/MissingSiteError" +import { NotFoundError } from "@root/errors/NotFoundError" import { GitHubService } from "@root/services/db/GitHubService" import CollaboratorsService from "@root/services/identity/CollaboratorsService" import NotificationsService from "@root/services/identity/NotificationsService" @@ -25,10 +26,10 @@ import { ResponseErrorBody } from "@root/types/dto/error" import { CommentItem, DashboardReviewRequestDto, - EditedItemDto, UpdateReviewRequestDto, ReviewRequestDto, BlobDiffDto, + EditedItemDto, } from "@root/types/dto/review" import ReviewRequestService from "@services/review/ReviewRequestService" // eslint-disable-next-line import/prefer-default-export @@ -125,14 +126,18 @@ export class ReviewsRouter { return this.sitesService .getStagingUrl(userWithSiteSessionData) .andThen((stagingLink) => - ResultAsync.fromSafePromise( - this.reviewRequestService.compareDiff( - userWithSiteSessionData, - stagingLink - ) + this.reviewRequestService.compareDiff( + userWithSiteSessionData, + stagingLink ) ) .map((items) => res.status(200).json({ items })) + .mapErr((err) => { + if (err instanceof MissingSiteError || err instanceof NotFoundError) { + return res.status(404).json({ message: err.message }) + } + return res.status(500).json({ message: err.message }) + }) } createReviewRequest: RequestHandler< diff --git a/src/routes/v2/authenticated/sites.ts b/src/routes/v2/authenticated/sites.ts index 001f6de47..73958e243 100644 --- a/src/routes/v2/authenticated/sites.ts +++ b/src/routes/v2/authenticated/sites.ts @@ -11,6 +11,10 @@ import type UserSessionData from "@root/classes/UserSessionData" import { attachSiteHandler } from "@root/middleware" import { StatsMiddleware } from "@root/middleware/stats" import type { RequestHandler } from "@root/types" +import { ResponseErrorBody } from "@root/types/dto/error" +import { ProdPermalink, StagingPermalink } from "@root/types/pages" +import { RepositoryData } from "@root/types/repoInfo" +import { SiteInfo } from "@root/types/siteInfo" import type SitesService from "@services/identity/SitesService" type SitesRouterProps = { @@ -19,6 +23,7 @@ type SitesRouterProps = { statsMiddleware: StatsMiddleware } +// eslint-disable-next-line import/prefer-default-export export class SitesRouter { private readonly sitesService @@ -40,7 +45,7 @@ export class SitesRouter { getSites: RequestHandler< never, - unknown, + { siteNames: RepositoryData[] }, never, never, { userSessionData: UserSessionData } @@ -52,7 +57,7 @@ export class SitesRouter { getLastUpdated: RequestHandler< { siteName: string }, - unknown, + { lastUpdated: string }, never, never, { userWithSiteSessionData: UserWithSiteSessionData } @@ -66,26 +71,21 @@ export class SitesRouter { getStagingUrl: RequestHandler< { siteName: string }, - unknown, + { stagingUrl: StagingPermalink } | ResponseErrorBody, never, never, { userWithSiteSessionData: UserWithSiteSessionData } > = async (req, res) => { const { userWithSiteSessionData } = res.locals - const possibleStagingUrl = await this.sitesService.getStagingUrl( - userWithSiteSessionData - ) - - // Check for error and throw - if (possibleStagingUrl.isErr()) { - return res.status(404).json({ message: possibleStagingUrl.error.message }) - } - return res.status(200).json({ stagingUrl: possibleStagingUrl.value }) + return this.sitesService + .getStagingUrl(userWithSiteSessionData) + .map((stagingUrl) => res.status(200).json({ stagingUrl })) + .mapErr(({ message }) => res.status(400).json({ message })) } getSiteUrl: RequestHandler< { siteName: string }, - unknown, + { siteUrl: ProdPermalink } | ResponseErrorBody, never, never, { userWithSiteSessionData: UserWithSiteSessionData } @@ -99,22 +99,17 @@ export class SitesRouter { getSiteInfo: RequestHandler< { siteName: string }, - unknown, + SiteInfo | ResponseErrorBody, never, never, { userWithSiteSessionData: UserWithSiteSessionData } > = async (req, res) => { const { userWithSiteSessionData } = res.locals - const possibleSiteInfo = await this.sitesService.getSiteInfo( - userWithSiteSessionData - ) - - // Check for error and throw - if (possibleSiteInfo.isErr()) { - return res.status(400).json({ message: possibleSiteInfo.error.message }) - } - return res.status(200).json(possibleSiteInfo.value) + return this.sitesService + .getSiteInfo(userWithSiteSessionData) + .map((siteInfo) => res.status(200).json(siteInfo)) + .mapErr(({ message }) => res.status(400).json({ message })) } getRouter() { diff --git a/src/server.js b/src/server.js index baca6a64c..384095b16 100644 --- a/src/server.js +++ b/src/server.js @@ -70,6 +70,7 @@ import getAuthenticatedSubrouter from "./routes/v2/authenticated" import { ReviewsRouter } from "./routes/v2/authenticated/review" import getAuthenticatedSitesSubrouter from "./routes/v2/authenticatedSites" import { PageService } from "./services/fileServices/MdPageServices/PageService" +import { ConfigService } from "./services/fileServices/YmlFileServices/ConfigService" import CollaboratorsService from "./services/identity/CollaboratorsService" import LaunchClient from "./services/identity/LaunchClient" import LaunchesService from "./services/identity/LaunchesService" @@ -187,7 +188,8 @@ const reviewRequestService = new ReviewRequestService( Reviewer, ReviewMeta, ReviewRequestView, - pageService + pageService, + new ConfigService() ) const sitesService = new SitesService({ siteRepository: Site, diff --git a/src/services/fileServices/MdPageServices/PageService.ts b/src/services/fileServices/MdPageServices/PageService.ts index 1f34c1a19..9ef08f441 100644 --- a/src/services/fileServices/MdPageServices/PageService.ts +++ b/src/services/fileServices/MdPageServices/PageService.ts @@ -3,9 +3,9 @@ import { ok, err, Result, ResultAsync, okAsync, errAsync } from "neverthrow" import UserSessionData from "@root/classes/UserSessionData" import { CONTACT_US_FILENAME, HOMEPAGE_FILENAME } from "@root/constants" import { BaseIsomerError } from "@root/errors/BaseError" -import EmptyStringError from "@root/errors/EmptyStringError" import MissingResourceRoomError from "@root/errors/MissingResourceRoomError" import { NotFoundError } from "@root/errors/NotFoundError" +import PageParseError from "@root/errors/PageParseError" import { ResourceRoomDirectoryService } from "@root/services/directoryServices/ResourceRoomDirectoryService" import { CollectionPageName, @@ -17,7 +17,6 @@ import { StagingPermalink, SubcollectionPageName, UnlinkedPageName, - PathInfo, Homepage, PageInfo, ContactUsPage, @@ -25,6 +24,7 @@ import { SubcollectionPage, ResourceCategoryPage, UnlinkedPage, + PathInfo, } from "@root/types/pages" import { Brand } from "@root/types/util" @@ -116,35 +116,46 @@ export class PageService { * @param sessionData session credentials of the user */ parsePageName = ( - pageName: string, + pathInfo: PathInfo, sessionData: UserSessionData ): ResultAsync => - this.parseHomepage(pageName) + this.isMarkdownPage(pathInfo.name) + .andThen(() => this.parseHomepage(pathInfo)) // NOTE: Order is important as `contact-us` and unlinked pages // are both rooted at `/pages` - .orElse(() => this.parseContactUsPage(pageName)) - .orElse(() => this.parseUnlinkedPages(pageName)) + .orElse(() => this.parseContactUsPage(pathInfo)) + .orElse(() => this.parseUnlinkedPages(pathInfo)) .asyncAndThen(okAsync) // NOTE: We read the `_config.yml` to determine if it is a resource page. // If it is not, we assume that this is a collection page. // Because this method is invoked on existing file paths from the frontend, // this assumption will hold true. - .orElse(() => this.parseResourceRoomPage(pageName, sessionData)) - .orElse(() => this.parseCollectionPage(pageName)) + .orElse(() => this.parseResourceRoomPage(pathInfo, sessionData)) + .orElse(() => this.parseCollectionPage(pathInfo)) + + isMarkdownPage = (pageName: string): Result => { + if (pageName.endsWith(".md")) return ok(pageName) + return err(new PageParseError(pageName)) + } // NOTE: Collection pages can be nested in either a collection: a/collection // or within a sub-collection: a/sub/collection - private parseCollectionPage = ( - pageName: string - ): ResultAsync< + private parseCollectionPage = ({ + name, + path, + }: PathInfo): ResultAsync< CollectionPageName | SubcollectionPageName, - EmptyStringError | NotFoundError + NotFoundError > => - this.extractPathInfo(pageName).asyncAndThen(({ name, path }) => - path - .mapErr(() => new NotFoundError()) - .asyncAndThen((rawPath) => { - if (rawPath.length === 1 && !!rawPath[0]) { + path + .mapErr(() => new NotFoundError()) + .asyncAndThen( + (rawPath) => { + // NOTE: Only 2 levels of nesting + if (rawPath.length > 2) { + return errAsync(new NotFoundError()) + } + if (rawPath.length === 1) { return okAsync({ name: Brand.fromString(name), collection: rawPath[0], @@ -164,51 +175,45 @@ export class PageService { `Error when parsing path: ${rawPath}, please ensure that the file exists!` ) ) - }) - ) - - private parseHomepage = ( - pageName: string - ): Result => - this.extractPathInfo(pageName).andThen( - ({ name, path }) => { - if (path.isErr() && name === HOMEPAGE_FILENAME) { - return ok({ name: Brand.fromString(name), kind: "Homepage" }) } - return err(new NotFoundError()) - } - ) + ) + + private parseHomepage = ({ + name, + path, + }: PathInfo): Result => { + if (path.isErr() && name === HOMEPAGE_FILENAME) { + return ok({ name: Brand.fromString(name), kind: "Homepage" }) + } + return err(new NotFoundError()) + } // NOTE: The contact us page has a fixed structure // It needs to be rooted at `/pages/contact-us` - private parseContactUsPage = ( - pageName: string - ): Result => - this.extractPathInfo(pageName).andThen( - ({ name, path }) => { - if ( - path.isOk() && - path.value.pop() === "pages" && - name === CONTACT_US_FILENAME - ) { - return ok({ name: Brand.fromString(name), kind: "ContactUsPage" }) - } - return err(new NotFoundError()) - } - ) + private parseContactUsPage = ({ + name, + path, + }: PathInfo): Result => { + if ( + path.isOk() && + path.value.at(-1) === "pages" && + name === CONTACT_US_FILENAME + ) { + return ok({ name: Brand.fromString(name), kind: "ContactUsPage" }) + } + return err(new NotFoundError()) + } - private parseUnlinkedPages = ( - pageName: string - ): Result => - this.extractPathInfo(pageName) - .andThen(({ path, name }) => - path - .map((rawPath) => rawPath.length === 1 && rawPath[0] === "pages") - .andThen((isPages) => - isPages - ? ok({ name: Brand.fromString(name), kind: "UnlinkedPage" }) - : err(new NotFoundError()) - ) + private parseUnlinkedPages = ({ + name, + path, + }: PathInfo): Result => + path + .map((rawPath) => rawPath.length === 1 && rawPath[0] === "pages") + .andThen((isPages) => + isPages + ? ok({ name: Brand.fromString(name), kind: "UnlinkedPage" }) + : err(new NotFoundError()) ) // NOTE: If there's no containing folder, it's not an unlinked page. .mapErr(() => new NotFoundError()) @@ -217,65 +222,65 @@ export class PageService { // The page can be nested 1 or 2 levels deep: // eg: one/level or two/levels/deep private parseResourceRoomPage = ( - pageName: string, + pathInfo: PathInfo, sessionData: UserSessionData ): ResultAsync< ResourceCategoryPageName, NotFoundError | MissingResourceRoomError > => this.extractResourceRoomName(sessionData).andThen((name) => - this.extractResourcePageName(name, pageName, sessionData) + this.extractResourcePageName(name, pathInfo, sessionData) ) private extractResourcePageName = ( resourceRoomName: ResourceRoomName, - pageName: string, + { name, path }: PathInfo, sessionData: UserSessionData ): ResultAsync => - this.extractPathInfo(pageName) - .asyncAndThen(({ name, path }) => - path.asyncAndThen( - (rawPath) => { - if (rawPath[0] !== resourceRoomName.name) { - return errAsync(new NotFoundError()) - } + path + .asyncAndThen((rawPath) => { + if (rawPath[0] !== resourceRoomName.name) { + return errAsync(new NotFoundError()) + } - if (rawPath.length !== 3 && rawPath.at(-1) !== "_posts") { - return errAsync(new NotFoundError()) - } + if (rawPath.length !== 3 && rawPath.at(-1) !== "_posts") { + return errAsync(new NotFoundError()) + } - // NOTE: We need to read the frontmatter and check the layout. - // The `layout` needs to be `post` for us to give a staging url - // as the others are either an ext link or a file. - // Because we only have the filename at this point, it is - // insufficient to use that to determine the resource type. - // This is because the actual underlying resource can be - // named totally differently from the containing github file. - return ResultAsync.fromPromise( - this.resourcePageService.read(sessionData, { - fileName: name, - resourceRoomName: resourceRoomName.name, - resourceCategoryName: rawPath[1], - }), - () => new NotFoundError() - ).andThen( - ({ content }) => { - if (content.frontMatter.layout !== "post") - return errAsync(new NotFoundError()) - return okAsync({ - name: Brand.fromString(name), - resourceRoom: resourceRoomName.name, - resourceCategory: rawPath[1], - kind: "ResourceCategoryPage", - }) - } + // NOTE: We need to read the frontmatter and check the layout. + // The `layout` needs to be `post` for us to give a staging url + // as the others are either an ext link or a file. + // Because we only have the filename at this point, it is + // insufficient to use that to determine the resource type. + // This is because the actual underlying resource can be + // named totally differently from the containing github file. + return ResultAsync.fromPromise( + this.resourcePageService.read(sessionData, { + fileName: name, + resourceRoomName: resourceRoomName.name, + resourceCategoryName: rawPath[1], + }), + () => new NotFoundError() + ).andThen(({ content }) => { + if (content.frontMatter.layout !== "post") + return errAsync( + new NotFoundError("Please ensure that the post exists!") ) - } - ) - ) + return okAsync({ + name: Brand.fromString(name), + resourceRoom: resourceRoomName.name, + resourceCategory: rawPath[1], + kind: "ResourceCategoryPage", + }) + }) + }) // NOTE: If we get an empty string as the `pageName`, // we just treat the file as not being found - .mapErr(() => new NotFoundError()) + .mapErr((e) => { + // NOTE: Done to preserve any existing error messages + if (e instanceof NotFoundError) return e + return new NotFoundError() + }) // NOTE: This is a safe wrapper over the js file for `getResourceRoomDirectoryName` extractResourceRoomName = ( @@ -298,30 +303,6 @@ export class PageService { : err(new MissingResourceRoomError()) ) - extractPathInfo = (pageName: string): Result => { - if (!pageName) { - return err(new EmptyStringError()) - } - - const fullPath = pageName.split("/") - // NOTE: Name is guaranteed to exist - // as this method only accepts a string - // and we've validated that the string is not empty - const name = fullPath.pop()! - - if (fullPath.length === 0) { - return ok({ - name, - path: err([]), - }) - } - - return ok({ - name, - path: ok(fullPath), - }) - } - retrieveCmsPermalink = ( pageName: PageName, siteName: string diff --git a/src/services/fileServices/MdPageServices/__tests__/PageService.spec.ts b/src/services/fileServices/MdPageServices/__tests__/PageService.spec.ts index 9d4268523..f43c65cd3 100644 --- a/src/services/fileServices/MdPageServices/__tests__/PageService.spec.ts +++ b/src/services/fileServices/MdPageServices/__tests__/PageService.spec.ts @@ -16,6 +16,7 @@ import { UnlinkedPageName, } from "@root/types/pages" import { Brand } from "@root/types/util" +import { extractPathInfo } from "@root/utils/files" import { ResourceRoomDirectoryService } from "@services/directoryServices/ResourceRoomDirectoryService" import { CollectionPageService } from "../CollectionPageService" @@ -88,7 +89,7 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( - "index.md", + { name: "index.md", path: err([]), __kind: "PathInfo" }, MOCK_USER_SESSION_DATA_ONE ) @@ -105,7 +106,7 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( - "pages/contact-us.md", + { name: CONTACT_US_FILENAME, path: ok(["pages"]), __kind: "PathInfo" }, MOCK_USER_SESSION_DATA_ONE ) @@ -115,14 +116,18 @@ describe("PageService", () => { it('should parse "contact-us.md" into an error', async () => { // Arrange - const expected = err(new NotFoundError()) + const expected = err( + new NotFoundError( + "Error when parsing path: , please ensure that the file exists!" + ) + ) mockResourceRoomDirectoryService.getResourceRoomDirectoryName.mockResolvedValueOnce( { resourceRoomName: MOCK_RESOURCE_ROOM_NAME } ) // Act const actual = await pageService.parsePageName( - CONTACT_US_FILENAME, + { name: CONTACT_US_FILENAME, path: ok([]), __kind: "PathInfo" }, MOCK_USER_SESSION_DATA_ONE ) @@ -139,7 +144,11 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( - `pages/${MOCK_UNLINKED_PAGE_NAME}`, + { + name: `${MOCK_UNLINKED_PAGE_NAME}`, + path: ok(["pages"]), + __kind: "PathInfo", + }, MOCK_USER_SESSION_DATA_ONE ) @@ -171,7 +180,15 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( - `${MOCK_RESOURCE_ROOM_NAME}/${MOCK_RESOURCE_CATEGORY_NAME}/_posts/${MOCK_UNLINKED_PAGE_NAME}`, + { + name: `${MOCK_UNLINKED_PAGE_NAME}`, + path: ok([ + MOCK_RESOURCE_ROOM_NAME, + MOCK_RESOURCE_CATEGORY_NAME, + "_posts", + ]), + __kind: "PathInfo", + }, MOCK_USER_SESSION_DATA_ONE ) @@ -182,15 +199,7 @@ describe("PageService", () => { it("should return `NotFoundError` if the `layout` of the resource item is not `post`", async () => { // Arrange - const MOCK_RESOURCE_ITEM = `${MOCK_RESOURCE_ROOM_NAME}/${MOCK_RESOURCE_CATEGORY_NAME}/_posts/${MOCK_UNLINKED_PAGE_NAME}` - const expected = err( - new NotFoundError( - `Error when parsing path: ${MOCK_RESOURCE_ITEM.split("/").slice( - 0, - -1 - )}, please ensure that the file exists!` - ) - ) + const expected = err(new NotFoundError()) mockResourceRoomDirectoryService.getResourceRoomDirectoryName.mockResolvedValueOnce( { resourceRoomName: MOCK_RESOURCE_ROOM_NAME } ) @@ -207,7 +216,15 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( - MOCK_RESOURCE_ITEM, + { + name: MOCK_UNLINKED_PAGE_NAME, + path: ok([ + MOCK_RESOURCE_ROOM_NAME, + MOCK_RESOURCE_CATEGORY_NAME, + "_posts", + ]), + __kind: "PathInfo", + }, MOCK_USER_SESSION_DATA_ONE ) @@ -229,7 +246,11 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( - `${MOCK_COLLECTION_NAME}/${MOCK_UNLINKED_PAGE_NAME}`, + { + name: MOCK_UNLINKED_PAGE_NAME, + path: ok([MOCK_COLLECTION_NAME]), + __kind: "PathInfo", + }, MOCK_USER_SESSION_DATA_ONE ) @@ -252,7 +273,11 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( - `${MOCK_COLLECTION_NAME}/${MOCK_SUBCOLLECTION_NAME}/${MOCK_UNLINKED_PAGE_NAME}`, + { + name: MOCK_UNLINKED_PAGE_NAME, + path: ok([MOCK_COLLECTION_NAME, MOCK_SUBCOLLECTION_NAME]), + __kind: "PathInfo", + }, MOCK_USER_SESSION_DATA_ONE ) @@ -263,11 +288,7 @@ describe("PageService", () => { it("should parse two level filepaths including 'index.md' to `NotFoundError`", async () => { // Arrange - const expected = err( - new NotFoundError( - `Error when parsing path: , please ensure that the file exists!` - ) - ) + const expected = err(new NotFoundError()) mockResourceRoomDirectoryService.getResourceRoomDirectoryName.mockResolvedValueOnce( { resourceRoomName: MOCK_RESOURCE_ROOM_NAME } ) @@ -275,25 +296,7 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( // NOTE: Extra front slash - `/${HOMEPAGE_FILENAME}`, - MOCK_USER_SESSION_DATA_ONE - ) - - // Assert - expect(actual).toEqual(expected) - expect(mockResourcePageService.read).toBeCalled() - }) - - it("should parse empty strings into `EmptyStringError`", async () => { - // Arrange - const expected = err(new EmptyStringError()) - mockResourceRoomDirectoryService.getResourceRoomDirectoryName.mockResolvedValueOnce( - { resourceRoomName: MOCK_RESOURCE_ROOM_NAME } - ) - - // Act - const actual = await pageService.parsePageName( - "", + { name: `/${HOMEPAGE_FILENAME}`, path: err([]), __kind: "PathInfo" }, MOCK_USER_SESSION_DATA_ONE ) @@ -304,18 +307,14 @@ describe("PageService", () => { it("should parse `/` into `NotFoundError`", async () => { // Arrange - const expected = err( - new NotFoundError( - "Error when parsing path: , please ensure that the file exists!" - ) - ) + const expected = err(new NotFoundError()) mockResourceRoomDirectoryService.getResourceRoomDirectoryName.mockResolvedValueOnce( { resourceRoomName: MOCK_RESOURCE_ROOM_NAME } ) // Act const actual = await pageService.parsePageName( - "/", + { name: "/", path: err([]), __kind: "PathInfo" }, MOCK_USER_SESSION_DATA_ONE ) @@ -333,7 +332,7 @@ describe("PageService", () => { // Act const actual = await pageService.parsePageName( - "gibberish", + { name: "gibberish", path: err([]), __kind: "PathInfo" }, MOCK_USER_SESSION_DATA_ONE ) @@ -411,10 +410,11 @@ describe("PageService", () => { const expected = ok({ name: MOCK_UNLINKED_PAGE_NAME, path: ok([MOCK_RESOURCE_ROOM_NAME, MOCK_RESOURCE_CATEGORY_NAME]), + __kind: "PathInfo", }) // Act - const actual = pageService.extractPathInfo( + const actual = extractPathInfo( `${MOCK_RESOURCE_ROOM_NAME}/${MOCK_RESOURCE_CATEGORY_NAME}/${MOCK_UNLINKED_PAGE_NAME}` ) @@ -427,10 +427,11 @@ describe("PageService", () => { const expected = ok({ name: MOCK_UNLINKED_PAGE_NAME, path: err([]), + __kind: "PathInfo", }) // Act - const actual = pageService.extractPathInfo(`${MOCK_UNLINKED_PAGE_NAME}`) + const actual = extractPathInfo(`${MOCK_UNLINKED_PAGE_NAME}`) // Assert expect(actual).toStrictEqual(expected) @@ -441,10 +442,11 @@ describe("PageService", () => { const expected = ok({ name: "", path: ok([MOCK_RESOURCE_ROOM_NAME]), + __kind: "PathInfo", }) // Act - const actual = pageService.extractPathInfo(`${MOCK_RESOURCE_ROOM_NAME}/`) + const actual = extractPathInfo(`${MOCK_RESOURCE_ROOM_NAME}/`) // Assert expect(actual).toStrictEqual(expected) @@ -455,7 +457,7 @@ describe("PageService", () => { const expected = err(new EmptyStringError()) // Act - const actual = pageService.extractPathInfo("") + const actual = extractPathInfo("") // Assert expect(actual).toEqual(expected) diff --git a/src/services/fileServices/YmlFileServices/ConfigService.ts b/src/services/fileServices/YmlFileServices/ConfigService.ts new file mode 100644 index 000000000..48e41267f --- /dev/null +++ b/src/services/fileServices/YmlFileServices/ConfigService.ts @@ -0,0 +1,75 @@ +import { err, ok, Result } from "neverthrow" + +import ConfigParseError from "@root/errors/ConfigParseError" +import { + ConfigFileName, + HtmlConfigFileName, + YmlConfigFileName, +} from "@root/types/configYml" +import { PathInfo } from "@root/types/pages" +import { Brand } from "@root/types/util" + +// eslint-disable-next-line import/prefer-default-export +export class ConfigService { + isConfigFile = ( + pathInfo: PathInfo + ): Result => + // NOTE: A file can be a config file in a few scenarios: + // 1. The file's extension is `.yml` (given by jekyll) + // 2. The file's extension is `.html` + it's rooted in a custom folder + // The second condition could be made more specific (resource room) + // but we'd then have an additional dep + require a github api call + // so we will rely on a less specific condition for now. + { + if (pathInfo.name.endsWith(".yml")) + return ok(this.extractYmlConfigFileName(pathInfo)) + + return this.isHtmlConfigFile(pathInfo).map((htmlPathInfo) => + this.extractHtmlConfigFileName(htmlPathInfo) + ) + } + + private extractYmlConfigFileName = ({ + name: rawName, + path, + }: PathInfo): YmlConfigFileName => { + const name = Brand.fromString<"YmlConfig">(rawName) + return { + name, + path, + kind: "YmlConfig", + } + } + + private extractHtmlConfigFileName = ({ + name: rawName, + path, + }: PathInfo): HtmlConfigFileName => { + const name = Brand.fromString<"HtmlConfig">(rawName) + + return { + name, + path, + kind: "HtmlConfig", + } + } + + private isHtmlConfigFile = ( + pathInfo: PathInfo + ): Result => { + const isHtmlConfig = + pathInfo.name.endsWith("html") && + // NOTE: Must not be top-level html + // and the folder name cannot contain `_` + // as that implies it is a collection + // and collections should not contain a `html` file + pathInfo.path.isOk() && + pathInfo.path.value + .map((folder) => folder.includes("_")) + .every((hasUnderscore) => !hasUnderscore) + + if (isHtmlConfig) return ok(pathInfo) + const rawPath = pathInfo.path.unwrapOr([""]).join("/") + return err(new ConfigParseError(`${rawPath}/${pathInfo.name}`)) + } +} diff --git a/src/services/identity/SitesService.ts b/src/services/identity/SitesService.ts index 17a73c2fe..74306d887 100644 --- a/src/services/identity/SitesService.ts +++ b/src/services/identity/SitesService.ts @@ -491,16 +491,11 @@ class SitesService { ...partialSiteInfo, })) ) - .map((siteInfo) => - _.pick(siteInfo, [ - "savedAt", - "savedBy", - "publishedAt", - "publishedBy", - "siteUrl", - "stagingUrl", - ]) - ) + .map(({ stagingUrl, siteUrl, ...rest }) => ({ + ..._.pick(rest, ["savedAt", "savedBy", "publishedAt", "publishedBy"]), + siteUrl: Brand.fromString(siteUrl), + stagingUrl: Brand.fromString(stagingUrl), + })) } } diff --git a/src/services/review/ReviewRequestService.ts b/src/services/review/ReviewRequestService.ts index 950903388..6ee4a9c4e 100644 --- a/src/services/review/ReviewRequestService.ts +++ b/src/services/review/ReviewRequestService.ts @@ -1,10 +1,20 @@ import { AxiosResponse } from "axios" import _ from "lodash" -import { errAsync, okAsync, ResultAsync } from "neverthrow" -import { ModelStatic, Op } from "sequelize" +import { + err, + errAsync, + ok, + okAsync, + Result, + ResultAsync, + combine, +} from "neverthrow" +import { Op, ModelStatic } from "sequelize" import UserWithSiteSessionData from "@classes/UserWithSiteSessionData" +import { ALLOWED_FILE_EXTENSIONS } from "@utils/file-upload-utils" + import { Reviewer } from "@database/models/Reviewers" import { ReviewMeta } from "@database/models/ReviewMeta" import { ReviewRequest } from "@database/models/ReviewRequest" @@ -12,23 +22,53 @@ import { ReviewRequestStatus } from "@root/constants" import { Repo, ReviewRequestView } from "@root/database/models" import { Site } from "@root/database/models/Site" import { User } from "@root/database/models/User" +import { BaseIsomerError } from "@root/errors/BaseError" +import ConfigParseError from "@root/errors/ConfigParseError" +import DatabaseError from "@root/errors/DatabaseError" +import MissingResourceRoomError from "@root/errors/MissingResourceRoomError" +import NetworkError from "@root/errors/NetworkError" import { NotFoundError } from "@root/errors/NotFoundError" +import PageParseError from "@root/errors/PageParseError" import RequestNotFoundError from "@root/errors/RequestNotFoundError" import { + BaseEditedItemDto, CommentItem, DashboardReviewRequestDto, + EditedConfigDto, EditedItemDto, - FileType, + EditedMediaDto, + EditedPageDto, GithubCommentData, ReviewRequestDto, + WithEditMeta, } from "@root/types/dto/review" import { isIsomerError } from "@root/types/error" -import { Commit, fromGithubCommitMessage } from "@root/types/github" -import { StagingPermalink } from "@root/types/pages" +import { + Commit, + fromGithubCommitMessage, + RawFileChangeInfo, + ShaMappings, +} from "@root/types/github" +import { PathInfo, StagingPermalink } from "@root/types/pages" import { RequestChangeInfo } from "@root/types/review" +import { extractPathInfo, getFileExt } from "@root/utils/files" import * as ReviewApi from "@services/db/review" import { PageService } from "../fileServices/MdPageServices/PageService" +import { ConfigService } from "../fileServices/YmlFileServices/ConfigService" + +const injectDefaultEditMeta = ({ + path, + name, +}: BaseEditedItemDto): WithEditMeta => ({ + lastEditedBy: "Unknown", + lastEditedTime: 0, + type: "page", + cmsFileUrl: "", + stagingUrl: "", + path, + name, +}) /** * NOTE: This class does not belong as a subset of GitHub service. @@ -55,6 +95,8 @@ export default class ReviewRequestService { private readonly pageService: PageService + private readonly configService: ConfigService + constructor( apiService: typeof ReviewApi, users: ModelStatic, @@ -62,7 +104,8 @@ export default class ReviewRequestService { reviewers: ModelStatic, reviewMeta: ModelStatic, reviewRequestView: ModelStatic, - pageService: PageService + pageService: PageService, + configService: ConfigService ) { this.apiService = apiService this.users = users @@ -71,75 +114,160 @@ export default class ReviewRequestService { this.reviewMeta = reviewMeta this.reviewRequestView = reviewRequestView this.pageService = pageService + this.configService = configService } - compareDiff = async ( - sessionData: UserWithSiteSessionData, + compareDiff = ( + userWithSiteSessionData: UserWithSiteSessionData, stagingLink: StagingPermalink - ): Promise => { + ): ResultAsync[], NetworkError | DatabaseError> => + ResultAsync.fromPromise( + this.apiService.getCommitDiff(userWithSiteSessionData.siteName), + // TODO: Write a handler for github errors to our own internal errors + () => new NetworkError() + ) + .andThen(({ files, commits }) => + ResultAsync.fromPromise( + this.computeShaMappings(commits), + () => new DatabaseError() + ).map((mappings) => ({ mappings, files })) + ) + .andThen(({ mappings, files }) => + combine( + this.compareDiffWithMappings( + userWithSiteSessionData, + stagingLink, + files, + mappings + ).map((res) => + res + // NOTE: Need to type-hint so that we can recover + .map>(_.identity) + .orElse((baseItem) => okAsync(injectDefaultEditMeta(baseItem))) + ) + ) + ) + + private compareDiffWithMappings = ( + sessionData: UserWithSiteSessionData, + stagingLink: StagingPermalink, + files: RawFileChangeInfo[], + mappings: ShaMappings + ): ResultAsync, BaseEditedItemDto>[] => { // Step 1: Get the site name const { siteName } = sessionData - // Step 2: Get the list of changed files using Github's API - // Refer here for details; https://docs.github.com/en/rest/commits/commits#compare-two-commits - // Note that we need a triple dot (...) between base and head refs - const { files, commits } = await this.apiService.getCommitDiff(siteName) + return files.map(({ filename, contents_url: contents }) => { + const extractedPathResult = extractPathInfo(filename) + if (extractedPathResult.isErr()) { + return errAsync({ + name: "", + path: [], + type: "file", + }) + } - const mappings = await this.computeShaMappings(commits) + const pathInfo = extractedPathResult.value - return Promise.all( - files.map(async ({ filename, contents_url }) => { - const fullPath = filename.split("/") - const items = contents_url.split("?ref=") - // NOTE: We have to compute sha this way rather than - // taking the file sha. - // This is because the sha present on the file is - // a checksum of the files contents. - // And the actual commit sha is given by the ref param - const sha = items[items.length - 1] - const pageNameRes = this.pageService.parsePageName( - filename, - sessionData + return this.extractConfigInfo(pathInfo) + .orElse(() => this.extractMediaInfo(pathInfo)) + .asyncMap(async (item) => item) + .orElse(() => + this.extractPageInfo(pathInfo, sessionData, stagingLink, siteName) ) - const stagingUrl = await pageNameRes - .andThen((pageName) => - this.pageService.retrieveStagingPermalink( - sessionData, - stagingLink, - pageName - ) - ) - // NOTE: We ignore the errors and use a placeholder - .unwrapOr("") - const fileUrl = await pageNameRes - .andThen((pageName) => - this.pageService.retrieveCmsPermalink(pageName, siteName) - ) - .unwrapOr("") + .map>((item) => { + const items = contents.split("?ref=") + // NOTE: We have to compute sha this way rather than + // taking the file sha. + // This is because the sha present on the file is + // a checksum of the files contents. + // And the actual commit sha is given by the ref param + const hash = items[items.length - 1] + return { + ...item, + lastEditedBy: mappings[hash]?.author || "Unknown user", + lastEditedTime: mappings[hash]?.unixTime || 0, + } + }) - return { - type: this.computeFileType(filename), - // NOTE: The string is guaranteed to be non-empty - // and hence this should exist. - name: fullPath.pop()!, - // NOTE: pop alters in place - path: fullPath, - stagingUrl, - fileUrl, - lastEditedBy: mappings[sha]?.author || "Unknown user", - lastEditedTime: mappings[sha]?.unixTime || 0, - } + .orElse(() => { + const { path, name } = pathInfo + return errAsync({ + name, + path: path.unwrapOr([]), + type: "page", + }) + }) + }) + } + + extractPageInfo = ( + pathInfo: PathInfo, + sessionData: UserWithSiteSessionData, + stagingLink: StagingPermalink, + siteName: string + ): ResultAsync< + EditedPageDto, + BaseIsomerError | NotFoundError | MissingResourceRoomError + > => { + const { name, path } = pathInfo + return this.pageService + .parsePageName(pathInfo, sessionData) + .andThen((pageName) => + combine([ + this.pageService.retrieveCmsPermalink(pageName, siteName), + this.pageService.retrieveStagingPermalink( + sessionData, + stagingLink, + pageName + ), + ]) + ) + .map(([cmsFileUrl, stagingUrl]) => ({ + type: "page", + // NOTE: The string is guaranteed to be non-empty + // and hence this should exist. + name, + path: path.unwrapOr([]), + stagingUrl, + cmsFileUrl, + })) + } + + extractMediaInfo = ({ + name, + path, + }: PathInfo): Result => { + const fileExt = getFileExt(name) + if (ALLOWED_FILE_EXTENSIONS.includes(fileExt)) { + return ok({ + name, + path: path.unwrapOr([""]), + type: fileExt === "pdf" ? "file" : "image", }) - ) + } + + return err(new PageParseError(name)) } - // TODO - computeFileType = (filename: string): FileType[] => ["page"] + extractConfigInfo = ( + pathInfo: PathInfo + ): Result => + this.configService.isConfigFile(pathInfo).map(({ name, path }) => { + const isNav = + name === "navigation.yml" && + path.isOk() && + path.value.length === 1 && + path.value.at(0) === "_data" + return { + name, + path: path.unwrapOr([""]), + type: isNav ? "nav" : "setting", + } + }) - computeShaMappings = async ( - commits: Commit[] - ): Promise> => { - const mappings: Record = {} + computeShaMappings = async (commits: Commit[]): Promise => { + const mappings: ShaMappings = {} // NOTE: commits from github are capped at 300. // This implies that there might possibly be some files @@ -579,13 +707,15 @@ export default class ReviewRequestService { })) ) .andThen((rest) => - ResultAsync.fromPromise( - this.compareDiff(userWithSiteSessionData, stagingLink), - () => new NotFoundError() - ).map((changedItems) => ({ - ...rest, - changedItems, - })) + // Step 2: Get the list of changed files using Github's API + // Refer here for details; https://docs.github.com/en/rest/commits/commits#compare-two-commits + // Note that we need a triple dot (...) between base and head refs + this.compareDiff(userWithSiteSessionData, stagingLink).map( + (changedItems) => ({ + ...rest, + changedItems, + }) + ) ) } diff --git a/src/services/review/__tests__/ReviewRequestService.spec.ts b/src/services/review/__tests__/ReviewRequestService.spec.ts index 775179bd2..4b2184176 100644 --- a/src/services/review/__tests__/ReviewRequestService.spec.ts +++ b/src/services/review/__tests__/ReviewRequestService.spec.ts @@ -50,7 +50,8 @@ import { } from "@root/fixtures/review" import { mockUserWithSiteSessionData } from "@root/fixtures/sessionData" import { PageService } from "@root/services/fileServices/MdPageServices/PageService" -import { EditedItemDto, GithubCommentData } from "@root/types/dto/review" +import { ConfigService } from "@root/services/fileServices/YmlFileServices/ConfigService" +import { EditedPageDto, GithubCommentData } from "@root/types/dto/review" import { Commit } from "@root/types/github" import * as ReviewApi from "@services/db/review" import _ReviewRequestService from "@services/review/ReviewRequestService" @@ -58,7 +59,7 @@ import _ReviewRequestService from "@services/review/ReviewRequestService" const MockPageService: { [K in keyof PageService]: ReturnType } = { - extractPathInfo: jest.fn(), + isMarkdownPage: jest.fn(), extractResourceRoomName: jest.fn(), parsePageName: jest.fn(), retrieveStagingPermalink: jest.fn(), @@ -108,6 +109,10 @@ const MockReviewRequest = { save: jest.fn(), } +const MockConfigService = { + isConfigFile: jest.fn(), +} + const ReviewRequestService = new _ReviewRequestService( (MockReviewApi as unknown) as typeof ReviewApi, (MockUsersRepository as unknown) as ModelStatic, @@ -115,12 +120,12 @@ const ReviewRequestService = new _ReviewRequestService( (MockReviewersRepository as unknown) as ModelStatic, (MockReviewMetaRepository as unknown) as ModelStatic, (MockReviewRequestViewRepository as unknown) as ModelStatic, - (MockPageService as unknown) as PageService + (MockPageService as unknown) as PageService, + (MockConfigService as unknown) as ConfigService ) const SpyReviewRequestService = { computeCommentData: jest.spyOn(ReviewRequestService, "computeCommentData"), - computeFileType: jest.spyOn(ReviewRequestService, "computeFileType"), computeShaMappings: jest.spyOn(ReviewRequestService, "computeShaMappings"), getComments: jest.spyOn(ReviewRequestService, "getComments"), getReviewRequest: jest.spyOn(ReviewRequestService, "getReviewRequest"), @@ -140,26 +145,26 @@ describe("ReviewRequestService", () => { ], commits: [MOCK_PULL_REQUEST_COMMIT_ONE, MOCK_PULL_REQUEST_COMMIT_TWO], } - const expected = [ + const expected = ok([ { - type: ["page"], + type: "page", name: MOCK_PULL_REQUEST_FILE_FILENAME_ONE, path: [], - fileUrl: "www.google.com", + cmsFileUrl: "www.google.com", stagingUrl: "www.google.com", lastEditedBy: MOCK_GITHUB_NAME_ONE, lastEditedTime: new Date(MOCK_GITHUB_DATE_ONE).getTime(), }, { - type: ["page"], + type: "page", name: MOCK_PULL_REQUEST_FILE_FILENAME_TWO, path: MOCK_COMMIT_FILEPATH_TWO.split("/"), - fileUrl: "www.google.com", + cmsFileUrl: "www.google.com", stagingUrl: "www.google.com", lastEditedBy: MOCK_GITHUB_NAME_TWO, lastEditedTime: new Date(MOCK_GITHUB_DATE_TWO).getTime(), }, - ] + ]) MockReviewApi.getCommitDiff.mockResolvedValueOnce(mockCommitDiff) MockPageService.parsePageName.mockReturnValue(okAsync("mock page name")) MockPageService.retrieveStagingPermalink.mockReturnValue( @@ -168,6 +173,12 @@ describe("ReviewRequestService", () => { MockPageService.retrieveCmsPermalink.mockReturnValue( okAsync("www.google.com") ) + MockConfigService.isConfigFile.mockReturnValueOnce( + err("not a config file") + ) + MockConfigService.isConfigFile.mockReturnValueOnce( + err("not a config file") + ) // Act const actual = await ReviewRequestService.compareDiff( @@ -178,7 +189,6 @@ describe("ReviewRequestService", () => { // Assert expect(actual).toEqual(expected) expect(SpyReviewRequestService.computeShaMappings).toHaveBeenCalled() - expect(SpyReviewRequestService.computeFileType).toHaveBeenCalled() expect(MockPageService.retrieveStagingPermalink).toHaveBeenCalled() }) @@ -188,7 +198,7 @@ describe("ReviewRequestService", () => { files: [], commits: [], } - const expected: EditedItemDto[] = [] + const expected = ok([]) MockReviewApi.getCommitDiff.mockResolvedValueOnce(mockCommitDiff) // Act @@ -200,7 +210,6 @@ describe("ReviewRequestService", () => { // Assert expect(actual).toEqual(expected) expect(SpyReviewRequestService.computeShaMappings).toHaveBeenCalled() - expect(SpyReviewRequestService.computeFileType).not.toHaveBeenCalled() expect(MockPageService.retrieveStagingPermalink).not.toHaveBeenCalled() }) @@ -210,7 +219,7 @@ describe("ReviewRequestService", () => { files: [], commits: [MOCK_PULL_REQUEST_COMMIT_ONE, MOCK_PULL_REQUEST_COMMIT_TWO], } - const expected: EditedItemDto[] = [] + const expected = ok([]) MockReviewApi.getCommitDiff.mockResolvedValueOnce(mockCommitDiff) // Act @@ -222,25 +231,10 @@ describe("ReviewRequestService", () => { // Assert expect(actual).toEqual(expected) expect(SpyReviewRequestService.computeShaMappings).toHaveBeenCalled() - expect(SpyReviewRequestService.computeFileType).not.toHaveBeenCalled() expect(MockPageService.retrieveStagingPermalink).not.toHaveBeenCalled() }) }) - describe("computeFileType", () => { - // TODO - it("should return the correct file type", () => { - // Arrange - const expected = ["page"] - - // Act - const actual = ReviewRequestService.computeFileType("filename") - - // Assert - expect(actual).toEqual(expected) - }) - }) - describe("computeShaMappings", () => { it("should return the correct sha mappings for pure identity commits", async () => { // Arrange diff --git a/src/types/configYml.ts b/src/types/configYml.ts index 7621ca5e7..e294419de 100644 --- a/src/types/configYml.ts +++ b/src/types/configYml.ts @@ -1,6 +1,19 @@ -import { ProdPermalink, StagingPermalink } from "./pages" +import { PathInfo, ProdPermalink, StagingPermalink } from "./pages" +import { FileNameBrand } from "./util" export type ConfigYmlData = { staging?: StagingPermalink prod?: ProdPermalink } + +// TODO: make this type of same shape as the PageNames +// as PageNames lacks `path` property at present +export type YmlConfigFileName = FileNameBrand<"YmlConfig"> & { + path: PathInfo["path"] +} + +export type HtmlConfigFileName = FileNameBrand<"HtmlConfig"> & { + path: PathInfo["path"] +} + +export type ConfigFileName = YmlConfigFileName | HtmlConfigFileName diff --git a/src/types/dto/review.ts b/src/types/dto/review.ts index bc4d61f0d..0e5eb3e36 100644 --- a/src/types/dto/review.ts +++ b/src/types/dto/review.ts @@ -4,16 +4,33 @@ export type ReviewRequestStatus = "OPEN" | "APPROVED" | "MERGED" | "CLOSED" export type FileType = "page" | "nav" | "setting" | "file" | "image" -export interface EditedItemDto { - type: FileType[] +export interface BaseEditedItemDto { name: string path: string[] - stagingUrl: string - fileUrl: string + type: FileType +} + +export type WithEditMeta = T & { lastEditedBy: string lastEditedTime: number } +export interface EditedPageDto extends BaseEditedItemDto { + type: "page" + stagingUrl: string + cmsFileUrl: string +} + +export interface EditedConfigDto extends BaseEditedItemDto { + type: "nav" | "setting" +} + +export interface EditedMediaDto extends BaseEditedItemDto { + type: "file" | "image" +} + +export type EditedItemDto = EditedPageDto | EditedConfigDto | EditedMediaDto + export interface UserDto { email: string role: CollaboratorRoles @@ -40,7 +57,7 @@ export interface ReviewRequestDto { reviewers: string[] reviewRequestedTime: number status: ReviewRequestStatus - changedItems: EditedItemDto[] + changedItems: WithEditMeta[] } export interface UpdateReviewRequestDto { diff --git a/src/types/github.ts b/src/types/github.ts index 1f0c57038..dcef06954 100644 --- a/src/types/github.ts +++ b/src/types/github.ts @@ -1,6 +1,12 @@ // NOTE: Types here are with reference to: // https://docs.github.com/en/rest/commits/commits#compare-two-commits +export type GithubEditInfo = { + author: string + unixTime: number +} +export type Sha = string +export type ShaMappings = Record export type FileChangeStatus = | "added" | "removed" diff --git a/src/types/pages.ts b/src/types/pages.ts index c23fb5333..50f2bac71 100644 --- a/src/types/pages.ts +++ b/src/types/pages.ts @@ -1,52 +1,28 @@ import { Result } from "neverthrow" -import { Brand, ToBrand } from "./util" +import { Brand, FileNameBrand, ToBrand } from "./util" -type NameInfo = { - name: string - kind: string -} - -type PageBrand = ToBrand - -export type ResourceRoomName = { - name: string & { __kind: "ResourceRoomName" } - kind: "ResourceRoomName" -} +export type ResourceRoomName = FileNameBrand<"ResourceRoomName"> -export type SubcollectionPageName = { - name: string & { __kind: "SubcollectionPage" } +export type SubcollectionPageName = FileNameBrand<"SubcollectionPage"> & { collection: string subcollection: string kind: "SubcollectionPage" } -export type CollectionPageName = { - name: string & { __kind: "CollectionPage" } +export type CollectionPageName = FileNameBrand<"CollectionPage"> & { collection: string - kind: "CollectionPage" } -export type ContactUsPageName = { - name: string & { __kind: "ContactUsPage" } - kind: "ContactUsPage" -} +export type ContactUsPageName = FileNameBrand<"ContactUsPage"> -export type UnlinkedPageName = { - name: string & { __kind: "UnlinkedPage" } - kind: "UnlinkedPage" -} +export type UnlinkedPageName = FileNameBrand<"UnlinkedPage"> -export type HomepageName = { - name: string & { __kind: "Homepage" } - kind: "Homepage" -} +export type HomepageName = FileNameBrand<"Homepage"> -export type ResourceCategoryPageName = { - name: string & { __kind: "ResourceCategoryPage" } +export type ResourceCategoryPageName = FileNameBrand<"ResourceCategoryPage"> & { resourceRoom: ToBrand resourceCategory: string - kind: "ResourceCategoryPage" } export type PageName = @@ -84,25 +60,24 @@ export type ResourcePageInfo = PageInfo & { } } -// Homepage also export type ContactUsPage = Brand export type Homepage = Brand export type CollectionPage = PageInfo & { - fileName: PageBrand + fileName: CollectionPageName } export type SubcollectionPage = PageInfo & { - fileName: PageBrand + fileName: SubcollectionPageName } export type ResourceCategoryPage = PageInfo & { - fileName: PageBrand + fileName: ResourceCategoryPageName } export type UnlinkedPage = PageInfo & { - fileName: PageBrand + fileName: UnlinkedPageName } export type StagingPermalink = Brand @@ -112,7 +87,25 @@ export type ProdPermalink = Brand // also includes the respective base url in front. export type FullPermalink = StagingPermalink | ProdPermalink -export type PathInfo = { - name: string - path: Result -} +/** + * NOTE: We brand this to prevent ad-hoc creation + * so that it has to come from the method. + * + * The `path` represents the path prefix of a given page + * and the `name` is the page name itself. + * + * This means that, for example, our homepage would be + * `path: Err([])` as there is no actual path to speak of + * and the name is `name: index.md` + * + * For another page such as `contact-us`, + * the path would then be `/pages` and the + * name would be `contact-us.md` + */ +export type PathInfo = Brand< + { + name: string + path: Result + }, + "PathInfo" +> diff --git a/src/types/siteInfo.ts b/src/types/siteInfo.ts index c0dadba6f..ccb0634a2 100644 --- a/src/types/siteInfo.ts +++ b/src/types/siteInfo.ts @@ -1,8 +1,10 @@ +import { ProdPermalink, StagingPermalink } from "./pages" + export type SiteInfo = { savedAt: number savedBy: string publishedAt: number publishedBy: string - stagingUrl: string - siteUrl: string + stagingUrl: StagingPermalink + siteUrl: ProdPermalink } diff --git a/src/types/util.ts b/src/types/util.ts index 7ba118988..bedcdffdc 100644 --- a/src/types/util.ts +++ b/src/types/util.ts @@ -21,3 +21,8 @@ export const Brand = { toString: (branded: Brand): string => (branded as unknown) as string, } + +export type FileNameBrand = { + name: string & { __kind: T } + kind: T +} diff --git a/src/utils/file-upload-utils.js b/src/utils/file-upload-utils.js index d15b12383..6ff45738a 100644 --- a/src/utils/file-upload-utils.js +++ b/src/utils/file-upload-utils.js @@ -1,11 +1,11 @@ -import { config } from "@config/config" - -import logger from "@logger/logger" - const CloudmersiveVirusApiClient = require("cloudmersive-virus-api-client") const FileType = require("file-type") const isSvg = require("is-svg") +const { config } = require("@config/config") + +const logger = require("@logger/logger") + const { sanitizer } = require("@services/utilServices/Sanitizer") const CLOUDMERSIVE_API_KEY = config.get("cloudmersiveKey") diff --git a/src/utils/files.ts b/src/utils/files.ts index 91e5dab88..e5581e6d2 100644 --- a/src/utils/files.ts +++ b/src/utils/files.ts @@ -1,3 +1,36 @@ +import { Result, err, ok } from "neverthrow" + +import EmptyStringError from "@root/errors/EmptyStringError" +import { PathInfo } from "@root/types/pages" + export const getFileExt = (fileName: string): string => // NOTE: will never be `undefined` as `fileName` is guaranteed to be a string fileName.split(".").pop()! + +export const extractPathInfo = ( + pageName: string +): Result => { + if (!pageName) { + return err(new EmptyStringError()) + } + + const fullPath = pageName.split("/") + // NOTE: Name is guaranteed to exist + // as this method only accepts a string + // and we've validated that the string is not empty + const name = fullPath.pop()! + + if (fullPath.length === 0) { + return ok({ + name, + path: err([]), + __kind: "PathInfo", + }) + } + + return ok({ + name, + path: ok(fullPath), + __kind: "PathInfo", + }) +}