From 58b69b682dc30bad06bf19ece1e396cc01795569 Mon Sep 17 00:00:00 2001 From: IQBAL HASAN Date: Thu, 25 Sep 2025 12:39:47 +0600 Subject: [PATCH] fix: logical operators without spaces and hyphenated permissions - Fix logical operators (||, &&, |, &) not working without spaces - Add support for permission names with hyphens (e.g., properties.view-all) - Improve operator normalization using negative lookbehind - Add comprehensive test suite for no-spaces scenarios - Update regex pattern to include hyphens in permission names - Bump version to 1.1.3 Fixes: properties.view-all||properties.view-own expressions Tests: 12 new test cases covering edge cases and hyphenated permissions --- CHANGELOG.md | 19 ++ .../logical-operators-no-spaces.test.tsx | 194 ++++++++++++++++++ hooks/use-permissions.tsx | 20 +- package.json | 2 +- 4 files changed, 224 insertions(+), 11 deletions(-) create mode 100644 __tests__/logical-operators-no-spaces.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 020e3ab..58329b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## v1.1.3 - 2025-01-27 + +### Fixed + +- **Logical operators without spaces**: Fixed issue where logical operators (`||`, `&&`, `|`, `&`) were not working when used without spaces around them (e.g., `properties.view-all||properties.view-own`) +- **Permission names with hyphens**: Updated regex pattern to properly support permission names containing hyphens (e.g., `properties.view-all`, `user-profile.edit`) +- **Operator normalization**: Improved operator normalization logic to prevent double replacement issues + +### Added + +- **Comprehensive test coverage**: Added extensive test suite for logical operators without spaces, covering various edge cases and scenarios +- **Hyphenated permission support**: Full support for permission names with hyphens in logical expressions + +### Technical Details + +- Updated regex pattern from `[a-zA-Z0-9_.*?]*` to `[a-zA-Z0-9_.*?-]*` to include hyphens +- Fixed operator normalization using negative lookbehind to prevent double replacement +- Added 12 new test cases covering no-spaces scenarios, hyphenated permissions, and edge cases + ## v1.1.2 - 2025-09-19 ### What's Changed diff --git a/__tests__/logical-operators-no-spaces.test.tsx b/__tests__/logical-operators-no-spaces.test.tsx new file mode 100644 index 0000000..4879b66 --- /dev/null +++ b/__tests__/logical-operators-no-spaces.test.tsx @@ -0,0 +1,194 @@ +import { renderHook } from '@testing-library/react'; +import { usePermissions } from '../hooks/use-permissions'; + +// Mock @inertiajs/react +jest.mock('@inertiajs/react', () => ({ + usePage: jest.fn(), +})); + +import { usePage } from '@inertiajs/react'; +const mockUsePage = usePage as jest.MockedFunction; + +describe('Logical Operators Without Spaces', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + const mockPageProps = (userPermissions: string[] = []) => { + mockUsePage.mockReturnValue({ + props: { + auth: { + user: { + permissions: userPermissions, + }, + }, + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any); + }; + + describe('OR operators without spaces', () => { + it('should handle || without spaces around operators', () => { + mockPageProps(['properties.view-all', 'properties.view-own']); + + const { result } = renderHook(() => usePermissions()); + + // Test various OR expressions without spaces + expect(result.current.hasPermission('properties.view-all||properties.view-own')).toBe(true); + expect(result.current.hasPermission('users.create||posts.view')).toBe(false); // Neither permission exists + expect(result.current.hasPermission('properties.view-all||users.create')).toBe(true); // One permission exists + }); + + it('should handle single | without spaces around operators', () => { + mockPageProps(['properties.view-all', 'properties.view-own']); + + const { result } = renderHook(() => usePermissions()); + + // Test single | expressions without spaces + expect(result.current.hasPermission('properties.view-all|properties.view-own')).toBe(true); + expect(result.current.hasPermission('properties.view-all|users.create')).toBe(true); // One permission exists + expect(result.current.hasPermission('users.create|posts.view')).toBe(false); // Neither permission exists + }); + }); + + describe('AND operators without spaces', () => { + it('should handle && without spaces around operators', () => { + mockPageProps(['properties.view-all', 'properties.view-own']); + + const { result } = renderHook(() => usePermissions()); + + // Test various AND expressions without spaces + expect(result.current.hasPermission('properties.view-all&&properties.view-own')).toBe(true); + expect(result.current.hasPermission('properties.view-all&&users.create')).toBe(false); // One permission missing + expect(result.current.hasPermission('users.create&&posts.view')).toBe(false); // Both permissions missing + }); + + it('should handle single & without spaces around operators', () => { + mockPageProps(['properties.view-all', 'properties.view-own']); + + const { result } = renderHook(() => usePermissions()); + + // Test single & expressions without spaces + expect(result.current.hasPermission('properties.view-all&properties.view-own')).toBe(true); + expect(result.current.hasPermission('properties.view-all&users.create')).toBe(false); // One permission missing + expect(result.current.hasPermission('users.create&posts.view')).toBe(false); // Both permissions missing + }); + }); + + describe('Complex expressions without spaces', () => { + it('should handle parentheses with no spaces around operators', () => { + mockPageProps(['properties.view-all', 'admin.access']); + + const { result } = renderHook(() => usePermissions()); + + // Test complex expressions without spaces + expect(result.current.hasPermission('(properties.view-all||users.create)&&admin.access')).toBe(true); + expect(result.current.hasPermission('(properties.view-all||users.create)&&admin.delete')).toBe(false); + expect(result.current.hasPermission('(users.create||posts.view)&&admin.access')).toBe(false); + }); + + it('should handle mixed operators without spaces', () => { + mockPageProps(['properties.view-all', 'admin.access', 'reports.read']); + + const { result } = renderHook(() => usePermissions()); + + // Test mixed operators without spaces + expect(result.current.hasPermission('properties.view-all&&admin.access||reports.read')).toBe(true); + expect(result.current.hasPermission('properties.view-all&admin.access|reports.read')).toBe(true); + expect(result.current.hasPermission('users.create&&admin.access||reports.read')).toBe(true); // reports.read exists + }); + }); + + describe('Permission names with hyphens', () => { + it('should handle permission names with hyphens in expressions without spaces', () => { + mockPageProps(['user-profile.edit', 'api-access.read', 'system-config.update']); + + const { result } = renderHook(() => usePermissions()); + + // Test hyphenated permission names without spaces + expect(result.current.hasPermission('user-profile.edit||api-access.read')).toBe(true); + expect(result.current.hasPermission('user-profile.edit&&api-access.read')).toBe(true); + expect(result.current.hasPermission('user-profile.edit&&system-config.update')).toBe(true); + expect(result.current.hasPermission('user-profile.edit&&users.create')).toBe(false); // users.create doesn't exist + }); + + it('should handle complex hyphenated permission expressions', () => { + mockPageProps(['user-profile.edit', 'api-access.read', 'admin-panel.access']); + + const { result } = renderHook(() => usePermissions()); + + // Test complex expressions with hyphenated permissions + expect(result.current.hasPermission('(user-profile.edit||api-access.read)&&admin-panel.access')).toBe(true); + expect(result.current.hasPermission('user-profile.edit&&(api-access.read||system-config.update)')).toBe(true); + expect(result.current.hasPermission('(user-profile.edit||api-access.read)&&system-config.update')).toBe(false); + }); + }); + + describe('Boolean literals without spaces', () => { + it('should handle boolean literals in expressions without spaces', () => { + mockPageProps(['properties.view-all']); + + const { result } = renderHook(() => usePermissions()); + + // Test boolean literals without spaces + expect(result.current.hasPermission('true||properties.view-all')).toBe(true); + expect(result.current.hasPermission('false||properties.view-all')).toBe(true); + expect(result.current.hasPermission('true&&properties.view-all')).toBe(true); + expect(result.current.hasPermission('false&&properties.view-all')).toBe(false); + expect(result.current.hasPermission('properties.view-all||true')).toBe(true); + expect(result.current.hasPermission('properties.view-all&&false')).toBe(false); + }); + }); + + describe('Edge cases without spaces', () => { + it('should handle expressions with multiple consecutive operators', () => { + mockPageProps(['properties.view-all', 'properties.view-own']); + + const { result } = renderHook(() => usePermissions()); + + // These should be handled gracefully (though they're not valid expressions) + expect(result.current.hasPermission('properties.view-all||||properties.view-own')).toBe(false); // Invalid + expect(result.current.hasPermission('properties.view-all&&&&properties.view-own')).toBe(false); // Invalid + }); + + it('should handle expressions starting or ending with operators', () => { + mockPageProps(['properties.view-all']); + + const { result } = renderHook(() => usePermissions()); + + // These should be handled gracefully + expect(result.current.hasPermission('||properties.view-all')).toBe(false); // Invalid + expect(result.current.hasPermission('properties.view-all||')).toBe(false); // Invalid + expect(result.current.hasPermission('&&properties.view-all')).toBe(false); // Invalid + expect(result.current.hasPermission('properties.view-all&&')).toBe(false); // Invalid + }); + }); + + describe('Comparison with spaced expressions', () => { + it('should produce the same results with and without spaces', () => { + mockPageProps(['properties.view-all', 'properties.view-own', 'admin.access']); + + const { result } = renderHook(() => usePermissions()); + + // Test that expressions with and without spaces produce the same results + const expressions = [ + 'properties.view-all||properties.view-own', + 'properties.view-all || properties.view-own', + 'properties.view-all&&admin.access', + 'properties.view-all && admin.access', + 'properties.view-all|properties.view-own', + 'properties.view-all | properties.view-own', + 'properties.view-all&admin.access', + 'properties.view-all & admin.access', + '(properties.view-all||properties.view-own)&&admin.access', + '(properties.view-all || properties.view-own) && admin.access', + ]; + + expressions.forEach(expr => { + const hasPermission = result.current.hasPermission(expr); + expect(hasPermission).toBeDefined(); + expect(typeof hasPermission).toBe('boolean'); + }); + }); + }); +}); diff --git a/hooks/use-permissions.tsx b/hooks/use-permissions.tsx index 88cf094..c559c18 100644 --- a/hooks/use-permissions.tsx +++ b/hooks/use-permissions.tsx @@ -96,27 +96,27 @@ export function usePermissions(permissions?: string[]) { if (expression.trim() === 'false') return false; // Normalize logical operators to JavaScript equivalents - // Process single operators first, then ensure double operators are correct + // First, add spaces around operators to ensure proper parsing let jsExpression = expression - .replace(/\|(?!\|)/g, '||') // Single | becomes || - .replace(/&(?!&)/g, '&&'); // Single & becomes && + .replace(/\|\|/g, ' || ') // Add spaces around || + .replace(/&&/g, ' && ') // Add spaces around && + .replace(/(?