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.

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

[NEW] REST API endpoint /directory #10442

Merged
merged 4 commits into from
Apr 19, 2018
Merged

Conversation

MarcosSpessatto
Copy link
Contributor

Add REST /directory endpoint.
Closes #9961.

@MarcosSpessatto MarcosSpessatto added this to the 0.64.0 milestone Apr 13, 2018
@MarcosSpessatto MarcosSpessatto self-assigned this Apr 13, 2018
@MarcosSpessatto MarcosSpessatto added this to Desireable in 0.64.0 via automation Apr 13, 2018
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10442 April 16, 2018 22:46 Inactive
@theorenck theorenck moved this from Desireable to Review/QA in 0.64.0 Apr 17, 2018
});
});

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
0.64.0 automation moved this from Review/QA to Done 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
No open projects
0.64.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants