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-1861] Updates and tweaks for dealing with single use codes. #75

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

v16Studios
Copy link
Contributor

@v16Studios v16Studios commented Jun 17, 2024

PR Type

Enhancement, Tests


Description

  • Refactored various parts of the codebase to handle single-use codes using the new BeamService::getSingleUseCodeData method.
  • Enhanced validation rules to support single-use codes.
  • Added a new test case to verify the functionality of pending claims using single-use codes.

Changes walkthrough 📝

Relevant files
Enhancement
9 files
ClaimBeamMutation.php
Refactor single-use code handling in ClaimBeamMutation     

src/GraphQL/Mutations/ClaimBeamMutation.php

  • Replaced decryption logic with BeamService::getSingleUseCodeData for
    single-use codes.
  • +1/-1     
    GetPendingClaimsQuery.php
    Enhance single-use code handling and validation in
    GetPendingClaimsQuery

    src/GraphQL/Queries/GetPendingClaimsQuery.php

  • Added BeamService::getSingleUseCodeData to handle single-use codes.
  • Updated validation rules to use BeamExists.
  • +15/-2   
    BeamClaim.php
    Refactor single-use code handling in BeamClaim model         

    src/Models/Laravel/BeamClaim.php

  • Replaced decryption logic with BeamService::getSingleUseCodeData in
    scopeWithSingleUseCode.
  • +2/-2     
    HasCodeScope.php
    Enhance single-use code handling in HasCodeScope trait     

    src/Models/Laravel/Traits/HasCodeScope.php

  • Added logic to handle single-use codes using
    BeamService::getSingleUseCodeData.
  • +10/-2   
    HasSingleUseCodeScope.php
    Refactor single-use code handling in HasSingleUseCodeScope trait

    src/Models/Laravel/Traits/HasSingleUseCodeScope.php

  • Replaced decryption logic with BeamService::getSingleUseCodeData for
    single-use codes.
  • +3/-2     
    BeamExists.php
    Enhance BeamExists rule to handle single-use codes             

    src/Rules/BeamExists.php

  • Added logic to handle single-use codes using
    BeamService::getSingleUseCodeData.
  • +5/-0     
    CanClaim.php
    Refactor single-use code handling in CanClaim rule             

    src/Rules/CanClaim.php

  • Replaced decryption logic with BeamService::getSingleUseCodeData for
    single-use codes.
  • +1/-1     
    VerifySignedMessage.php
    Refactor single-use code handling in VerifySignedMessage rule

    src/Rules/VerifySignedMessage.php

  • Replaced decryption logic with BeamService::getSingleUseCodeData for
    single-use codes.
  • +1/-1     
    BeamService.php
    Add and utilize getSingleUseCodeData method in BeamService

    src/Services/BeamService.php

  • Added getSingleUseCodeData method to handle single-use code parsing.
  • Updated isSingleUse method to use getSingleUseCodeData.
  • +12/-2   
    Tests
    1 files
    GetPendingClaimsTest.php
    Add test for pending claims with single-use code                 

    tests/Feature/GraphQL/Queries/GetPendingClaimsTest.php

    • Added test for getting pending claims using single-use code.
    +18/-0   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 3
    🧪 Relevant tests Yes
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The refactoring from decrypt to BeamService::getSingleUseCodeData changes how the code is parsed and used. Ensure that the new method correctly handles all edge cases and formats that decrypt handled.
    Data Integrity:
    The changes in how codes are parsed and used could affect data integrity if not handled correctly. Review the changes to ensure that they maintain data integrity, especially in BeamService::getSingleUseCodeData.

    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Handle potential null values to prevent errors

    Consider handling the case where getSingleUseCodeData returns null to avoid potential null
    reference errors when accessing beamCode.

    src/GraphQL/Mutations/ClaimBeamMutation.php [108]

    -$beamCode = BeamService::getSingleUseCodeData($args['code'])?->beamCode;
    +$beamData = BeamService::getSingleUseCodeData($args['code']);
    +$beamCode = $beamData ? $beamData->beamCode : null;
     
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential null reference error, which is a significant issue that can cause runtime exceptions. Handling null values ensures the code is more robust and error-resistant.

    9
    Add error handling for null return values

    Add error handling for the scenario where getSingleUseCodeData might return null, to
    ensure $args['code'] is not set to an invalid value.

    src/GraphQL/Queries/GetPendingClaimsQuery.php [66-67]

    -if ($beamData = BeamService::getSingleUseCodeData($args['code'])) {
    +$beamData = BeamService::getSingleUseCodeData($args['code']);
    +if ($beamData) {
         $args['code'] = $beamData->beamCode;
    +} else {
    +    // Handle error or fallback scenario
     }
     
    Suggestion importance[1-10]: 9

    Why: This suggestion improves the robustness of the code by ensuring that $args['code'] is not set to an invalid value if getSingleUseCodeData returns null. This prevents potential runtime errors and ensures better error handling.

    9
    Check for null to avoid accessing properties on a non-object

    Ensure that the getSingleUseCodeData method does not return null before accessing
    properties to prevent runtime errors.

    src/Models/Laravel/BeamClaim.php [147-149]

     $parsed = BeamService::getSingleUseCodeData($code);
    -return $query->where(['code' => $parsed->claimCode, 'nonce' => $parsed->nonce]);
    +if ($parsed) {
    +    return $query->where(['code' => $parsed->claimCode, 'nonce' => $parsed->nonce]);
    +} else {
    +    // Handle the case where parsed is null
    +}
     
    Suggestion importance[1-10]: 9

    Why: This suggestion addresses a potential runtime error by checking for null values before accessing properties. This is crucial for maintaining the stability and reliability of the code.

    9
    Enhancement
    Improve exception handling and simplify object creation

    Refactor the getSingleUseCodeData method to handle exceptions more gracefully and ensure
    consistent return types.

    src/Services/BeamService.php [313-323]

     try {
         [$claimCode, $beamCode, $nonce] = explode(':', decrypt($code), 3);
    +    return (object)[
    +        'claimCode' => $claimCode,
    +        'beamCode' => $beamCode,
    +        'nonce' => $nonce,
    +    ];
     } catch (Throwable) {
         return null;
     }
    -return json_decode(json_encode([
    -    'claimCode' => $claimCode,
    -    'beamCode' => $beamCode,
    -    'nonce' => $nonce,
    -]));
     
    Suggestion importance[1-10]: 8

    Why: This suggestion enhances the method by improving exception handling and ensuring consistent return types. While it is an enhancement, it significantly improves code readability and maintainability.

    8

    @v16Studios v16Studios merged commit ca60c9f into master Jun 17, 2024
    7 checks passed
    @v16Studios v16Studios deleted the update/pla-1861/update-scope-for-single-use-codes branch June 17, 2024 13:38
    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