Skip to content

Commit

Permalink
feat: add collaborators statistics API endpoint (#520)
Browse files Browse the repository at this point in the history
* 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

* feat: add collaborators statistics API endpoint

* test: add unit tests for collaborators statistics

* fix: return 404 instead of throwing an exception

* tests: add test to check for 404 status
  • Loading branch information
dcshzj authored Oct 14, 2022
1 parent df76494 commit 7a98b34
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 3 deletions.
2 changes: 2 additions & 0 deletions src/constants/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,5 @@ export const ISOMER_ADMIN_REPOS = [
"infra",
"markdown-helper",
]

export const INACTIVE_USER_THRESHOLD_DAYS = 60
50 changes: 50 additions & 0 deletions src/routes/v2/authenticated/__tests__/collaborators.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe("Collaborator Router", () => {
delete: jest.fn(),
list: jest.fn(),
getRole: jest.fn(),
getStatistics: jest.fn(),
}
const mockAuthorizationMiddleware = {
verifySiteAdmin: jest.fn(),
Expand Down Expand Up @@ -50,6 +51,12 @@ describe("Collaborator Router", () => {
`/:siteName/collaborators/:userId`,
attachReadRouteHandlerWrapper(collaboratorsRouter.deleteCollaborator)
)
subrouter.get(
`/:siteName/collaborators/statistics`,
attachReadRouteHandlerWrapper(
collaboratorsRouter.getCollaboratorsStatistics
)
)

const app = generateRouter(subrouter)

Expand Down Expand Up @@ -176,4 +183,47 @@ describe("Collaborator Router", () => {
)
})
})

describe("get collaborators statistics", () => {
it("should get collaborators statistics", async () => {
// Arrange
const MOCK_COLLABORATORS_STATISTICS = {
total: 1,
inactive: 1,
}
mockCollaboratorsService.getStatistics.mockResolvedValue(
MOCK_COLLABORATORS_STATISTICS
)

// Act
const resp = await request(app)
.get(`/${mockSiteName}/collaborators/statistics`)
.expect(200)

// Assert
expect(resp.body).toStrictEqual(MOCK_COLLABORATORS_STATISTICS)
expect(mockCollaboratorsService.getStatistics).toHaveBeenCalledWith(
mockSiteName
)
})

it("should return 404 if a NotFoundError occurred", async () => {
// Arrange
const mockErrorMessage = "error"
mockCollaboratorsService.getStatistics.mockResolvedValue(
new NotFoundError(mockErrorMessage)
)

// Act
const resp = await request(app)
.get(`/${mockSiteName}/collaborators/statistics`)
.expect(404)

// Assert
expect(resp.body).toStrictEqual({ message: mockErrorMessage })
expect(mockCollaboratorsService.getStatistics).toHaveBeenCalledWith(
mockSiteName
)
})
})
})
23 changes: 23 additions & 0 deletions src/routes/v2/authenticated/collaborators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,23 @@ export class CollaboratorsRouter {
return res.status(200).json({ role })
}

getCollaboratorsStatistics: RequestHandler<
{ siteName: string },
unknown,
never,
never,
{ userWithSiteSessionData: UserWithSiteSessionData }
> = async (req, res) => {
const { siteName } = req.params
const statistics = await this.collaboratorsService.getStatistics(siteName)

// Check for error and throw
if (statistics instanceof BaseIsomerError) {
return res.status(404).json({ message: statistics.message })
}
return res.status(200).json(statistics)
}

