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

api: Add support for GET /realm/filters. #66

Merged
merged 3 commits into from
Apr 28, 2019

Conversation

Anupam-dagar
Copy link
Contributor

@Anupam-dagar Anupam-dagar commented Aug 22, 2018

Issue #63
@aero31aero I found out that the emoji.js file present already in the repository does the same which this PR does. I am not sure to remove that and keep this or remove this and keep that.
IMO removing that will be good as there is one more endpoint to implement for realm.

Update: This PR consist of 2 commits as follows:

  1. This commit add endpoint to get realm emojis.
  2. This commit add endpoint to get realm filters.

Testing:
Working of the endpoints has been tested by node tests and by doing a real time query on a server.

@Anupam-dagar Anupam-dagar changed the title Add endpoint to get realm emojis. Add API endpoints for realm. Aug 23, 2018
@aero31aero
Copy link
Member

@arpith since you are probably already using the emoji feature, what is your take on this?

I feel that having z.filters.retrieve() (new) and z.emojis.retrieve() (already exists) should be better than keeping them under a realm. I'd prefer abstracting out the realm detail from the API as that's just our internal implementation. Anyone dealing with a single API key is dealing with a single realm at a time.

@arpith
Copy link
Collaborator

arpith commented Aug 27, 2018

Yeah I agree!

@aero31aero
Copy link
Member

@Anupam-dagar can you rebase this and follow the suggestion in my previous comment?

@Anupam-dagar
Copy link
Contributor Author

@aero31aero please review

@aero31aero aero31aero changed the title Add API endpoints for realm. api: Add support for GET /realm/filters. Apr 28, 2019
@aero31aero aero31aero merged commit 7a28567 into zulip:master Apr 28, 2019
@Milind712000 Milind712000 mentioned this pull request Jan 5, 2020
30 tasks
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