diff --git a/test/api/v3/integration/challenges/GET-challenges_group_groupid.test.js b/test/api/v3/integration/challenges/GET-challenges_group_groupid.test.js index 19ee8798c9..8e4fc6a432 100644 --- a/test/api/v3/integration/challenges/GET-challenges_group_groupid.test.js +++ b/test/api/v3/integration/challenges/GET-challenges_group_groupid.test.js @@ -187,8 +187,7 @@ describe('GET challenges/groups/:groupId', () => { }); context('official challenge is present', () => { - let publicGuild; let user; let officialChallenge; let challenge; let - challenge2; + let publicGuild; let user; let officialChallenge; let unofficialChallenges; before(async () => { const { group, groupLeader } = await createAndPopulateGroup({ @@ -214,10 +213,14 @@ describe('GET challenges/groups/:groupId', () => { }); await user.post(`/challenges/${officialChallenge._id}/join`); - challenge = await generateChallenge(user, group); - await user.post(`/challenges/${challenge._id}/join`); - challenge2 = await generateChallenge(user, group); - await user.post(`/challenges/${challenge2._id}/join`); + // We add 10 extra challenges to test whether the official challenge + // (the oldest) makes it to the front page. + unofficialChallenges = []; + for (let i = 0; i < 10; i += 1) { + const challenge = await generateChallenge(user, group); // eslint-disable-line + await user.post(`/challenges/${challenge._id}/join`); // eslint-disable-line + unofficialChallenges.push(challenge); + } }); it('should return official challenges first', async () => { @@ -230,18 +233,17 @@ describe('GET challenges/groups/:groupId', () => { it('should return newest challenges first, after official ones', async () => { let challenges = await user.get(`/challenges/groups/${publicGuild._id}`); - let foundChallengeIndex = _.findIndex(challenges, { _id: challenge._id }); - expect(foundChallengeIndex).to.eql(2); - - foundChallengeIndex = _.findIndex(challenges, { _id: challenge2._id }); - expect(foundChallengeIndex).to.eql(1); + unofficialChallenges.forEach((chal, index) => { + const foundChallengeIndex = _.findIndex(challenges, { _id: chal._id }); + expect(foundChallengeIndex).to.eql(10 - index); + }); const newChallenge = await generateChallenge(user, publicGuild); await user.post(`/challenges/${newChallenge._id}/join`); challenges = await user.get(`/challenges/groups/${publicGuild._id}`); - foundChallengeIndex = _.findIndex(challenges, { _id: newChallenge._id }); + const foundChallengeIndex = _.findIndex(challenges, { _id: newChallenge._id }); expect(foundChallengeIndex).to.eql(1); }); }); diff --git a/test/api/v3/integration/challenges/GET-challenges_user.test.js b/test/api/v3/integration/challenges/GET-challenges_user.test.js index abb7b32804..1770745900 100644 --- a/test/api/v3/integration/challenges/GET-challenges_user.test.js +++ b/test/api/v3/integration/challenges/GET-challenges_user.test.js @@ -242,7 +242,7 @@ describe('GET challenges/user', () => { }); context('official challenge is present', () => { - let user; let officialChallenge; let challenge; let challenge2; let + let user; let officialChallenge; let unofficialChallenges; let publicGuild; before(async () => { @@ -270,10 +270,14 @@ describe('GET challenges/user', () => { }); await user.post(`/challenges/${officialChallenge._id}/join`); - challenge = await generateChallenge(user, group); - await user.post(`/challenges/${challenge._id}/join`); - challenge2 = await generateChallenge(user, group); - await user.post(`/challenges/${challenge2._id}/join`); + // We add 10 extra challenges to test whether the official challenge + // (the oldest) makes it to the front page. + unofficialChallenges = []; + for (let i = 0; i < 10; i += 1) { + const challenge = await generateChallenge(user, group); // eslint-disable-line + await user.post(`/challenges/${challenge._id}/join`); // eslint-disable-line + unofficialChallenges.push(challenge); + } }); it('should return official challenges first', async () => { @@ -284,20 +288,23 @@ describe('GET challenges/user', () => { }); it('should return newest challenges first, after official ones', async () => { - let challenges = await user.get('/challenges/user'); + let challenges = await user.get('/challenges/user?page=0'); - let foundChallengeIndex = _.findIndex(challenges, { _id: challenge._id }); - expect(foundChallengeIndex).to.eql(2); - - foundChallengeIndex = _.findIndex(challenges, { _id: challenge2._id }); - expect(foundChallengeIndex).to.eql(1); + unofficialChallenges.forEach((chal, index) => { + const foundChallengeIndex = _.findIndex(challenges, { _id: chal._id }); + if (index === 0) { + expect(foundChallengeIndex).to.eql(-1); + } else { + expect(foundChallengeIndex).to.eql(10 - index); + } + }); const newChallenge = await generateChallenge(user, publicGuild); await user.post(`/challenges/${newChallenge._id}/join`); challenges = await user.get('/challenges/user'); - foundChallengeIndex = _.findIndex(challenges, { _id: newChallenge._id }); + const foundChallengeIndex = _.findIndex(challenges, { _id: newChallenge._id }); expect(foundChallengeIndex).to.eql(1); }); }); diff --git a/website/server/controllers/api-v3/challenges.js b/website/server/controllers/api-v3/challenges.js index a68e08ffa3..c5373a5467 100644 --- a/website/server/controllers/api-v3/challenges.js +++ b/website/server/controllers/api-v3/challenges.js @@ -26,6 +26,7 @@ import { getChallengeGroupResponse, createChallenge, cleanUpTask, + createChallengeQuery, } from '../../libs/challenges'; import apiError from '../../libs/apiError'; @@ -409,13 +410,12 @@ api.getUserChallenges = { query.categories = { $elemMatch: { slug: { $in: categorySlugs } } }; } - let mongoQuery = Challenge.find(query) - .sort('-createdAt'); - + // Ensure that official challenges are always first + let mongoQuery = createChallengeQuery(query); if (page) { mongoQuery = mongoQuery - .limit(CHALLENGES_PER_PAGE) - .skip(CHALLENGES_PER_PAGE * page); + .skip(CHALLENGES_PER_PAGE * page) + .limit(CHALLENGES_PER_PAGE); } // see below why we're not using populate @@ -423,9 +423,8 @@ api.getUserChallenges = { // .populate('leader', nameFields) const challenges = await mongoQuery.exec(); - let resChals = challenges.map(challenge => challenge.toJSON()); - - resChals = _.orderBy(resChals, [challenge => challenge.categories.map(category => category.slug).includes('habitica_official')], ['desc']); + // Unserialize, then serialize the challenges to fill in default fields + const resChals = challenges.map(chal => (new Challenge(chal)).toJSON()); // Instead of populate we make a find call manually because of https://github.com/Automattic/mongoose/issues/3833 await Promise.all(resChals.map((chal, index) => Promise.all([ @@ -483,19 +482,12 @@ api.getGroupChallenges = { const group = await Group.getGroup({ user, groupId }); if (!group) throw new NotFound(res.t('groupNotFound')); - const challenges = await Challenge.find({ group: groupId }) - .sort('-createdAt') + const challenges = await createChallengeQuery({ group: groupId }) // Only populate the leader as the group is implicit // see below why we're not using populate // .populate('leader', nameFields) .exec(); - let resChals = challenges.map(challenge => challenge.toJSON()); - - resChals = _.orderBy( - resChals, - [challenge => challenge.categories.map(category => category.slug).includes('habitica_official')], - ['desc'], - ); + const resChals = challenges.map(challenge => (new Challenge(challenge)).toJSON()); // Instead of populate we make a find call manually because of https://github.com/Automattic/mongoose/issues/3833 await Promise.all(resChals.map((chal, index) => User diff --git a/website/server/libs/challenges/index.js b/website/server/libs/challenges/index.js index 8373f06f98..d6fdedfb60 100644 --- a/website/server/libs/challenges/index.js +++ b/website/server/libs/challenges/index.js @@ -109,3 +109,29 @@ export function cleanUpTask (task) { return cleansedTask; } + +// Create an aggregation query for querying challenges. +// Ensures that official challenges are listed first. +export function createChallengeQuery (query) { + return Challenge.aggregate() + .match(query) + .addFields({ + isOfficial: { + $gt: [ + { + $size: { + $filter: { + input: '$categories', + as: 'cat', + cond: { + $eq: ['$$cat.slug', 'habitica_official'], + }, + }, + }, + }, + 0, + ], + }, + }) + .sort('-isOfficial -createdAt'); +}