From 026af080a14ae9a0532b0011e5c04d9b1a1704c0 Mon Sep 17 00:00:00 2001 From: Matt Gabrenya Date: Thu, 8 Sep 2022 13:05:43 -0400 Subject: [PATCH 1/7] feat(api): before assigning praise, confirm that no quantifier is assigned the same reciever's praise multiple times --- packages/api/src/period/utils/assignment.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/api/src/period/utils/assignment.ts b/packages/api/src/period/utils/assignment.ts index 9931b6e6f..b52c29f44 100644 --- a/packages/api/src/period/utils/assignment.ts +++ b/packages/api/src/period/utils/assignment.ts @@ -221,6 +221,20 @@ const verifyAssignments = async ( `Not all redundant praise assignments accounted for: ${accountedPraiseCount} / ${expectedAccountedPraiseCount} expected in period` ); } + + const verifiedUniqueAssignments = assignments.poolAssignments.map( + (quantifier) => + quantifier.receivers.length === + new Set(quantifier.receivers.map((r) => r._id.toString())).size + ); + + if (every(verifiedUniqueAssignments)) { + logger.info('All redundant praise are assigned to unique quantifiers'); + } else { + throw new InternalServerError( + 'Some redundant praise are assigned to the same quantifier multiple times' + ); + } }; /** From bbd64048eb51de47e77f77b5841990217404a3d9 Mon Sep 17 00:00:00 2001 From: Matt Gabrenya Date: Thu, 8 Sep 2022 13:10:26 -0400 Subject: [PATCH 2/7] fix(api): refactor prepareAssignmentsEvenly to prevent a quantifier from being assigned to the same receiver multiple times --- packages/api/src/period/utils/assignment.ts | 45 +++++++++++++-------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/packages/api/src/period/utils/assignment.ts b/packages/api/src/period/utils/assignment.ts index b52c29f44..f5409c2e7 100644 --- a/packages/api/src/period/utils/assignment.ts +++ b/packages/api/src/period/utils/assignment.ts @@ -5,6 +5,7 @@ import intersection from 'lodash/intersection'; import range from 'lodash/range'; import sum from 'lodash/sum'; import zip from 'lodash/zip'; +import every from 'lodash/every'; import greedyPartitioning from 'greedy-number-partitioning'; import { InternalServerError, NotFoundError } from '@/error/errors'; import { Quantifier, QuantifierPoolById, Receiver } from '@/praise/types'; @@ -331,6 +332,20 @@ const prepareAssignmentsEvenly = async ( 'Unable to assign redudant quantifications without more members in quantifier pool' ); + // Check that the number of redundant assignments is greater than to the number of receivers + // otherwise a quantifier could be assigned the same praise multiple times + if (PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER > receivers.length) + throw new Error( + 'Quantifiers per Receiver is too large for the number of receivers, unable to prevent duplicate assignments' + ); + + // Run "Greedy number partitioning" algorithm: + greedyPartitioning( + receivers, // Items to place in bins + quantifierPool.length, // Available bins + (r: Receiver) => r.praiseCount // Bin space taken by each item + ); + const redundantReceiversShuffled: Receiver[][] = zip( ...range(PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER).map((i) => { // Create a "rotated" copy of array for each redundant quantification @@ -338,7 +353,12 @@ const prepareAssignmentsEvenly = async ( // ensure each rotation does not overlap const receiversShuffledClone = [...receivers]; - range(i).forEach(() => { + const redundantAssignmentBins: Receiver[][] = zip( + ...range(PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER).map((rotations) => { + const receiversShuffledClone = [...receiversDistributedByPraiseCount]; + + // "Rotate" array back-to-front (i.e. [a,b,c,d] -> [d,a,b,c]) + range(rotations).forEach(() => { const lastElem = receiversShuffledClone.pop(); if (!lastElem) throw Error( @@ -350,24 +370,15 @@ const prepareAssignmentsEvenly = async ( return receiversShuffledClone; }) - ) as Receiver[][]; - - // Run "Greedy number partitioning" algorithm on list of receivers - // with a fixed 'bin' size of: quantifierPool.length - // where each item takes up bin space based on its praiseCount - const redundantAssignmentBins: Receiver[][][] = greedyPartitioning< - Receiver[] - >( - redundantReceiversShuffled, - quantifierPool.length, - (receivers: Receiver[]) => sum(receivers.map((r) => r.praiseCount)) - ); - - const redundantAssignmentBinsFlattened: Receiver[][] = - redundantAssignmentBins.map((binOfBins) => flatten(binOfBins)); + ) + .map((binOfBins) => + binOfBins.map((bins) => (bins === undefined ? ([] as Receiver[]) : bins)) + ) + .map((binOfBins) => flatten(binOfBins)); + // Randomly assign each quantifier to an array of unique receivers const assignments = generateAssignments( - redundantAssignmentBinsFlattened, + redundantAssignmentBins, quantifierPool ); From 191ddab1a1acbc66cd1d9a1de0c498ecfc994a1a Mon Sep 17 00:00:00 2001 From: Matt Gabrenya Date: Thu, 8 Sep 2022 13:11:13 -0400 Subject: [PATCH 3/7] docs(api): improve comments exaplaining prepareAssignmentsEvenly algorithm --- packages/api/src/period/utils/assignment.ts | 64 ++++++++++++++++----- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/packages/api/src/period/utils/assignment.ts b/packages/api/src/period/utils/assignment.ts index f5409c2e7..44d7a1944 100644 --- a/packages/api/src/period/utils/assignment.ts +++ b/packages/api/src/period/utils/assignment.ts @@ -45,7 +45,7 @@ const queryReceiversWithPraise = async ( }, }, - // Sort decsending as first step of "First Fit Decreasing" bin-packing algorithm + // Sort descending as first step of "First Fit Decreasing" bin-packing algorithm { $sort: { praiseCount: -1, @@ -262,7 +262,7 @@ const prepareAssignmentsByTargetPraiseCount = async ( // Query the list of quantifiers & randomize order const quantifierPool = await queryQuantifierPoolRandomized(); - // Clone the list of recievers for each redundant assignment + // Clone the list of receivers for each redundant assignment // (as defined by setting PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER) const redundantAssignmentBins: Receiver[][] = flatten( range(PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER).map(() => { @@ -303,7 +303,7 @@ const prepareAssignmentsByTargetPraiseCount = async ( }; /** - * Apply a multiway number partitioning algorithm to + * Apply a Multiway Number Partitioning algorithm to * evenly distribute differently-sized collections of praise (i.e. all praise given to a single receiver) * into a fixed number of "bins" (i.e. quantifiers) * @@ -324,12 +324,11 @@ const prepareAssignmentsEvenly = async ( // Query the list of quantifiers & randomize order const quantifierPool = await queryQuantifierPoolRandomized(); - // Clone the list of recievers for each redundant assignment - // (as defined by setting PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER) - // zip them together, rotated, to prevent identical redundant receivers in a single bin + // Check that there are more quantifiers in the pool than redundant praise to be assigned + // otherwise a quantifier could be assigned the same praise multiple times if (PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER > quantifierPool.length) throw new Error( - 'Unable to assign redudant quantifications without more members in quantifier pool' + 'Unable to assign redundant quantifications without more members in quantifier pool' ); // Check that the number of redundant assignments is greater than to the number of receivers @@ -340,18 +339,57 @@ const prepareAssignmentsEvenly = async ( ); // Run "Greedy number partitioning" algorithm: + // Places any number of "items" into "bins", where there are a *fixed* number of bins, each with *dynamic* capacity. + // Each item takes up some amount of space in a bin. + // + // Attempts to distribute space taken up in bins as evenly as possible between all bins. + // + // For our use case: + // - Bin: quantifier + // - Item: receiver + // - Number of Bins: quantifier pool size + // - Size of each Item: receivers's praise count + const receiversDistributedByPraiseCount: Receiver[][] = greedyPartitioning( receivers, // Items to place in bins quantifierPool.length, // Available bins (r: Receiver) => r.praiseCount // Bin space taken by each item ); - const redundantReceiversShuffled: Receiver[][] = zip( - ...range(PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER).map((i) => { - // Create a "rotated" copy of array for each redundant quantification - // i.e. [a, b, c, d] => [b, c, d, a] - // ensure each rotation does not overlap - const receiversShuffledClone = [...receivers]; + /** + * Generate redundant copies, without overlapping assignments + * Then, transform into create groups of unique receivers ready for assignment to a single quantifier + * + * Example: For 3 redundant quantifications of 4 receivers a, b, c, d + * to be assigned to 4 quantifiers + * + * If greedy number partitioning gives us: + * [[a, b], [c], [d], [e,f,g]] + * + * + * Generate: + * [ + * [[a, b], [c], [d], [e,f,g]], + * [[e,f,g], [a, b], [c], [d]], + * [[d], [e,f,g], [a, b], [c]], + * ] + * + * Zipped to: + * [ + * [[a, b], [e,f,g], [d]], + * [[c], [a,b], [e,f,g]], + * [[d], [c], [a, b]], + * [[e,f,g], [d], [c]] + * ] + * + * Flattened to: + * [ + * [a, b, e, f, g, d], + * [c, a, b, e, f, g], + * [d, c, a, b], + * [e, f, g, d, c] + * ] + */ const redundantAssignmentBins: Receiver[][] = zip( ...range(PRAISE_QUANTIFIERS_PER_PRAISE_RECEIVER).map((rotations) => { From c15c627885f433bf9459e13b912941049e9b7d0b Mon Sep 17 00:00:00 2001 From: Matt Gabrenya Date: Thu, 8 Sep 2022 13:12:05 -0400 Subject: [PATCH 4/7] docs: document commands to connect to mongodb and tests within docker container --- DEVELOPMENT.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index ee0e0e88c..7854a433e 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -10,5 +10,15 @@ docker exec -i mongodb-praise /usr/bin/mongodump --authenticationDatabase admin **Restore** ``` -docker exec -i mongodb-praise sh -c 'mongorestore --authenticationDatabase admin -u DB_ROOT_USER -p DB_ROOT_PASSWORD --db praise_db --archive' < database-backup-BACKUP_DATE.archive +docker exec -i mongodb-praise sh -c "mongorestore --authenticationDatabase admin -u DB_ROOT_USER -p DB_ROOT_PASSWORD --db praise_db --archive" < database-backup-BACKUP_DATE.archive +``` + +### Connect to mongodb database running on docker +``` +docker exec -it mongodb-praise sh -c "mongosh \"mongodb://DB_USER:DB_PASSWORD@127.0.0.1:27017/DB_NAME\"" +``` + +### Run Api Tests +``` +docker exec -it api-praise yarn workspace api test ``` \ No newline at end of file From c4c778051372af88e557757879f4d126ad0513c4 Mon Sep 17 00:00:00 2001 From: Matt Gabrenya Date: Thu, 8 Sep 2022 16:00:34 -0400 Subject: [PATCH 5/7] test(api): fix test failing due to static expectations of assignment --- .../api/src/tests/period.assignment.spec.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/api/src/tests/period.assignment.spec.ts b/packages/api/src/tests/period.assignment.spec.ts index 4d81ff0c3..f654544dd 100644 --- a/packages/api/src/tests/period.assignment.spec.ts +++ b/packages/api/src/tests/period.assignment.spec.ts @@ -2,6 +2,7 @@ import { Wallet } from 'ethers'; import { expect } from 'chai'; import { faker } from '@faker-js/faker'; import some from 'lodash/some'; +import sum from 'lodash/sum'; import { seedPeriod, seedPraise, @@ -14,6 +15,7 @@ import { PraiseModel } from '@/praise/entities'; import { PeriodSettingsModel } from '@/periodsettings/entities'; import { UserModel } from '@/user/entities'; import { UserAccountModel } from '@/useraccount/entities'; +import { PeriodDetailsQuantifierDto } from '@/period/types'; import { loginUser } from './utils'; describe('PATCH /api/admin/periods/:periodId/assignQuantifiers', () => { @@ -244,6 +246,9 @@ describe('PATCH /api/admin/periods/:periodId/assignQuantifiers', () => { await seedUser({ roles: ['USER', 'QUANTIFIER'], }); + await seedUser({ + roles: ['USER', 'QUANTIFIER'], + }); const response = await this.client .patch( @@ -275,10 +280,14 @@ describe('PATCH /api/admin/periods/:periodId/assignQuantifiers', () => { ); expect(response.body.receivers[2].praiseCount).to.equal(3); - expect(response.body.quantifiers).to.have.length(3); - expect(response.body.quantifiers[0].praiseCount).to.equal(12); - expect(response.body.quantifiers[1].praiseCount).to.equal(12); - expect(response.body.quantifiers[2].praiseCount).to.equal(12); + expect(response.body.quantifiers).to.have.length(5); + expect( + sum( + response.body.quantifiers.map( + (q: PeriodDetailsQuantifierDto) => q.praiseCount + ) + ) + ).to.equal(12 * 3); expect(response.body.quantifiers[0].finishedCount).to.equal(0); expect(response.body.quantifiers[1].finishedCount).to.equal(0); From f9d93acae89486f64e06864fd1cd3b23efc94484 Mon Sep 17 00:00:00 2001 From: kristoferlund Date: Fri, 9 Sep 2022 09:44:01 +0200 Subject: [PATCH 6/7] Return error from `handleErrors` function --- packages/frontend/src/utils/axios.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/frontend/src/utils/axios.ts b/packages/frontend/src/utils/axios.ts index f6b859297..9001aeb50 100644 --- a/packages/frontend/src/utils/axios.ts +++ b/packages/frontend/src/utils/axios.ts @@ -6,7 +6,7 @@ import { toast } from 'react-hot-toast'; * * @param err */ -export const handleErrors = (err: AxiosError): void => { +export const handleErrors = (err: AxiosError): AxiosError => { // Any HTTP Code which is not 2xx will be considered as error const statusCode = err?.response?.status; @@ -25,6 +25,7 @@ export const handleErrors = (err: AxiosError): void => { } else { toast.error('Unknown Error'); } + return err; }; /** From 7c29c35335c5051af896013c8cbfc4d0c1e9d838 Mon Sep 17 00:00:00 2001 From: kristoferlund Date: Fri, 9 Sep 2022 09:47:53 +0200 Subject: [PATCH 7/7] Handle assign errors better in the frontend --- packages/frontend/src/model/periods.ts | 6 +++++- .../Periods/Details/components/PeriodDetails.tsx | 13 ++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/frontend/src/model/periods.ts b/packages/frontend/src/model/periods.ts index de0390b60..60ce478a2 100644 --- a/packages/frontend/src/model/periods.ts +++ b/packages/frontend/src/model/periods.ts @@ -26,7 +26,7 @@ import { periodReceiverPraiseListKey, } from '@/utils/periods'; import { useApiAuthClient } from '@/utils/api'; -import { ApiAuthGet, isResponseOk } from './api'; +import { ApiAuthGet, isApiResponseAxiosError, isResponseOk } from './api'; import { ActiveUserId } from './auth'; import { AllPraiseList, PraiseIdList, SinglePraise } from './praise'; // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -353,6 +353,10 @@ export const useAssignQuantifiers = ( status: 'QUANTIFY' as PeriodStatusType, }; setPeriod(updatedPeriod); + return response as AxiosResponse; + } + if (isApiResponseAxiosError(response)) { + throw response; } return response as AxiosResponse | AxiosError; }; diff --git a/packages/frontend/src/pages/Periods/Details/components/PeriodDetails.tsx b/packages/frontend/src/pages/Periods/Details/components/PeriodDetails.tsx index c527d3807..cdd4d3311 100644 --- a/packages/frontend/src/pages/Periods/Details/components/PeriodDetails.tsx +++ b/packages/frontend/src/pages/Periods/Details/components/PeriodDetails.tsx @@ -58,22 +58,29 @@ export const PeriodDetails = (): JSX.Element | null => { }; const handleAssign = (): void => { + const toastId = 'assignToast'; const promise = assignQuantifiers(); void toast.promise( promise, { loading: 'Assigning quantifiers …', - success: 'Quantifiers assigned', - error: 'Assign failed', + success: () => { + setTimeout(() => history.go(0), 2000); + return 'Quantifiers assigned'; + }, + error: () => { + setTimeout(() => toast.remove(toastId), 2000); + return 'Assign failed'; + }, }, { + id: toastId, position: 'top-center', loading: { duration: Infinity, }, } ); - promise.finally(() => setTimeout(() => history.go(0), 1000)); }; const handleExport = (): void => {