Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added get group challenges route and initial tests #6590

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import {
generateUser,
generateChallenge,
createAndPopulateGroup,
translate as t,
} from '../../../../helpers/api-v3-integration.helper';

describe('GET challenges/group/:groupId', () => {
context('Public Guild', () => {
let publicGuild, user, nonMember, challenge, challenge2;

before(async () => {
let { group, groupLeader } = await createAndPopulateGroup({
groupDetails: {
name: 'TestGuild',
type: 'guild',
privacy: 'public',
},
});

publicGuild = group;
user = groupLeader;

nonMember = await generateUser();

challenge = await generateChallenge(user, group);
challenge2 = await generateChallenge(user, group);
});

it('should return group challenges for non member', async () => {
let challenges = await nonMember.get(`/challenges/groups/${publicGuild._id}`);

let foundChallenge1 = _.find(challenges, { _id: challenge._id });
expect(foundChallenge1).to.exist;
let foundChallenge2 = _.find(challenges, { _id: challenge2._id });
expect(foundChallenge2).to.exist;
});

it('should return group challenges for member', async () => {
let challenges = await user.get(`/challenges/groups/${publicGuild._id}`);

let foundChallenge1 = _.find(challenges, { _id: challenge._id });
expect(foundChallenge1).to.exist;
let foundChallenge2 = _.find(challenges, { _id: challenge2._id });
expect(foundChallenge2).to.exist;
});
});

context('Private Guild', () => {
let privateGuild, user, nonMember, challenge, challenge2;

before(async () => {
let { group, groupLeader } = await createAndPopulateGroup({
groupDetails: {
name: 'TestPrivateGuild',
type: 'guild',
privacy: 'private',
},
});

privateGuild = group;
user = groupLeader;

nonMember = await generateUser();

challenge = await generateChallenge(user, group);
challenge2 = await generateChallenge(user, group);
});

it('should prevent non-member from seeing challenges', async () => {
await expect(nonMember.get(`/challenges/groups/${privateGuild._id}`))
.to.eventually.be.rejected.and.eql({
code: 404,
error: 'NotFound',
message: t('groupNotFound'),
});
});

it('should return group challenges for member', async () => {
let challenges = await user.get(`/challenges/groups/${privateGuild._id}`);

let foundChallenge1 = _.find(challenges, { _id: challenge._id });
expect(foundChallenge1).to.exist;
let foundChallenge2 = _.find(challenges, { _id: challenge2._id });
expect(foundChallenge2).to.exist;
});
});
});
72 changes: 72 additions & 0 deletions test/api/v3/integration/challenges/GET-challenges_user.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import {
generateUser,
generateChallenge,
createAndPopulateGroup,
} from '../../../../helpers/api-v3-integration.helper';

describe('GET challenges/user', () => {
let user, member, nonMember, challenge, challenge2;

before(async () => {
let { group, groupLeader, members } = await createAndPopulateGroup({
groupDetails: {
name: 'TestGuild',
type: 'guild',
privacy: 'public',
},
members: 1,
});

user = groupLeader;

member = members[0];
nonMember = await generateUser();

challenge = await generateChallenge(user, group);
challenge2 = await generateChallenge(user, group);
});

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;
});

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;
let foundChallenge2 = _.find(challenges, { _id: challenge2._id });
expect(foundChallenge2).to.exist;
});

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;
let foundChallenge2 = _.find(challenges, { _id: challenge2._id });
expect(foundChallenge2).to.exist;
});

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;
});
});
48 changes: 43 additions & 5 deletions website/src/controllers/api-v3/challenges.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ api.joinChallenge = {
let challenge = await Challenge.findOne({ _id: req.params.challengeId });
if (!challenge) throw new NotFound(res.t('challengeNotFound'));

if (!challenge.hasAccess(user)) throw new NotFound(res.t('challengeNotFound'));
let group = await Group.getGroup({user, groupId: challenge.groupId, fields: '_id type privacy', optionalMembership: true});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheHollidayInn why you added this check? challenge.hasAccess should haveve been enough

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I undestand, you should be able to join challenges in public guilds without being a member of the guild and I guess this fixes it. But probably this logic should be put in hasAccess

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, good catch

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And yes, this logic should be in hasAccess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KristianTashkov Yep, this is exactly the case.

I'll move the logic. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it seems the challenge.canView callas hasAccess, so basically, we only need to check viewAccess.

if (!group || !challenge.canView(user, group)) throw new NotFound(res.t('challengeNotFound'));

if (_.contains(user.challenges, challenge._id)) throw new NotAuthorized(res.t('userAlreadyInChallenge'));

Expand Down Expand Up @@ -172,16 +173,16 @@ api.leaveChallenge = {
};

/**
* @api {get} /challenges Get challenges for a user
* @api {get} /challenges/user Get challenges for a user
* @apiVersion 3.0.0
* @apiName GetChallenges
* @apiName GetUserChallenges
* @apiGroup Challenge
*
* @apiSuccess {Array} challenges An array of challenges
*/
api.getChallenges = {
api.getUserChallenges = {
method: 'GET',
url: '/challenges',
url: '/challenges/user',
middlewares: [authWithHeaders(), cron],
async handler (req, res) {
let user = res.locals.user;
Expand All @@ -208,6 +209,43 @@ api.getChallenges = {
},
};

/**
* @api {get} /challenges/group/group:Id Get challenges for a group
* @apiVersion 3.0.0
* @apiName GetGroupChallenges
* @apiGroup Challenge
*
* @apiParam {groupId} groupId The group _id
*
* @apiSuccess {Array} challenges An array of challenges
*/
api.getGroupChallenges = {
method: 'GET',
url: '/challenges/groups/:groupId',
middlewares: [authWithHeaders(), cron],
async handler (req, res) {
let user = res.locals.user;
let groupId = req.params.groupId;

req.checkParams('groupId', res.t('groupIdRequired')).notEmpty();

let validationErrors = req.validationErrors();
if (validationErrors) throw validationErrors;

let group = await Group.getGroup({user, groupId});
if (!group) throw new NotFound(res.t('groupNotFound'));

let challenges = await Challenge.find({groupId})
.sort('-official -timestamp')
// TODO populate
// .populate('group', '_id name type')
// .populate('leader', 'profile.name')
.exec();

res.respond(200, challenges);
},
};

/**
* @api {get} /challenges/:challengeId Get a challenge given its id
* @apiVersion 3.0.0
Expand Down