Skip to content

Conversation

@MarcosSpessatto
Copy link
Contributor

Fix REST spotlight to allow searches with # and @.
Closes #8205.

Marcos Defendi added 2 commits April 10, 2018 15:25
Fix REST spotlight endpoint, add same logic of front to the backend, to search users and channels with # and @
});
});

describe('[/spotlight]', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a test that ensures the #channel works as expected? Also, actually ensure the result contains member and room objects instead of the below length that checks for 0?

I know this was just moved to the correct test file, but since we are adding a new feature to the endpoint I think it would be great to test it.

const { query } = this.queryParams;

const result = Meteor.runAsUser(this.userId, () =>
Meteor.call('spotlight', query, null, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a small change, but the { rooms: true, users: true } isn't required since that's the default... so it could just be Meteor.call('spotlight, query) 🤔

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10410 April 11, 2018 15:06 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10410 April 20, 2018 15:19 Inactive
@rodrigok rodrigok dismissed graywolf336’s stale review April 20, 2018 18:22

Changes was made

@rodrigok rodrigok changed the title [FIX] Fix REST spotlight to allow searches with # and @ [FIX] REST spotlight API wasn't allowing searches with # and @ Apr 20, 2018
@rodrigok rodrigok merged commit fe663cf into develop Apr 20, 2018
@rodrigok rodrigok deleted the fix-rest-spotlight branch April 20, 2018 18:24
@rodrigok rodrigok mentioned this pull request Apr 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants