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(methods): remove deprecated canAccessRoom method #35436

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

blackmamba1231
Copy link

Proposed changes

This PR removes the deprecated canAccessRoom method that was scheduled for removal after v5.0 but is still present in v7.4.0. The method was only enabled when the environment variable ALLOW_CANACCESSROOM_METHOD was set to "yes" or "true".

Changes made:

  • Removed the canAccessRoom.ts file
  • Updated the index.ts file to remove the import for the canAccessRoom method

Issue(s)

Fixes #35432

Steps to test or reproduce

  1. Verify that the canAccessRoom method is no longer available
  2. Ensure that applications using the async alternative canAccessRoomAsync continue to work correctly
  3. No functionality should be affected as this method was already deprecated and only available when explicitly enabled via environment variable

Further comments

This change is part of the ongoing effort to clean up deprecated code in the Rocket.Chat codebase. The method was marked for removal after v5.0 and we're now at v7.4.0, so it's time to complete the deprecation process.

@blackmamba1231 blackmamba1231 requested a review from a team as a code owner March 8, 2025 12:10
Copy link
Contributor

dionisio-bot bot commented Mar 8, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is targeting the wrong base branch. It should target 8.0.0, but it targets 7.5.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Mar 8, 2025

⚠️ No Changeset found

Latest commit: bc0b500

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Gustrb
Copy link
Contributor

Gustrb commented Mar 8, 2025

Even though this was already scheduled for removal I don't see why removing it in a minor version. We should probably wait for 8.0 so we can remove it without the concern of introducing breaking changes

@blackmamba1231
Copy link
Author

blackmamba1231 commented Mar 8, 2025

I appreciate your perspective on following semantic versioning principles. You make a valid point about avoiding breaking changes in minor versions.

While the method was marked for removal after v5.0 and is currently only enabled via an environment variable, I understand the concern about potential disruption to users who might still be relying on it.

I'm happy to adjust the PR to target v8.0 instead. Would you like me to update the PR description to reflect this timing, or would you prefer to handle this differently? I'll follow the team's guidance on the appropriate approach for handling this deprecated functionality.

@debdutdeb debdutdeb added this to the 8.0.0 milestone Mar 8, 2025
@debdutdeb debdutdeb marked this pull request as draft March 8, 2025 17:19
@blackmamba1231 blackmamba1231 marked this pull request as ready for review March 9, 2025 20:13
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.

Remove deprecated canAccessRoom method still present in v7.4.0
3 participants