-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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 initial get groups tests #6531
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
import { | ||
generateUser, | ||
resetHabiticaDB, | ||
generateGroup, | ||
} from '../../../../helpers/api-v3-integration.helper'; | ||
|
||
describe('GET /groups', () => { | ||
let user; | ||
const NUMBER_OF_PUBLIC_GUILDS = 3; | ||
const NUMBER_OF_USERS_PRIVATE_GUILDS = 1; | ||
const NUMBER_OF_GROUPS_USER_CAN_VIEW = 5; | ||
|
||
before(async () => { | ||
await resetHabiticaDB(); | ||
|
||
let leader = await generateUser({ balance: 10 }); | ||
user = await generateUser({balance: 4}); | ||
|
||
let publicGuildUserIsMemberOf = await generateGroup(leader, { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason why you are not using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably because it's not part of the interface to add an existing user in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep ^^ |
||
name: 'public guild - is member', | ||
type: 'guild', | ||
privacy: 'public', | ||
}); | ||
await leader.post(`/groups/${publicGuildUserIsMemberOf._id}/invite`, { uuids: [user._id]}); | ||
await user.post(`/groups/${publicGuildUserIsMemberOf._id}/join`); | ||
|
||
await generateGroup(leader, { | ||
name: 'public guild - is not member', | ||
type: 'guild', | ||
privacy: 'public', | ||
}); | ||
|
||
let privateGuildUserIsMemberOf = await generateGroup(leader, { | ||
name: 'private guild - is member', | ||
type: 'guild', | ||
privacy: 'private', | ||
}); | ||
await leader.post(`/groups/${privateGuildUserIsMemberOf._id}/invite`, { uuids: [user._id]}); | ||
await user.post(`/groups/${privateGuildUserIsMemberOf._id}/join`); | ||
|
||
await generateGroup(leader, { | ||
name: 'private guild - is not member', | ||
type: 'guild', | ||
privacy: 'private', | ||
}); | ||
|
||
await generateGroup(leader, { | ||
name: 'party - is not member', | ||
type: 'party', | ||
privacy: 'private', | ||
}); | ||
|
||
await user.post('/groups', { | ||
name: 'party - is member', | ||
type: 'party', | ||
privacy: 'private', | ||
}); | ||
}); | ||
|
||
it('returns error when no query passed in', async () => { | ||
await expect(user.get('/groups')) | ||
.to.eventually.be.rejected.and.eql({ | ||
code: 400, | ||
error: 'BadRequest', | ||
message: 'Invalid request parameters.', | ||
}); | ||
}); | ||
|
||
it('returns only the tavern when tavern passed in as query', async () => { | ||
await expect(user.get('/groups?type=tavern')) | ||
.to.eventually.have.a.lengthOf(1) | ||
.and.to.have.deep.property('[0]') | ||
.and.to.have.property('_id', 'habitrpg'); | ||
}); | ||
|
||
it('returns only the user\'s party when party passed in as query', async () => { | ||
await expect(user.get('/groups?type=party')) | ||
.to.eventually.have.a.lengthOf(1) | ||
.and.to.have.deep.property('[0]'); | ||
}); | ||
|
||
it('returns all public guilds when publicGuilds passed in as query', async () => { | ||
await expect(user.get('/groups?type=publicGuilds')) | ||
.to.eventually.have.a.lengthOf(NUMBER_OF_PUBLIC_GUILDS); | ||
}); | ||
|
||
it('returns all private guilds user is a part of when privateGuilds passed in as query', async () => { | ||
await expect(user.get('/groups?type=privateGuilds')) | ||
.to.eventually.have.a.lengthOf(NUMBER_OF_USERS_PRIVATE_GUILDS); | ||
}); | ||
|
||
it('returns a list of groups user has access to', async () => { | ||
await expect(user.get('/groups?type=privateGuilds,publicGuilds,party,tavern')) | ||
.to.eventually.have.lengthOf(NUMBER_OF_GROUPS_USER_CAN_VIEW); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,14 +86,14 @@ api.getGroups = { | |
|
||
// TODO validate types are acceptable? probably not necessary | ||
let types = req.query.type.split(','); | ||
let groupFields = 'name description memberCount balance leader'; | ||
let groupFields = 'name description memberCount balance'; | ||
let sort = '-memberCount'; | ||
let queries = []; | ||
|
||
types.forEach(type => { | ||
switch (type) { | ||
case 'party': | ||
queries.push(Group.getGroup({user, groupId: 'party', fields: groupFields, populateLeader: true})); | ||
queries.push(Group.getGroup({user, groupId: 'party', fields: groupFields})); | ||
break; | ||
case 'privateGuilds': | ||
queries.push(Group.find({ | ||
|
@@ -109,7 +109,9 @@ api.getGroups = { | |
}).select(groupFields).sort(sort).exec()); // TODO use lean? | ||
break; | ||
case 'tavern': | ||
queries.push(Group.getGroup({user, groupId: 'habitrpg', fields: groupFields, populateLeader: true})); | ||
if (types.indexOf('publicGuilds') === -1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @TheHollidayInn could you remove the populateLeader at line 96? thanks There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and at line 113 too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eh, sorry about that. I didn't notice it was a parameter. I was thinking it was just in the projection. |
||
queries.push(Group.getGroup({user, groupId: 'habitrpg', fields: groupFields})); | ||
} | ||
break; | ||
} | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, it shouldn't be necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is necessary. Any other test that creates a public guild before this test runs will have that included in the public guild query. Resetting the db to an empty (plus tavern) state ensures that only the newly created public guilds will be counted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove the dependency on counting. I can use
indexOf()
and_.findIndex
to check rather than depend on the number of groups. Thoughts??There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paglias or @crookedneighbor . Let me know if you have any thoughts on changing this. I'm leaning towards using the index, but they both seem to work.