Skip to content

Commit

Permalink
fix slice targetLevelIndex bug
Browse files Browse the repository at this point in the history
  • Loading branch information
sspenst committed Mar 20, 2024
1 parent 1184ba9 commit 472455d
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 35 deletions.
2 changes: 1 addition & 1 deletion components/collection/collectionScrollList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default function CollectionScrollList({ collection, id, isHidden, onLoadi

const fetchLevels = useCallback(async (cursor: string, direction: 'before' | 'after') => {
const params = new URLSearchParams({
populateLevelCursor: cursor,
populateAroundId: cursor,
populateLevelDirection: direction,
});

Expand Down
3 changes: 1 addition & 2 deletions lib/initializeLocalDb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,7 @@ export default async function initializeLocalDb() {
name: 'test collection 3',
slug: 'bbb/test-collection',
userId: new Types.ObjectId(TestId.USER_B),
}

},
],
{ ordered: false }
));
Expand Down
68 changes: 36 additions & 32 deletions pages/api/collection-by-id/[id].ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ export default apiWrapper({
GET: {
query: {
id: ValidObjectId(true),
populateLevelCursor: ValidObjectId(false),
populateAroundId: ValidObjectId(false),
populateLevelDirection: ValidEnum(['before', 'after', 'around'], false),
}
} }, async (req: NextApiRequest, res: NextApiResponse) => {
const { id, populateLevelCursor, populateLevelDirection } = req.query;
const { id, populateAroundId, populateLevelDirection } = req.query;
const token = req.cookies?.token;
const gameId = getGameIdFromReq(req);
const reqUser = token ? await getUserFromToken(token, req) : null;
const collection = await getCollection({
matchQuery: { _id: new Types.ObjectId(id as string), gameId: gameId },
...(populateLevelCursor ? { populateLevelCursor: new Types.ObjectId(populateLevelCursor as string) } : {}),
...(populateAroundId ? { populateAroundId: new Types.ObjectId(populateAroundId as string) } : {}),
...(populateLevelDirection ? { populateLevelDirection: populateLevelDirection as 'before' | 'after' | 'around' } : {}),
reqUser,
});
Expand All @@ -43,8 +43,8 @@ interface GetCollectionProps {
includeDraft?: boolean;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
matchQuery: FilterQuery<any>;
populateAroundId?: Types.ObjectId; // target level
populateAroundSlug?: string; // target level
populateLevelCursor?: Types.ObjectId; // target level
populateLevelData?: boolean;
populateLevelDirection?: 'before' | 'after' | 'around';
reqUser: User | null;
Expand Down Expand Up @@ -72,8 +72,8 @@ export async function getCollections({
includeDraft,
matchQuery,
reqUser,
populateAroundId,
populateAroundSlug,
populateLevelCursor,
populateLevelData = true,
populateLevelDirection,
}: GetCollectionProps): Promise<Collection[]> {
Expand Down Expand Up @@ -211,17 +211,13 @@ export async function getCollections({
}
}
},
...(populateLevelCursor || populateAroundSlug ? [
// TODO: we should probably pass in an index rather than a level id/slug
// no guarantee the level will be in the collection, and if it isn't we have no good way to continue the infinite scroll
// BUG: targetLevelIndex can be -1 if the level is not found in the collection,
// which will result in an error if we are populating 'before' ($min must be a positive number)
...(populateAroundId || populateAroundSlug ? [
{
$addFields: {
targetLevelIndex: {
$cond: {
if: populateLevelCursor,
then: { $indexOfArray: ['$levels._id', populateLevelCursor] },
if: populateAroundId,
then: { $indexOfArray: ['$levels._id', populateAroundId] },
else: { $indexOfArray: ['$levels.slug', populateAroundSlug] },
}
},
Expand All @@ -230,34 +226,42 @@ export async function getCollections({
{
$addFields: {
levels: {
// if the target level is not found, return empty array
// this can happen if an invalid cid is passed in the URL, or if the target level is removed from the collection
$cond: {
if: { $eq: [populateLevelDirection, 'before'] },
// populate the target level + at most 5 levels before
then: {
$slice: [
'$levels',
{ $max: [0, { $subtract: ['$targetLevelIndex', populateLevelCount] }] },
{ $min: [populateLevelCount + 1, { $add: ['$targetLevelIndex', 1] }] }
]
},
if: { $lt: ['$targetLevelIndex', 0] },
then: [],
else: {
$cond: {
if: { $eq: [populateLevelDirection, 'after'] },
// populate the target level + at most 5 levels after
if: { $eq: [populateLevelDirection, 'before'] },
// populate the target level + at most 5 levels before
then: {
$slice: [
'$levels',
'$targetLevelIndex',
{ $min: [populateLevelCount + 1, { $subtract: [{ $size: '$levels' }, '$targetLevelIndex'] }] }
]
},
else: { // 'around'
// populate the target level + at most 5 levels before and 5 levels after
$slice: [
'$levels',
{ $max: [0, { $subtract: ['$targetLevelIndex', populateLevelCount] }] },
{ $min: [populateLevelCount * 2 + 1, { $size: '$levels' }] },
{ $min: [populateLevelCount + 1, { $add: ['$targetLevelIndex', 1] }] }
]
},
else: {
$cond: {
if: { $eq: [populateLevelDirection, 'after'] },
// populate the target level + at most 5 levels after
then: {
$slice: [
'$levels',
'$targetLevelIndex',
{ $min: [populateLevelCount + 1, { $subtract: [{ $size: '$levels' }, '$targetLevelIndex'] }] }
]
},
else: { // 'around'
// populate the target level + at most 5 levels before and 5 levels after
$slice: [
'$levels',
{ $max: [0, { $subtract: ['$targetLevelIndex', populateLevelCount] }] },
{ $min: [populateLevelCount * 2 + 1, { $size: '$levels' }] },
]
}
}
}
}
}
Expand Down
88 changes: 88 additions & 0 deletions tests/pages/api/collection/collection.byid.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { logger } from '@root/helpers/logger';
import { EnrichedCollection } from '@root/models/db/collection';
import { enableFetchMocks } from 'jest-fetch-mock';
import { testApiHandler } from 'next-test-api-route-handler';
import { Logger } from 'winston';
Expand Down Expand Up @@ -64,6 +65,93 @@ describe('pages/api/collection/[id].ts', () => {
},
});
});
test('GET collection populate before', async () => {
await testApiHandler({
pagesHandler: async (_, res) => {
const req: NextApiRequestWithAuth = {
method: 'GET',
userId: TestId.USER_B,
cookies: {
token: getTokenCookieValue(TestId.USER_B),
},
query: {
id: TestId.COLLECTION_B,
populateAroundId: TestId.LEVEL,
populateLevelDirection: 'before',
},
} as unknown as NextApiRequestWithAuth;

await getCollectionHandler(req, res);
},
test: async ({ fetch }) => {
const res = await fetch();

expect(res.status).toBe(200);
const response = await res.json() as EnrichedCollection;

expect(response.levels.length).toBe(1);
expect(response.levels[0]._id).toBe(TestId.LEVEL);
},
});
});
test('GET collection populate after', async () => {
await testApiHandler({
pagesHandler: async (_, res) => {
const req: NextApiRequestWithAuth = {
method: 'GET',
userId: TestId.USER_B,
cookies: {
token: getTokenCookieValue(TestId.USER_B),
},
query: {
id: TestId.COLLECTION_B,
populateAroundId: TestId.LEVEL,
populateLevelDirection: 'after',
},
} as unknown as NextApiRequestWithAuth;

await getCollectionHandler(req, res);
},
test: async ({ fetch }) => {
const res = await fetch();

expect(res.status).toBe(200);
const response = await res.json() as EnrichedCollection;

expect(response.levels.length).toBe(2);
expect(response.levels[0]._id).toBe(TestId.LEVEL);
expect(response.levels[1]._id).toBe(TestId.LEVEL_3);
},
});
});
test('GET collection populate not found', async () => {
await testApiHandler({
pagesHandler: async (_, res) => {
const req: NextApiRequestWithAuth = {
method: 'GET',
userId: TestId.USER_B,
cookies: {
token: getTokenCookieValue(TestId.USER_B),
},
query: {
id: TestId.COLLECTION_B,
populateAroundId: TestId.LEVEL_4,
populateLevelDirection: 'around',
},
} as unknown as NextApiRequestWithAuth;

await getCollectionHandler(req, res);
},
test: async ({ fetch }) => {
const res = await fetch();

expect(res.status).toBe(200);
const response = await res.json() as EnrichedCollection;

expect(response.levels.length).toBe(0);
},
});
});
test('PUT other user\'s collection should 401', async () => {
jest.spyOn(logger, 'error').mockImplementation(() => ({} as Logger));
await testApiHandler({
Expand Down

0 comments on commit 472455d

Please sign in to comment.