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 "value" parameter to /api/v1/chat.react #10370

Closed
cardoso opened this issue Apr 6, 2018 · 4 comments
Closed

[API] Add "value" parameter to /api/v1/chat.react #10370

cardoso opened this issue Apr 6, 2018 · 4 comments

Comments

@cardoso
Copy link
Contributor

cardoso commented Apr 6, 2018

Currently /api/v1/chat.react works like a toggle. We call it once, it either removes or adds the reaction.

This behavior causes some difficulties when working with optimistic UI updates.

My proposal is we add a "value" parameter.

"value": true = adds the reaction (returns error if already added)
"value": false = removes the reaction (returns error if not found)
"value" not present = retains old behavior (doesn't break old API usage)

I think this would qualify as a non-breaking update to an existing API

@rafaelks @MarcosSpessatto @graywolf336

What do you think?

@graywolf336
Copy link
Contributor

As long as ensure we don't break the current implementation, I would be fine with that.

Although, this isn't a simple change because it will require some changes to how the method it internally calls works.

@rafaelks
Copy link
Contributor

rafaelks commented Apr 6, 2018

@cardoso I support the change, but I agree that we cannot break the current implementation. 👍

@cardoso
Copy link
Contributor Author

cardoso commented Apr 9, 2018

@graywolf336 @rafaelks that's the idea,guys. If the API consumer doesn't provide the "value" parameter, the API should behave the same. That's not considered a breaking change, right?

@nemani
Copy link
Contributor

nemani commented Apr 10, 2018

Seems non-breaking to me.

@MarcosSpessatto MarcosSpessatto added this to the 0.65.0 milestone Apr 26, 2018
@rodrigok rodrigok modified the milestones: 0.65.0, 0.66.0 May 22, 2018
ggazzo pushed a commit that referenced this issue Jun 20, 2018
… a setter (#10447)

Add parameter to REST chat.react endpoint, to make it work like a setter
Closes #10370.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants