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-1900] Adds default value to provides deposit #59

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

leonardocustodio
Copy link
Member

@leonardocustodio leonardocustodio commented Jul 17, 2024

PR Type

Bug fix


Description

  • Added a default value of false to the provides_deposit field in the FuelTankCreated event to prevent potential null value issues.

Changes walkthrough 📝

Relevant files
Bug fix
FuelTankCreated.php
Add default value for `provides_deposit` field                     

src/Services/Processor/Substrate/Events/Implementations/FuelTanks/FuelTankCreated.php

  • Added a default value of false to the provides_deposit field.
+1/-1     

💡 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 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No key issues to review

Copy link

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Score
Possible issue
Improve the handling of null values for 'provides_deposit' to avoid unintended coercion of falsy values

Consider using a more explicit check for null when setting the default value for
'provides_deposit'. This ensures that only null values are replaced with false, and
all other falsy values like 0, empty strings, or arrays are not unintentionally
coerced to false.

src/Services/Processor/Substrate/Events/Implementations/FuelTanks/FuelTankCreated.php [54]

-'provides_deposit' => $providesDeposit ?? false,
+'provides_deposit' => isset($providesDeposit) ? $providesDeposit : false,
 
Suggestion importance[1-10]: 9

Why: The suggestion correctly identifies a potential issue with the use of the null coalescing operator (??), which can unintentionally coerce other falsy values to false. The proposed change to use isset ensures that only null values are replaced with false, improving the robustness of the code.

9

@leonardocustodio leonardocustodio merged commit 53672a1 into master Jul 17, 2024
7 checks passed
@leonardocustodio leonardocustodio deleted the PLA-1900 branch July 17, 2024 21:41
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