fix: add permission and role helper class to node #1243
Conversation
WalkthroughThe diff centralizes permission-example generation into the Language base: new abstract helpers (getStaticAccessOperator, getStringQuote, getArrayOf) and a concrete getPermissionExample implementation were added to src/SDK/Language.php. Node.php gained language-agnostic helpers (getStaticAccessOperator, getStringQuote, getArrayOf, protected getPermissionPrefix) and updated getParamExample to treat TYPE_ARRAY separately, calling isPermissionString(...) ? getPermissionExample(...) : raw example. Many language subclasses added the three helper methods and had their per-language getPermissionExample implementations removed. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/SDK/Language/Node.php (2)
155-170: Add PHPDoc documentation for the new method.The method lacks documentation describing its purpose, parameter, and return value. This would help maintainers understand its role in permission example formatting.
Apply this diff to add documentation:
+ /** + * Formats a permission string example into SDK permission calls + * + * @param string $example Raw permission string example + * @return string Formatted array of SDK permission calls + */ public function getPermissionExample(string $example): string
155-155: Consider making this method protected instead of public.Since
getPermissionExampleappears to be an internal helper called only bygetParamExamplewithin the same class, it could be markedprotectedto reduce the public API surface.Apply this diff if you prefer narrower visibility:
- public function getPermissionExample(string $example): string + protected function getPermissionExample(string $example): string
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/SDK/Language/Node.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/SDK/Language/Node.php (1)
src/SDK/Language.php (2)
isPermissionString(141-145)extractPermissionParts(147-184)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: build (8.3, WebNode)
- GitHub Check: build (8.3, Swift56)
- GitHub Check: build (8.3, Ruby31)
- GitHub Check: build (8.3, KotlinJava8)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, Node20)
- GitHub Check: build (8.3, FlutterBeta)
- GitHub Check: build (8.3, CLINode16)
- GitHub Check: build (8.3, DartStable)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: build (8.3, DotNet80)
- GitHub Check: build (8.3, CLINode18)
- GitHub Check: android (client)
- GitHub Check: kotlin (server)
- GitHub Check: swift (server)
- GitHub Check: apple (client)
- GitHub Check: flutter (client)
🔇 Additional comments (1)
src/SDK/Language/Node.php (1)
143-144: Clean conditional logic for permission handling.The conditional check using
isPermissionStringproperly delegates permission detection and maintains backward compatibility for non-permission arrays.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/SDK/Language.php(2 hunks)src/SDK/Language/Dart.php(1 hunks)src/SDK/Language/Deno.php(1 hunks)src/SDK/Language/DotNet.php(1 hunks)src/SDK/Language/Go.php(1 hunks)src/SDK/Language/GraphQL.php(1 hunks)src/SDK/Language/Kotlin.php(1 hunks)src/SDK/Language/Node.php(2 hunks)src/SDK/Language/PHP.php(1 hunks)src/SDK/Language/Python.php(1 hunks)src/SDK/Language/REST.php(1 hunks)src/SDK/Language/Ruby.php(1 hunks)src/SDK/Language/Swift.php(1 hunks)src/SDK/Language/Web.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (14)
src/SDK/Language/Web.php (4)
src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)src/SDK/Language/Dart.php (3)
getStaticAccessOperator(124-127)getStringQuote(129-132)getArrayOf(134-137)src/SDK/Language/Node.php (3)
getStaticAccessOperator(15-18)getStringQuote(20-23)getArrayOf(25-28)src/SDK/Language/PHP.php (3)
getStaticAccessOperator(133-136)getStringQuote(138-141)getArrayOf(143-146)
src/SDK/Language/Go.php (2)
src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)src/SDK/Language/PHP.php (3)
getStaticAccessOperator(133-136)getStringQuote(138-141)getArrayOf(143-146)
src/SDK/Language/Kotlin.php (1)
src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)
src/SDK/Language.php (4)
src/SDK/Language/Web.php (3)
getStaticAccessOperator(17-20)getStringQuote(22-25)getArrayOf(27-30)src/SDK/Language/DotNet.php (5)
getStaticAccessOperator(149-152)getStringQuote(154-157)getArrayOf(159-162)transformPermissionAction(164-167)transformPermissionRole(169-172)src/SDK/Language/Node.php (4)
getStaticAccessOperator(15-18)getStringQuote(20-23)getArrayOf(25-28)getPermissionPrefix(30-33)src/SDK/Language/PHP.php (3)
getStaticAccessOperator(133-136)getStringQuote(138-141)getArrayOf(143-146)
src/SDK/Language/Python.php (2)
src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)src/SDK/Language/PHP.php (3)
getStaticAccessOperator(133-136)getStringQuote(138-141)getArrayOf(143-146)
src/SDK/Language/REST.php (1)
src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)
src/SDK/Language/PHP.php (3)
src/SDK/Language/Web.php (3)
getStaticAccessOperator(17-20)getStringQuote(22-25)getArrayOf(27-30)src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)src/SDK/Language/DotNet.php (3)
getStaticAccessOperator(149-152)getStringQuote(154-157)getArrayOf(159-162)
src/SDK/Language/Deno.php (3)
src/SDK/Language/Web.php (3)
getStaticAccessOperator(17-20)getStringQuote(22-25)getArrayOf(27-30)src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)src/SDK/Language/Node.php (3)
getStaticAccessOperator(15-18)getStringQuote(20-23)getArrayOf(25-28)
src/SDK/Language/Dart.php (3)
src/SDK/Language/Web.php (3)
getStaticAccessOperator(17-20)getStringQuote(22-25)getArrayOf(27-30)src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)src/SDK/Language/DotNet.php (3)
getStaticAccessOperator(149-152)getStringQuote(154-157)getArrayOf(159-162)
src/SDK/Language/Node.php (1)
src/SDK/Language.php (6)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)getPermissionPrefix(220-223)isPermissionString(160-164)getPermissionExample(252-278)
src/SDK/Language/Swift.php (1)
src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)
src/SDK/Language/DotNet.php (1)
src/SDK/Language.php (5)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)transformPermissionAction(231-234)transformPermissionRole(242-245)
src/SDK/Language/GraphQL.php (1)
src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)
src/SDK/Language/Ruby.php (2)
src/SDK/Language/Web.php (3)
getStaticAccessOperator(17-20)getStringQuote(22-25)getArrayOf(27-30)src/SDK/Language.php (3)
getStaticAccessOperator(39-39)getStringQuote(45-45)getArrayOf(52-52)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: node (server)
- GitHub Check: android (client)
- GitHub Check: apple (client)
- GitHub Check: build (8.3, PHP80)
- GitHub Check: build (8.3, DotNet90)
- GitHub Check: build (8.3, DotNet80)
🔇 Additional comments (22)
src/SDK/Language/Deno.php (1)
15-28: LGTM! Helper methods implemented correctly for Deno.The static access operator (
.), string quote ('), and array syntax ([...]) align with Deno/TypeScript conventions.src/SDK/Language.php (2)
35-52: LGTM! Well-designed language abstraction.The three abstract methods provide a clean interface for language-specific syntax elements (static access, string quotes, array literals).
216-245: LGTM! Sensible defaults for permission transformations.The protected helpers provide appropriate no-op defaults that languages can selectively override (e.g., Node.php adds 'sdk.' prefix, DotNet uses ucfirst).
src/SDK/Language/DotNet.php (1)
149-172: LGTM! C#-specific implementations are correct.The helper methods correctly implement C# syntax:
- Static access with
.operator- Double quotes for string literals
new List<string> { ... }for array initializationucfirsttransformations for PascalCase naming (e.g.,Permission.Read(),Role.User())src/SDK/Language/Node.php (2)
15-33: LGTM! Node.js helpers correctly implemented.The helper methods use appropriate JavaScript/Node.js syntax:
- Static access with
.operator- Single quotes for string literals
- Array literal syntax
[...]sdk.prefix for Permission/Role classes as requested in past review feedbackNote: The quote escaping concern from the previous review now applies to the base
Language.phpclass (flagged separately).
162-163: LGTM! Proper delegation to base class permission example generation.The array handling correctly checks for permission strings and delegates to
getPermissionExample()from the base class.src/SDK/Language/Go.php (1)
47-60: LGTM! Go-specific implementations are correct.The helper methods correctly implement Go syntax:
- Static access with
.operator (e.g.,permission.Read())- Double quotes for string literals
- Slice literal syntax
[...]src/SDK/Language/REST.php (1)
15-28: LGTM! REST/JSON syntax implementations are appropriate.The helper methods use generic JSON-compatible syntax:
- Static access with
.operator- Double quotes for JSON string literals
- Array literal syntax
[...]src/SDK/Language/GraphQL.php (1)
15-28: LGTM! GraphQL syntax implementations are correct.The helper methods implement GraphQL-compatible syntax:
- Static access with
.operator- Double quotes for string literals (GraphQL standard)
- Array literal syntax
[...]src/SDK/Language/Dart.php (1)
124-137: LGTM! Dart-specific implementations are correct.The helper methods correctly implement Dart syntax:
- Static access with
.operator- Single quotes for string literals (Dart convention)
- List literal syntax
[...]src/SDK/Language/Ruby.php (2)
89-102: LGTM! Helper methods correctly implement Ruby syntax.The three new helper methods properly implement the abstract methods declared in the base Language class with Ruby-specific syntax.
323-323: No issues found—the method is inherited from the parent class.The
getPermissionExample()method is defined as a public function in the parentLanguageclass (src/SDK/Language.php:252), making it accessible to theRubyclass through inheritance. The call at line 323 is valid and will not cause a fatal error.Likely an incorrect or invalid review comment.
src/SDK/Language/Web.php (2)
17-30: LGTM! Helper methods correctly implement JavaScript/Web syntax.The three new helper methods properly implement the abstract methods declared in the base Language class with JavaScript-specific syntax.
168-168: No action required. ThegetPermissionExample()method is properly defined in the parentLanguageclass (line 252) and is accessible toWebthrough inheritance (Web→JS→Language). The method is not removed and will not cause a fatal error.Likely an incorrect or invalid review comment.
src/SDK/Language/Swift.php (2)
104-117: LGTM! Helper methods correctly implement Swift syntax.The three new helper methods properly implement the abstract methods declared in the base Language class with Swift-specific syntax.
446-446: The method is available through inheritance—no issue.The
getPermissionExample()method is publicly defined in the parentLanguageclass (line 252 ofsrc/SDK/Language.php), andSwiftextendsLanguage(line 8). The call at line 446 will work correctly through inheritance and will not cause a runtime error.Likely an incorrect or invalid review comment.
src/SDK/Language/Kotlin.php (2)
102-115: LGTM! Helper methods correctly implement Kotlin syntax.The three new helper methods properly implement the abstract methods declared in the base Language class with Kotlin-specific syntax, including the use of
listOf()for array construction.
274-276: <!-- [scratchpad_start]
[task verify_review_comment_1/1]
Verify getPermissionExample() method availability in Kotlin.php at line 276[observations]
- Line 276 in Kotlin.php calls: $this->getPermissionExample($example)
- Kotlin extends Language (src/SDK/Language.php:8)
- getPermissionExample is defined in src/SDK/Language.php:252 as public function
- Method signature: public function getPermissionExample(string $example): string
- Method is accessible to child classes via inheritance
[analysis]
Review comment incorrectly claims method was removed. Evidence shows:
- Method exists in parent Language class
- Method is public and accessible to children
- Kotlin inherits from Language
- No fatal error will occur
[done]
Review comment is INCORRECT - method is available and properly inherited
The
getPermissionExample()method is not removed and is properly available. The method is defined in the parentLanguageclass at line 252 ofsrc/SDK/Language.phpas a public method, making it accessible to theKotlinclass via inheritance. The call at line 276 ofKotlin.phpis valid and will not cause a runtime error.Likely an incorrect or invalid review comment.
src/SDK/Language/PHP.php (2)
133-146: LGTM! Helper methods correctly implement PHP syntax.The three new helper methods properly implement the abstract methods declared in the base Language class with PHP-specific syntax, correctly using
::for static member access.
377-377: Method is properly inherited from parent class—no issue exists.The
getPermissionExample()method is defined in the parentLanguageclass atsrc/SDK/Language.php:252as a public method. SincePHPextendsLanguage, the method is accessible via$this->on line 377. This is standard PHP inheritance and will not cause a fatal error.Likely an incorrect or invalid review comment.
src/SDK/Language/Python.php (2)
87-100: LGTM! Helper methods correctly implement Python syntax.The three new helper methods properly implement the abstract methods declared in the base Language class with Python-specific syntax.
360-360: No issues found — method is properly inherited.The
getPermissionExample()method is defined as a public function in the parentLanguageclass (src/SDK/Language.php:252) and is correctly accessible from thePythonclass via inheritance. The call on line 360 will resolve to the parent method at runtime without error.
Summary by CodeRabbit
Improvements
Bug Fixes