getRouter() {
const router = express.Router({ mergeParams: true })
router.get(
Expand All @@ -129,6 +146,12 @@ export class CollaboratorsRouter {
this.authorizationMiddleware.verifySiteAdmin,
attachReadRouteHandlerWrapper(this.deleteCollaborator)
)
router.get(
"/statistics",
attachSiteHandler,
this.authorizationMiddleware.verifySiteMember,
attachReadRouteHandlerWrapper(this.getCollaboratorsStatistics)
)

return router
}
Expand Down
38 changes: 37 additions & 1 deletion src/services/identity/CollaboratorsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { ForbiddenError } from "@errors/ForbiddenError"
import { NotFoundError } from "@errors/NotFoundError"
import { UnprocessableError } from "@errors/UnprocessableError"

import { CollaboratorRoles } from "@constants/constants"
import {
CollaboratorRoles,
INACTIVE_USER_THRESHOLD_DAYS,
} from "@constants/constants"

import { Whitelist, User, Site, SiteMember } from "@database/models"
import { BadRequestError } from "@root/errors/BadRequestError"
Expand Down Expand Up @@ -235,6 +238,39 @@ class CollaboratorsService {

return (site?.site_members?.[0]?.SiteMember?.role as string | null) ?? null
}

getStatistics = async (siteName: string) => {
const inactiveLimit = new Date()
inactiveLimit.setDate(
inactiveLimit.getDate() - INACTIVE_USER_THRESHOLD_DAYS
)
const site = await this.siteRepository.findOne({
where: { name: siteName },
include: [
{
model: User,
as: "site_members",
},
],
})

const collaborators = site?.site_members ?? []
const totalCount = collaborators.length

if (totalCount === 0) {
// Every site must have at least one collaborator
return new NotFoundError(`Site does not exist`)
}

const inactiveCount = collaborators.filter(
(collaborator) => collaborator.lastLoggedIn < inactiveLimit
).length

return {
total: totalCount,
inactive: inactiveCount,
}
}
}

export default CollaboratorsService
73 changes: 71 additions & 2 deletions src/services/identity/__tests__/CollaboratorsService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,18 @@ import { ForbiddenError } from "@errors/ForbiddenError"
import { NotFoundError } from "@errors/NotFoundError"
import { UnprocessableError } from "@errors/UnprocessableError"

import { Site, SiteMember, Whitelist } from "@database/models"
import { Site, SiteMember, User, Whitelist } from "@database/models"
import {
expectedSortedMockCollaboratorsList,
mockSiteOrmResponseWithAllCollaborators,
mockSiteOrmResponseWithOneAdminCollaborator,
mockSiteOrmResponseWithOneContributorCollaborator,
mockSiteOrmResponseWithNoCollaborators,
} from "@fixtures/identity"
import { CollaboratorRoles } from "@root/constants"
import {
CollaboratorRoles,
INACTIVE_USER_THRESHOLD_DAYS,
} from "@root/constants"
import { BadRequestError } from "@root/errors/BadRequestError"
import { ConflictError } from "@root/errors/ConflictError"
import CollaboratorsService from "@services/identity/CollaboratorsService"
Expand Down Expand Up @@ -473,4 +476,70 @@ describe("CollaboratorsService", () => {
expect(resp instanceof UnprocessableError).toBe(true)
})
})

describe("getStatistics", () => {
const inactiveDate = new Date()
inactiveDate.setDate(
inactiveDate.getDate() - INACTIVE_USER_THRESHOLD_DAYS - 1
)
const mockActiveCollaborator: Partial<User> = {
lastLoggedIn: new Date(),
}
const mockInactiveCollaborator: Partial<User> = {
lastLoggedIn: inactiveDate,
}

it("should return non-zero collaborators statistics", async () => {
// Arrange
const expected = {
total: 2,
inactive: 1,
}
mockSiteRepo.findOne.mockResolvedValue({
site_members: [mockActiveCollaborator, mockInactiveCollaborator],
})

// Act
const actual = await collaboratorsService.getStatistics(mockSiteName)

// Assert
expect(actual).toEqual(expected)
expect(mockSiteRepo.findOne).toBeCalled()
})

it("should return zero inactive collaborators statistics if there is none", async () => {
// Arrange
const expected = {
total: 1,
inactive: 0,
}
mockSiteRepo.findOne.mockResolvedValue({
site_members: [mockActiveCollaborator],
})

// Act
const actual = await collaboratorsService.getStatistics(mockSiteName)

// Assert
expect(actual).toEqual(expected)
expect(mockSiteRepo.findOne).toBeCalled()
})

it("should return NotFoundError if site is not found", async () => {
// Arrange
const expected = {
total: 0,
inactive: 0,
}
mockSiteRepo.findOne.mockResolvedValue(null)

// Act
await expect(
collaboratorsService.getStatistics(mockSiteName)
).resolves.toBeInstanceOf(NotFoundError)

// Assert
expect(mockSiteRepo.findOne).toBeCalled()
})
})
})

0 comments on commit 7a98b34

Please sign in to comment.