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

[PLA-1946] Add after param validation #268

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

enjinabner
Copy link
Contributor

@enjinabner enjinabner commented Oct 14, 2024

PR Type

enhancement, tests


Description

  • Added a new validation message for the 'after' parameter in the validation language file.
  • Implemented validation rules and custom logic for the 'after' parameter in the GetTokensQuery.
  • Added a test case to ensure the 'after' parameter is validated correctly, checking for invalid encoded data.

Changes walkthrough 📝

Relevant files
Enhancement
validation.php
Add validation message for 'after' parameter                         

lang/en/validation.php

  • Added a new validation message for invalid 'after' parameter.
+1/-0     
GetTokensQuery.php
Implement 'after' parameter validation in GetTokensQuery 

src/GraphQL/Schemas/Primary/Substrate/Queries/GetTokensQuery.php

  • Added validation rules for 'after' parameter.
  • Implemented custom validation logic for 'after' parameter.
  • +19/-0   
    Tests
    GetTokensTest.php
    Add test for 'after' parameter validation                               

    tests/Feature/GraphQL/Queries/GetTokensTest.php

  • Added a test to validate the 'after' parameter.
  • Ensured the test checks for invalid encoded data.
  • +13/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @enjinabner enjinabner self-assigned this Oct 14, 2024
    @github-actions github-actions bot added enhancement New feature or request tests labels Oct 14, 2024
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Typo in Validation Message
    The validation message for 'invalid_after' contains a typo "and" which should be "an".

    Error Handling
    The custom validation rule for 'after' uses a translation key directly in the fail function which might not correctly translate the message depending on the configuration.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Correct a typo in the error message to enhance message clarity

    Correct the typo in the validation message for 'invalid_after' to improve clarity.

    lang/en/validation.php [43]

    -'invalid_after' => 'The :attribute contains and invalid encoded data.',
    +'invalid_after' => 'The :attribute contains an invalid encoded data.',
    Suggestion importance[1-10]: 9

    Why: The suggestion corrects a typo in the error message, improving clarity and professionalism in user-facing text, which is important for user experience.

    9
    Possible bug
    Add null checks to prevent potential null pointer exceptions

    Ensure that the Cursor::fromEncoded($value) method call is safely handled to avoid
    potential null pointer exceptions.

    src/GraphQL/Schemas/Primary/Substrate/Queries/GetTokensQuery.php [95-97]

    -if (!Arr::get(Cursor::fromEncoded($value)?->toArray(), 'identifier')) {
    +$cursor = Cursor::fromEncoded($value);
    +if (!$cursor || !Arr::get($cursor->toArray(), 'identifier')) {
         $fail('enjin-platform::validation.invalid_after')->translate();
     }
    Suggestion importance[1-10]: 8

    Why: The suggestion adds a null check to prevent potential null pointer exceptions, which is crucial for ensuring the robustness and reliability of the code.

    8
    Maintainability
    Improve method naming for better code readability and maintainability

    Use a more descriptive method name than rules to clarify the purpose of the
    validation rules within the context of the application.

    src/GraphQL/Schemas/Primary/Substrate/Queries/GetTokensQuery.php [89]

    -protected function rules(array $args = []): array
    +protected function tokenPaginationRules(array $args = []): array
    Suggestion importance[1-10]: 6

    Why: The suggestion improves the method name for better readability and maintainability, which is beneficial for understanding the code's purpose and context.

    6

    @enjinabner enjinabner merged commit 6bf7c49 into master Oct 14, 2024
    6 of 7 checks passed
    @enjinabner enjinabner deleted the feature/pla-1946/validate-after-param branch October 14, 2024 13:02
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Development

    Successfully merging this pull request may close these issues.

    2 participants