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

[FIX] Remove deprecated permissions api endpoint #12285

Closed
wants to merge 3 commits into from
Closed

[FIX] Remove deprecated permissions api endpoint #12285

wants to merge 3 commits into from

Conversation

kaiiiiiiiii
Copy link
Contributor

The endpoint "permissions" is deprecated and will be removed after version v0.69

Cheers,
Kai

geekgonecrazy
geekgonecrazy previously approved these changes Oct 17, 2018
@geekgonecrazy
Copy link
Contributor

@RocketChat/android @RocketChat/ios I don't think you guys are using this. But as typical heads up. Since we did schedule to be deprecated

@rafaelks
Copy link
Contributor

@geekgonecrazy @kaiiiiiiiii Wow, thanks for the heads up! Yes, we are using it! What API should we use?

@MarcosSpessatto
Copy link
Member

@rafaelks you should use permissions.list, it was renamed.

@geekgonecrazy geekgonecrazy dismissed their stale review October 17, 2018 20:12

Lets no accidentally merge this yet

@rafaelks
Copy link
Contributor

Copy link
Contributor

@rafaelks rafaelks left a comment

Choose a reason for hiding this comment

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

Let's hold on until 0.73 at least? Mobile apps needs to be updated.

@MarcosSpessatto
Copy link
Member

@rafaelks yes of course, but the endpoint has been deprecated since v0.66, https://rocket.chat/docs/developer-guides/deprecation/.

@sampaiodiego
Copy link
Member

I'll add this to 0.72 milestone so we don't forgot to merge.

@rafaelks can you update apps before that? it's a very small change, the response is now within permissions field, see docs

@sampaiodiego sampaiodiego added this to the 0.72.0 milestone Oct 20, 2018
@rafaelks
Copy link
Contributor

@sampaiodiego If 0.72 is the next version, I would say let's wait until 0.73 because we still need to ship new versions of the mobile apps with this changes. Is there any rush to merge this before that?

@rodrigok rodrigok modified the milestones: 0.72.0, 0.73.0 Nov 1, 2018
@rafaelks
Copy link
Contributor

Guys, can we hold on this change to 0.74? iOS and Android still haven't released this change to the store (iOS is done already but not released and Android is on the schedule to do it).

@MarcosSpessatto
Copy link
Member

@rafaelks I needed to change the endpoint again, so I ended up deprecating permissions.list. When you are switching to the new endpoint, please switch to permissions.listAll. The reason for this new change is described in this issue. I already added the new endpoint in the docs along with the new deprecation notice.(Docs). Thanks.

@rafaelks
Copy link
Contributor

@MarcosSpessatto Got it! OK, let's plan to deprecate the APIs 8~12 versions after we create the deprecation message, because currently the iOS and Android apps are supporting 10 version older (0.62+) of the back-end and would be really painful to start supporting all these deprecations right now, unless there's a serious performance issue or any other reason that would really require this update.

@rodrigok rodrigok modified the milestones: 0.73.0, 0.74.0 Dec 20, 2018
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.

6 participants