-
Notifications
You must be signed in to change notification settings - Fork 33
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: refactoring getNextProject (#608)
* feat: refactoring getNextProject * improving comments
- Loading branch information
1 parent
5c84ce7
commit 6f1788c
Showing
8 changed files
with
168 additions
and
65 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
95 changes: 72 additions & 23 deletions
95
packages/database/src/entitiesUtils/Judge/getNextProject.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,82 @@ | ||
| import { | ||
| EntityManager, | ||
| FilterQuery, | ||
| FindOneOptions, | ||
| LockMode, | ||
| QueryOrder, | ||
| } from '@mikro-orm/postgresql'; | ||
| import seedrandom from 'seedrandom'; | ||
| import { EntityManager } from '@mikro-orm/postgresql'; | ||
| import { Project } from '../../entities/Project'; | ||
| import { ExpoJudgingVote } from '../../entities/ExpoJudgingVote'; | ||
| import { ExpoJudgingSession } from '../../entities/ExpoJudgingSession'; | ||
| import { Judge } from '../../entities/Judge'; | ||
|
|
||
| type GetNextProjectArgs = { | ||
| judge: Judge; | ||
| expoJudgingSession: ExpoJudgingSession; | ||
| excludedProjectIds: string[]; | ||
| entityManager: EntityManager; | ||
| }; | ||
|
|
||
| /** | ||
| * Fetches the next available project and appropriately increments related counters | ||
| * @param args | ||
| * | ||
| * The algorithm for selecting the next project is as follows: | ||
| * 1. Fetch all projects that the judge has not yet visited or skipped | ||
| * 2. Sort projects randomly (deterministically based on the judge's ID) | ||
| * 3. Sort projects by the number of times they have been visited | ||
| * 4. Sort projects by the number of active judges if they have the same number of visits | ||
| * 5. Load and return the first project from that sorted list | ||
| * @param args {@link GetNextProjectArgs} | ||
| * @returns A {@link Project} if one could be found | ||
| */ | ||
| export const getNextProject = async ({ | ||
| judge, | ||
| expoJudgingSession, | ||
| excludedProjectIds, | ||
| entityManager, | ||
| }: { | ||
| excludedProjectIds: string[]; | ||
| entityManager: EntityManager; | ||
| }): Promise<Project | null> => { | ||
| // Don't visit projects the judge has already visited or skipped | ||
| const query: FilterQuery<Project> = { id: { $nin: excludedProjectIds } }; | ||
|
|
||
| // Find the project with the fewest judgeVisits and the lowest activeJudgeCount | ||
| const queryOptions: FindOneOptions<Project> = { | ||
| orderBy: { judgeVisits: QueryOrder.ASC, activeJudgeCount: QueryOrder.ASC }, | ||
| lockMode: LockMode.PESSIMISTIC_READ, // Don't skip locked projects | ||
| }; | ||
|
|
||
| const project = await entityManager.findOne(Project, query, queryOptions); | ||
| return project; | ||
| }: GetNextProjectArgs): Promise<Project | null> => { | ||
| const [projects, votesForSession] = await Promise.all([ | ||
| entityManager.find( | ||
| Project, | ||
| { id: { $nin: excludedProjectIds } }, // Don't visit projects the judge has already visited or skipped | ||
| { fields: ['id', 'activeJudgeCount'] }, // Only pull the fields we need | ||
| ), | ||
| // Fetch all relevant votes to figure out which teams have been visited the most | ||
| entityManager.find( | ||
| ExpoJudgingVote, | ||
| { judgingSession: expoJudgingSession.id }, | ||
| { fields: ['currentProject.id', 'previousProject.id'] }, | ||
| ), | ||
| ]); | ||
|
|
||
| if (projects.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| // Iterate over the votes and add a counter for each project | ||
| const voteCounts: { [id: string]: number } = {}; | ||
| for (let i = 0; i < votesForSession.length; i += 1) { | ||
| const vote = votesForSession[i] as (typeof votesForSession)[0]; | ||
| if (vote.currentProject) { | ||
| voteCounts[vote.currentProject.$.id] = (voteCounts[vote.currentProject.$.id] ?? 0) + 1; | ||
| } | ||
| if (vote.previousProject) { | ||
| voteCounts[vote.previousProject.$.id] = (voteCounts[vote.previousProject.$.id] ?? 0) + 1; | ||
| } | ||
| } | ||
|
|
||
| const random = seedrandom(judge.id); // Create a deterministic random number generator based on the judge's ID | ||
| const sortedProjects = projects | ||
| .sort(() => (random() > 0.5 ? -1 : 1)) // Shuffle the order of the projects as the baseline | ||
| .sort((a, b) => { | ||
| if (voteCounts[a.id] === voteCounts[b.id]) { | ||
| // Teams have the same vote count; sort by active judge count | ||
| return a.activeJudgeCount - b.activeJudgeCount; | ||
| } | ||
| return (voteCounts[a.id] ?? 0) - (voteCounts[b.id] ?? 0); | ||
| }); | ||
|
|
||
| const nextProject = sortedProjects[0] as (typeof sortedProjects)[0]; // Assert the type; it WILL exist due to the empty array check above | ||
|
|
||
| // Load the entire project instead of just the fields we needed | ||
| const loadedProject = await entityManager.findOneOrFail(Project, { | ||
| id: nextProject.id, | ||
| }); | ||
|
|
||
| return loadedProject; | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
100 changes: 90 additions & 10 deletions
100
packages/database/tests/entitiesUtils/Judge/getNextProject.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,101 @@ | ||
| import { Project } from '../../../src'; | ||
| import { ExpoJudgingVote, Project } from '../../../src'; | ||
| import { getNextProject } from '../../../src/entitiesUtils'; | ||
|
|
||
| const mockJudge = { | ||
| id: '123', | ||
| }; | ||
|
|
||
| const mockExpoJudgingSession = { | ||
| id: '2', | ||
| }; | ||
|
|
||
| const mockEntityManager = { | ||
| find: jest.fn().mockResolvedValue([]), | ||
| findOneOrFail: jest.fn(), | ||
| }; | ||
|
|
||
| describe('getNextProject', () => { | ||
| it('queries for a project with the correct values', async () => { | ||
| const mockEntityManager = { | ||
| findOne: jest.fn(), | ||
| }; | ||
| const mockProjectData = [ | ||
| { id: '1', activeJudgeCount: 0 }, // 1 vote | ||
| { id: '2', activeJudgeCount: 0 }, // 1 vote | ||
| { id: '3', activeJudgeCount: 1 }, // No votes | ||
| { id: '4', activeJudgeCount: 0 }, // No votes; should be next project | ||
| ] as const; | ||
|
|
||
| const mockVotes = [ | ||
| { | ||
| previousProject: { $: { id: mockProjectData[0].id } }, | ||
| currentProject: { $: { id: mockProjectData[1].id } }, | ||
| }, | ||
| ] as const; | ||
|
|
||
| const excludedProjectIds = ['1']; | ||
| await getNextProject({ entityManager: mockEntityManager as any, excludedProjectIds }); | ||
|
|
||
| expect(mockEntityManager.findOne).toHaveBeenCalledWith( | ||
| mockEntityManager.find | ||
| .mockResolvedValueOnce(mockProjectData) // Projects is the first call | ||
| .mockResolvedValueOnce(mockVotes); // Votes is the second call | ||
|
|
||
| await getNextProject({ | ||
| judge: mockJudge as any, | ||
| entityManager: mockEntityManager as any, | ||
| excludedProjectIds, | ||
| expoJudgingSession: mockExpoJudgingSession as any, | ||
| }); | ||
|
|
||
| expect(mockEntityManager.find).toHaveBeenCalledWith( | ||
| Project, | ||
| expect.objectContaining({ id: { $nin: excludedProjectIds } }), | ||
| expect.objectContaining({ | ||
| lockMode: 2, | ||
| orderBy: { judgeVisits: 'ASC', activeJudgeCount: 'ASC' }, | ||
| }), | ||
| expect.objectContaining({ fields: ['id', 'activeJudgeCount'] }), | ||
| ); | ||
|
|
||
| expect(mockEntityManager.find).toHaveBeenCalledWith( | ||
| ExpoJudgingVote, | ||
| expect.objectContaining({ judgingSession: mockExpoJudgingSession.id }), | ||
| expect.objectContaining({ fields: ['currentProject.id', 'previousProject.id'] }), | ||
| ); | ||
|
|
||
| expect(mockEntityManager.findOneOrFail).toHaveBeenCalledWith( | ||
| Project, | ||
| expect.objectContaining({ id: '4' }), // Project with the least votes and active judges | ||
| ); | ||
| }); | ||
|
|
||
| it('shuffles the projects and returns the first shuffled project when none have votes', async () => { | ||
| const mockProjectData = [ | ||
| { id: '1', activeJudgeCount: 0 }, | ||
| { id: '2', activeJudgeCount: 0 }, | ||
| { id: '3', activeJudgeCount: 0 }, | ||
| ] as const; | ||
|
|
||
| const mockVotes = [] as const; | ||
|
|
||
| const excludedProjectIds = ['1']; | ||
|
|
||
| mockEntityManager.find | ||
| .mockResolvedValueOnce(mockProjectData) // Projects is the first call | ||
| .mockResolvedValueOnce(mockVotes); // Votes is the second call | ||
|
|
||
| await getNextProject({ | ||
| judge: mockJudge as any, | ||
| entityManager: mockEntityManager as any, | ||
| excludedProjectIds, | ||
| expoJudgingSession: mockExpoJudgingSession as any, | ||
| }); | ||
|
|
||
| expect(mockEntityManager.findOneOrFail).toHaveBeenCalledWith( | ||
| Project, | ||
| expect.objectContaining({ id: '2' }), // Random project based on seed | ||
| ); | ||
| }); | ||
|
|
||
| it('returns null if no remaining projects exist', async () => { | ||
| const nextProject = await getNextProject({ | ||
| judge: mockJudge as any, | ||
| entityManager: mockEntityManager as any, | ||
| excludedProjectIds: [], | ||
| expoJudgingSession: mockExpoJudgingSession as any, | ||
| }); | ||
| expect(nextProject).toBe(null); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters