From de21487038cc720f21150b7c662536a8a37d965d Mon Sep 17 00:00:00 2001 From: Shervin Sarain Date: Mon, 30 May 2016 19:04:13 +0200 Subject: [PATCH] Fix challenge sorting Closes #7543 Closes #7507 --- .../GET-challenges_group_groupid.test.js | 66 +++++ .../challenges/GET-challenges_user.test.js | 280 +++++++++++------- .../server/controllers/api-v3/challenges.js | 8 +- 3 files changed, 244 insertions(+), 110 deletions(-) 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 275fe798f5..c507d0a4b9 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 @@ -64,6 +64,20 @@ describe('GET challenges/group/:groupId', () => { profile: {name: user.profile.name}, }); }); + + it('should return newest challenges first', async () => { + let challenges = await user.get(`/challenges/groups/${publicGuild._id}`); + + let foundChallengeIndex = _.findIndex(challenges, { _id: challenge2._id }); + expect(foundChallengeIndex).to.eql(0); + + let newChallenge = await generateChallenge(user, publicGuild); + + challenges = await user.get(`/challenges/groups/${publicGuild._id}`); + + foundChallengeIndex = _.findIndex(challenges, { _id: newChallenge._id }); + expect(foundChallengeIndex).to.eql(0); + }); }); context('Private Guild', () => { @@ -115,4 +129,56 @@ describe('GET challenges/group/:groupId', () => { }); }); }); + + context('official challenge is present', () => { + let publicGuild, user, officialChallenge, challenge, challenge2; + + before(async () => { + let { group, groupLeader } = await createAndPopulateGroup({ + groupDetails: { + name: 'TestGuild', + type: 'guild', + privacy: 'public', + }, + }); + + user = groupLeader; + publicGuild = group; + + await user.update({ + 'contributor.admin': true, + }); + + officialChallenge = await generateChallenge(user, group, { + official: true, + }); + + challenge = await generateChallenge(user, group); + challenge2 = await generateChallenge(user, group); + }); + + it('should return official challenges first', async () => { + let challenges = await user.get(`/challenges/groups/${publicGuild._id}`); + + let foundChallengeIndex = _.findIndex(challenges, { _id: officialChallenge._id }); + expect(foundChallengeIndex).to.eql(0); + }); + + 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); + + let newChallenge = await generateChallenge(user, publicGuild); + + challenges = await user.get(`/challenges/groups/${publicGuild._id}`); + + 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 ecb9f55383..b46367f46f 100644 --- a/test/api/v3/integration/challenges/GET-challenges_user.test.js +++ b/test/api/v3/integration/challenges/GET-challenges_user.test.js @@ -5,128 +5,196 @@ import { } from '../../../../helpers/api-v3-integration.helper'; describe('GET challenges/user', () => { - let user, member, nonMember, challenge, challenge2, publicGuild; + context('no official challenges', () => { + let user, member, nonMember, challenge, challenge2, publicGuild; - before(async () => { - let { group, groupLeader, members } = await createAndPopulateGroup({ - groupDetails: { - name: 'TestGuild', - type: 'guild', - privacy: 'public', - }, - members: 1, + before(async () => { + let { group, groupLeader, members } = await createAndPopulateGroup({ + groupDetails: { + name: 'TestGuild', + type: 'guild', + privacy: 'public', + }, + members: 1, + }); + + user = groupLeader; + publicGuild = group; + member = members[0]; + nonMember = await generateUser(); + + challenge = await generateChallenge(user, group); + challenge2 = await generateChallenge(user, group); }); - user = groupLeader; - publicGuild = group; - member = members[0]; - nonMember = await generateUser(); + it('should return challenges user has joined', async () => { + await nonMember.post(`/challenges/${challenge._id}/join`); - challenge = await generateChallenge(user, group); - challenge2 = await generateChallenge(user, group); - }); + let challenges = await nonMember.get('/challenges/user'); - it('should return challenges user has joined', async () => { - await nonMember.post(`/challenges/${challenge._id}/join`); - - let challenges = await nonMember.get('/challenges/user'); - - let foundChallenge = _.find(challenges, { _id: challenge._id }); - expect(foundChallenge).to.exist; - expect(foundChallenge.leader).to.eql({ - _id: publicGuild.leader._id, - id: publicGuild.leader._id, - profile: {name: user.profile.name}, + let foundChallenge = _.find(challenges, { _id: challenge._id }); + expect(foundChallenge).to.exist; + expect(foundChallenge.leader).to.eql({ + _id: publicGuild.leader._id, + id: publicGuild.leader._id, + profile: {name: user.profile.name}, + }); + expect(foundChallenge.group).to.eql({ + _id: publicGuild._id, + id: publicGuild._id, + type: publicGuild.type, + privacy: publicGuild.privacy, + name: publicGuild.name, + }); }); - expect(foundChallenge.group).to.eql({ - _id: publicGuild._id, - id: publicGuild._id, - type: publicGuild.type, - privacy: publicGuild.privacy, - name: publicGuild.name, + + it('should return challenges user has created', async () => { + let challenges = await user.get('/challenges/user'); + + let foundChallenge1 = _.find(challenges, { _id: challenge._id }); + expect(foundChallenge1).to.exist; + expect(foundChallenge1.leader).to.eql({ + _id: publicGuild.leader._id, + id: publicGuild.leader._id, + profile: {name: user.profile.name}, + }); + expect(foundChallenge1.group).to.eql({ + _id: publicGuild._id, + id: publicGuild._id, + type: publicGuild.type, + privacy: publicGuild.privacy, + name: publicGuild.name, + }); + let foundChallenge2 = _.find(challenges, { _id: challenge2._id }); + expect(foundChallenge2).to.exist; + expect(foundChallenge2.leader).to.eql({ + _id: publicGuild.leader._id, + id: publicGuild.leader._id, + profile: {name: user.profile.name}, + }); + expect(foundChallenge2.group).to.eql({ + _id: publicGuild._id, + id: publicGuild._id, + type: publicGuild.type, + privacy: publicGuild.privacy, + name: publicGuild.name, + }); + }); + + it('should return challenges in user\'s group', async () => { + let challenges = await member.get('/challenges/user'); + + let foundChallenge1 = _.find(challenges, { _id: challenge._id }); + expect(foundChallenge1).to.exist; + expect(foundChallenge1.leader).to.eql({ + _id: publicGuild.leader._id, + id: publicGuild.leader._id, + profile: {name: user.profile.name}, + }); + expect(foundChallenge1.group).to.eql({ + _id: publicGuild._id, + id: publicGuild._id, + type: publicGuild.type, + privacy: publicGuild.privacy, + name: publicGuild.name, + }); + let foundChallenge2 = _.find(challenges, { _id: challenge2._id }); + expect(foundChallenge2).to.exist; + expect(foundChallenge2.leader).to.eql({ + _id: publicGuild.leader._id, + id: publicGuild.leader._id, + profile: {name: user.profile.name}, + }); + expect(foundChallenge2.group).to.eql({ + _id: publicGuild._id, + id: publicGuild._id, + type: publicGuild.type, + privacy: publicGuild.privacy, + name: publicGuild.name, + }); + }); + + it('should return newest challenges first', async () => { + let challenges = await user.get('/challenges/user'); + + let foundChallengeIndex = _.findIndex(challenges, { _id: challenge2._id }); + expect(foundChallengeIndex).to.eql(0); + + let newChallenge = await generateChallenge(user, publicGuild); + + challenges = await user.get('/challenges/user'); + + foundChallengeIndex = _.findIndex(challenges, { _id: newChallenge._id }); + expect(foundChallengeIndex).to.eql(0); + }); + + it('should not return challenges user doesn\'t have access to', async () => { + let { group, groupLeader } = await createAndPopulateGroup({ + groupDetails: { + name: 'TestPrivateGuild', + type: 'guild', + privacy: 'private', + }, + }); + + let privateChallenge = await generateChallenge(groupLeader, group); + + let challenges = await nonMember.get('/challenges/user'); + + let foundChallenge = _.find(challenges, { _id: privateChallenge._id }); + expect(foundChallenge).to.not.exist; }); }); - it('should return challenges user has created', async () => { - let challenges = await user.get('/challenges/user'); + context('official challenge is present', () => { + let user, officialChallenge, challenge, challenge2, publicGuild; - let foundChallenge1 = _.find(challenges, { _id: challenge._id }); - expect(foundChallenge1).to.exist; - expect(foundChallenge1.leader).to.eql({ - _id: publicGuild.leader._id, - id: publicGuild.leader._id, - profile: {name: user.profile.name}, - }); - expect(foundChallenge1.group).to.eql({ - _id: publicGuild._id, - id: publicGuild._id, - type: publicGuild.type, - privacy: publicGuild.privacy, - name: publicGuild.name, - }); - let foundChallenge2 = _.find(challenges, { _id: challenge2._id }); - expect(foundChallenge2).to.exist; - expect(foundChallenge2.leader).to.eql({ - _id: publicGuild.leader._id, - id: publicGuild.leader._id, - profile: {name: user.profile.name}, - }); - expect(foundChallenge2.group).to.eql({ - _id: publicGuild._id, - id: publicGuild._id, - type: publicGuild.type, - privacy: publicGuild.privacy, - name: publicGuild.name, - }); - }); + before(async () => { + let { group, groupLeader } = await createAndPopulateGroup({ + groupDetails: { + name: 'TestGuild', + type: 'guild', + privacy: 'public', + }, + }); - it('should return challenges in user\'s group', async () => { - let challenges = await member.get('/challenges/user'); + user = groupLeader; + publicGuild = group; - let foundChallenge1 = _.find(challenges, { _id: challenge._id }); - expect(foundChallenge1).to.exist; - expect(foundChallenge1.leader).to.eql({ - _id: publicGuild.leader._id, - id: publicGuild.leader._id, - profile: {name: user.profile.name}, - }); - expect(foundChallenge1.group).to.eql({ - _id: publicGuild._id, - id: publicGuild._id, - type: publicGuild.type, - privacy: publicGuild.privacy, - name: publicGuild.name, - }); - let foundChallenge2 = _.find(challenges, { _id: challenge2._id }); - expect(foundChallenge2).to.exist; - expect(foundChallenge2.leader).to.eql({ - _id: publicGuild.leader._id, - id: publicGuild.leader._id, - profile: {name: user.profile.name}, - }); - expect(foundChallenge2.group).to.eql({ - _id: publicGuild._id, - id: publicGuild._id, - type: publicGuild.type, - privacy: publicGuild.privacy, - name: publicGuild.name, - }); - }); + await user.update({ + 'contributor.admin': true, + }); - it('should not return challenges user doesn\'t have access to', async () => { - let { group, groupLeader } = await createAndPopulateGroup({ - groupDetails: { - name: 'TestPrivateGuild', - type: 'guild', - privacy: 'private', - }, + officialChallenge = await generateChallenge(user, group, { + official: true, + }); + + challenge = await generateChallenge(user, group); + challenge2 = await generateChallenge(user, group); }); - let privateChallenge = await generateChallenge(groupLeader, group); + it('should return official challenges first', async () => { + let challenges = await user.get('/challenges/user'); - let challenges = await nonMember.get('/challenges/user'); + let foundChallengeIndex = _.findIndex(challenges, { _id: officialChallenge._id }); + expect(foundChallengeIndex).to.eql(0); + }); - let foundChallenge = _.find(challenges, { _id: privateChallenge._id }); - expect(foundChallenge).to.not.exist; + it('should return newest challenges first, after official ones', async () => { + let challenges = await user.get('/challenges/user'); + + let foundChallengeIndex = _.findIndex(challenges, { _id: challenge._id }); + expect(foundChallengeIndex).to.eql(2); + + foundChallengeIndex = _.findIndex(challenges, { _id: challenge2._id }); + expect(foundChallengeIndex).to.eql(1); + + let newChallenge = await generateChallenge(user, publicGuild); + + challenges = await user.get('/challenges/user'); + + 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 6d7bc56667..652fcc76d8 100644 --- a/website/server/controllers/api-v3/challenges.js +++ b/website/server/controllers/api-v3/challenges.js @@ -201,7 +201,7 @@ api.leaveChallenge = { * @apiName GetUserChallenges * @apiGroup Challenge * - * @apiSuccess {Array} data An array of challenges + * @apiSuccess {Array} data An array of challenges sorted with official challenges first, followed by the challenges in order from newest to oldest */ api.getUserChallenges = { method: 'GET', @@ -218,7 +218,7 @@ api.getUserChallenges = { ], _id: {$ne: '95533e05-1ff9-4e46-970b-d77219f199e9'}, // remove the Spread the Word Challenge for now, will revisit when we fix the closing-challenge bug TODO revisit }) - .sort('-official -timestamp') + .sort('-official -createdAt') // see below why we're not using populate // .populate('group', basicGroupFields) // .populate('leader', nameFields) @@ -249,7 +249,7 @@ api.getUserChallenges = { * * @apiParam {groupId} groupId The group _id * - * @apiSuccess {Array} data An array of challenges + * @apiSuccess {Array} data An array of challenges sorted with official challenges first, followed by the challenges in order from newest to oldest */ api.getGroupChallenges = { method: 'GET', @@ -268,7 +268,7 @@ api.getGroupChallenges = { if (!group) throw new NotFound(res.t('groupNotFound')); let challenges = await Challenge.find({group: groupId}) - .sort('-official -timestamp') + .sort('-official -createdAt') // .populate('leader', nameFields) // Only populate the leader as the group is implicit .exec();