Skip to content

Regression: sidebar sorting was being wrong in some cases where the rooms records were returned before the subscriptions#11273

Merged
rodrigok merged 4 commits intodevelopfrom
fix-sort-list
Jun 27, 2018
Merged

Regression: sidebar sorting was being wrong in some cases where the rooms records were returned before the subscriptions#11273
rodrigok merged 4 commits intodevelopfrom
fix-sort-list

Conversation

@ggazzo
Copy link
Copy Markdown
Member

@ggazzo ggazzo commented Jun 27, 2018

Closes #ISSUE_NUMBER

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11273 June 27, 2018 15:36 Inactive
@ggazzo ggazzo requested a review from sampaiodiego June 27, 2018 15:36
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-11273 June 27, 2018 17:41 Inactive
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-11273 June 27, 2018 17:47 Inactive
const sub = RocketChat.models.Subscriptions.findOne({ rid: room._id });
if (!sub) {
return;
return room;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why return something?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

actually there are no difference (callbacks use the last value returned !== undefined, just to avoid some misunderstanding.

return;
return room;
}
const $set = {lastMessage : room.lastMessage, lm: room._updatedAt, ...getLowerCaseNames(room, sub.name)};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please break this object in multiple lines and remove the space before the first colon.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm ok with the spaces (our linter could help with that) but I dont like create a tabbed simple querys =/

return room;
}
const $set = {lastMessage : room.lastMessage, lm: room._updatedAt, ...getLowerCaseNames(room, sub.name)};
RocketChat.models.Subscriptions.update({ rid: room._id }, {$set});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not declare the set here instead of have a variable and pass it here?

@ggazzo ggazzo temporarily deployed to rocket-chat-pr-11273 June 27, 2018 20:44 Inactive
@@ -0,0 +1,5 @@
import redisMq from 'mqemitter-redis';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove this changes

@rodrigok rodrigok changed the title [FIX] sort sidebar Regression: sidebar sorting was being wrong in some cases where the rooms records were returned before the subscriptions Jun 27, 2018
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-11273 June 27, 2018 22:32 Inactive
rodrigok
rodrigok previously approved these changes Jun 27, 2018
@ggazzo ggazzo temporarily deployed to rocket-chat-pr-11273 June 27, 2018 22:51 Inactive
@rodrigok rodrigok merged commit 204baf6 into develop Jun 27, 2018
@rodrigok rodrigok added this to the 0.66.0 milestone Jun 27, 2018
@rodrigok rodrigok deleted the fix-sort-list branch June 27, 2018 23:20
@rodrigok rodrigok mentioned this pull request Jun 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants