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: Resolve incorrect permission names and improve search functionality #34843

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

AyushKumar123456789
Copy link

@AyushKumar123456789 AyushKumar123456789 commented Dec 29, 2024

Proposed changes (including videos or screenshots)

  • Fixed: Incorrect translation rendering (en to en) for three specific permission IDs in PermissionRow out of 196 permissions. This issue caused:
    1. Search not functioning correctly (you have to search for id not the rendered name).
    2. Incorrect sorting of permission names, as both operations were performed on _id instead of the rendered permission name.
- Permissions row name rendered incorrectly in the UI:

    Rendered Name in UI                       ID
    Change Some Settings                      manage-selected-settings
    Modify Setting-Based Permissions          access-setting-permissions
    Bypass rate limit for REST API            api-bypass-rate-limita
  • Enhanced: Search functionality in PermissionsTableFilter to correctly handle spaces .

Screenshots:

  1. Before Fix:
WrongSearch.mp4
  1. After Fix:
AfterFix.mp4

Issue(s)

Similar to this issue


Steps to test or reproduce

  1. Navigate to Administration → Workspace → Permissions.
  2. Search for Modify Setting-Based Permissions (third from start in the list):
    • Before Fix: The search does not find the permission unless you search for access-setting-permissions (its _id).
    • Additionally, spaces are not recognized, and you need to use - instead of a space.
  3. Check sorting to ensure permission names are displayed in the correct order after the fix.

Further comments

This fix ensures the following:

  1. Permission names are correctly rendered and displayed in PermissionRow, matching their _id.
  2. The search functionality accommodates spaces .
  3. Sorting is now aligned with the corrected rendered permission names.
  4. Minimal changes were made to maintain compatibility with the existing codebase.

Let me know if there are any suggestions or additional refinements required.

@AyushKumar123456789 AyushKumar123456789 requested a review from a team as a code owner December 29, 2024 06:00
Copy link
Contributor

dionisio-bot bot commented Dec 29, 2024

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 missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

Copy link

changeset-bot bot commented Dec 29, 2024

🦋 Changeset detected

Latest commit: 7de5c39

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 37 packages
Name Type
@rocket.chat/i18n Patch
@rocket.chat/meteor Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/ui-client Patch
@rocket.chat/ui-voip Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/gazzodown Patch
@rocket.chat/livechat Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-video-conf Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/ddp-client Patch
@rocket.chat/freeswitch Patch
@rocket.chat/model-typings Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/network-broker Patch
@rocket.chat/models Patch
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

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

@CLAassistant
Copy link

CLAassistant commented Dec 29, 2024

CLA assistant check
All committers have signed the CLA.

@reetp
Copy link

reetp commented Dec 29, 2024

I believe this is meant to fix #28227

There are already PRs open on this.

Also:

  • No bug reference.

  • No changeset.

  • Contributor licence greement not signed.

Asking the team to take a look.

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