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.

Sign up for GitHub

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

[FIX] 'Query' support for channels.list.joined, groups.list, groups.listAll, im.list #9424

Merged
merged 5 commits into from
Feb 17, 2018
Merged

[FIX] 'Query' support for channels.list.joined, groups.list, groups.listAll, im.list #9424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
36ce870
[REST API] Add query parameter to channels.list.joined, groups.list, …
xbolshe Jan 16, 2018
b5b0794
Add customFields from Room to Subscription to enable query
xbolshe Jan 17, 2018
b16bde5
Add customFields for Direct Messages of the calling user
xbolshe Jan 17, 2018
d9dfcd8
Propogate user's customFields changes to Direct Messages' Rooms
xbolshe Jan 21, 2018
0f4da73
Merge branch 'develop' into develop
xbolshe Jan 27, 2018
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
9 changes: 7 additions & 2 deletions packages/rocketchat-api/server/v1/channels.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,13 @@ RocketChat.API.v1.addRoute('channels.list', { authRequired: true }, {
RocketChat.API.v1.addRoute('channels.list.joined', { authRequired: true }, {
get() {
const { offset, count } = this.getPaginationItems();
const { sort, fields } = this.parseJsonQuery();
let rooms = _.pluck(RocketChat.models.Subscriptions.findByTypeAndUserId('c', this.userId).fetch(), '_room');
const { sort, fields, query } = this.parseJsonQuery();
const ourQuery = Object.assign({}, query, {
t: 'c',
'u._id': this.userId
});

let rooms = _.pluck(RocketChat.models.Subscriptions.find(ourQuery).fetch(), '_room');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to just blindly pass query through?

Copy link
Contributor

Choose a reason for hiding this comment

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

@geekgonecrazy it's not exactly passing the query blindly. The function parseJsonQuery handles several cases and line 410 ensures that the calling user is only able to query their own...although I say that without trying it. @xbolshe what happens whenever someone passes the query with a nested property like the one the string u._id is doing? For example, { "u": { "_id": "other_users_id" } }?

Copy link
Contributor Author

@xbolshe xbolshe Jan 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@graywolf336 One string and nested string will have different actions as described here: https://stackoverflow.com/questions/16002659/how-to-query-nested-objects
Becase "u" has several fields inside, only nested "_id" will find no records at all:

default

But one string provides a records selection:

default

So, need to use all fields nested inside "u" like shown below:

default

Providing 2 different values for one field like "u._id" will cause a search of no records:

default

Answering on your question, one field cannot be equal at the same time to 2 different values.

If a value will be duplicated like shown below, a search will be performed:

default

So, only records belongs to this.userId will be searched for channels.list.joined.
I have tried to inherit constraints like this.userId applied before my changes.

Copy link
Contributor Author

@xbolshe xbolshe Jan 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

BTW: the last constraint's value overwrites the previous one:
default

default

So, 'u._id': this.userId shall be after 'query' as it's done.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for looking into it in more depth! 👍

const totalCount = rooms.length;

rooms = RocketChat.models.Rooms.processQueryOptionsOnResult(rooms, {
Expand Down
15 changes: 11 additions & 4 deletions packages/rocketchat-api/server/v1/groups.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,13 @@ RocketChat.API.v1.addRoute('groups.leave', { authRequired: true }, {
RocketChat.API.v1.addRoute('groups.list', { authRequired: true }, {
get() {
const { offset, count } = this.getPaginationItems();
const { sort, fields } = this.parseJsonQuery();
let rooms = _.pluck(RocketChat.models.Subscriptions.findByTypeAndUserId('p', this.userId).fetch(), '_room');
const { sort, fields, query } = this.parseJsonQuery();
const ourQuery = Object.assign({}, query, {
t: 'p',
'u._id': this.userId
});

let rooms = _.pluck(RocketChat.models.Subscriptions.find(ourQuery).fetch(), '_room');
const totalCount = rooms.length;

rooms = RocketChat.models.Rooms.processQueryOptionsOnResult(rooms, {
Expand All @@ -344,8 +349,10 @@ RocketChat.API.v1.addRoute('groups.listAll', { authRequired: true }, {
return RocketChat.API.v1.unauthorized();
}
const { offset, count } = this.getPaginationItems();
const { sort, fields } = this.parseJsonQuery();
let rooms = RocketChat.models.Rooms.findByType('p').fetch();
const { sort, fields, query } = this.parseJsonQuery();
const ourQuery = Object.assign({}, query, { t: 'p' });

let rooms = RocketChat.models.Rooms.find(ourQuery).fetch();
const totalCount = rooms.length;

rooms = RocketChat.models.Rooms.processQueryOptionsOnResult(rooms, {
Expand Down
9 changes: 7 additions & 2 deletions packages/rocketchat-api/server/v1/im.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,13 @@ RocketChat.API.v1.addRoute(['dm.messages.others', 'im.messages.others'], { authR
RocketChat.API.v1.addRoute(['dm.list', 'im.list'], { authRequired: true }, {
get() {
const { offset, count } = this.getPaginationItems();
const { sort, fields } = this.parseJsonQuery();
let rooms = _.pluck(RocketChat.models.Subscriptions.findByTypeAndUserId('d', this.userId).fetch(), '_room');
const { sort, fields, query } = this.parseJsonQuery();
const ourQuery = Object.assign({}, query, {
t: 'd',
'u._id': this.userId
});

let rooms = _.pluck(RocketChat.models.Subscriptions.find(ourQuery).fetch(), '_room');
const totalCount = rooms.length;

rooms = RocketChat.models.Rooms.processQueryOptionsOnResult(rooms, {
Expand Down
3 changes: 3 additions & 0 deletions packages/rocketchat-lib/server/functions/saveCustomFieldsWithoutValidation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ RocketChat.saveCustomFieldsWithoutValidation = function(userId, formData) {
Object.keys(customFieldsMeta).forEach(key => customFields[key] = formData[key]);
RocketChat.models.Users.setCustomFields(userId, customFields);

// Update customFields of all Direct Messages' Rooms for userId
RocketChat.models.Subscriptions.setCustomFieldsDirectMessagesByUserId(userId, customFields);

Object.keys(customFields).forEach((fieldName) => {
if (!customFieldsMeta[fieldName].modifyRecordField) {
return;
Expand Down
17 changes: 17 additions & 0 deletions packages/rocketchat-lib/server/models/Subscriptions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,22 @@ class ModelSubscriptions extends RocketChat.models._Base {
return this.update(query, update);
}

setCustomFieldsDirectMessagesByUserId(userId, fields) {
const values = {};
Object.keys(fields).forEach(key => {
values[`customFields.${ key }`] = fields[key];
});

const query = {
'u._id': userId,
't': 'd'
};
const update = { $set: values };
const options = { 'multi': true };

return this.update(query, update, options);
}

setFavoriteByRoomIdAndUserId(roomId, userId, favorite) {
if (favorite == null) { favorite = true; }
const query = {
Expand Down Expand Up @@ -557,6 +573,7 @@ class ModelSubscriptions extends RocketChat.models._Base {
rid: room._id,
name: room.name,
fname: room.fname,
customFields: room.customFields,
t: room.t,
u: {
_id: user._id,
Expand Down
2 changes: 2 additions & 0 deletions server/methods/createDirectMessage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Meteor.methods({
unread: 0,
userMentions: 0,
groupMentions: 0,
customFields: me.customFields,
u: {
_id: me._id,
username: me.username
Expand Down Expand Up @@ -96,6 +97,7 @@ Meteor.methods({
unread: 0,
userMentions: 0,
groupMentions: 0,
customFields: to.customFields,
u: {
_id: to._id,
username: to.username
Expand Down