-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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(core): ensure consistent casing in resource names for access control #6021
fix(core): ensure consistent casing in resource names for access control #6021
Conversation
🦋 Changeset detectedLatest commit: 2d0cde5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
Hey @JayBhensdadia thanks for the PR! Can you add some tests to reproduce the problem, so we make sure these changes fixes the issue? |
sure, i'll do that! |
e4d98ab
to
cfd4dd7
Compare
hey, @BatuhanW |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the contribution! Great PR 🚀
Can you add a changeset for the packages as well? You can check out our Contributing Guide to learn how. It will be great if you can add two changesets, one for @refinedev/ui-tests
and other for UI packages 🙏
Let me know if any help is needed 🤖
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes in this file is OK to keep but we can revert the changes in versioned_docs/version-3.xx.xx
🙏
expect(() => getByText("Users")).toThrow(); | ||
}); | ||
|
||
it("should handle camelcased resource names correctly", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests looks great and nice to see you've implemented them in ui-tests
🤝
Sure, I'll start working on it! |
hey @aliemir |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution @JayBhensdadia 🎖️ I reverted the changes in documentation/versioned_docs
and kept them untouched 🙏
Sorry, I missed that change about versioned_docs ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @JayBhensdadia
PR Checklist
Bugs / Features
What is the current behavior?
The current behavior intermittently lower cases resources due to inconsistent handling of resource names.
What is the new behavior?
The new behavior ensures consistent casing of resource names, addressing the issue of intermittent lower casing.
fixes #6004
Notes for reviewers
This PR addresses the intermittent issue where resource names were being lower-cased. The fix ensures consistent casing throughout the access control logic.