Skip to content

Conversation

@MarcosSpessatto
Copy link
Contributor

Add REST /directory endpoint.
Closes #9961.

Add REST /directory endpoint
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10442 April 16, 2018 22:46 Inactive
@theorenck theorenck requested a review from sampaiodiego April 18, 2018 18:49
});
});

describe('/settings.oauth', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be moved to the settings one and not the miscellaneous one? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

oops.. looks it's duplicated of this one.

@MarcosSpessatto can you please remove this test from here?

});
});

describe('/settings.oauth', () => {
Copy link
Member

Choose a reason for hiding this comment

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

oops.. looks it's duplicated of this one.

@MarcosSpessatto can you please remove this test from here?

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10442 April 18, 2018 20:53 Inactive
graywolf336
graywolf336 previously approved these changes Apr 18, 2018
limit: count
}));

if (result) {
Copy link
Member

Choose a reason for hiding this comment

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

I think as a good practice, the unexpected condition should be treated specially and not the successful one.

so I will ask you to change this here, so the if will trigger the failure.

@rodrigok rodrigok changed the title [NEW] Add REST /directory endpoint [NEW] REST API endpoint /directory Apr 19, 2018
@rodrigok rodrigok merged commit ee06a91 into develop Apr 19, 2018
@rodrigok rodrigok deleted the feature/rest-directory-endpoint branch April 19, 2018 22:40
@graywolf336
Copy link
Contributor

@rodrigok did you test this yourself? We were investigating an issue with the rest API not working on the deployed heroku instance.

@sampaiodiego
Copy link
Member

@graywolf336 the last instance I tried on heroku was built based on develop branch instead of PR's code.. not sure it was the same for this one, just sharing

@graywolf336
Copy link
Contributor

Well it had the directory endpoint, so it was working to a certain point... 🤔 oh well.

@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.

6 participants