Update @planet-sdk/common and fix typos in currency functions #475
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request primarily involve updating the version of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (6)
src/Donations/LeftPanel/TransactionSummary.tsx (1)
3-3: Consider using relative imports for consistencyThe import path uses an absolute path with
src/prefix, while other files likeDonationAmount.tsxuse relative paths. Consider using relative imports for consistency across the codebase.-import getFormattedCurrency from "src/Utils/getFormattedCurrency"; +import getFormattedCurrency from "../../Utils/getFormattedCurrency";src/Donations/Micros/DonationTypes/TreeDonation.tsx (1)
Line range hint
168-172: Fix empty currency parameter in getFormattedCurrencyThe
getFormattedCurrencyfunction is called with an empty string as the currency parameter. This could lead to incorrect currency formatting.-getFormattedCurrency( - i18n.language, - "", - Number(paymentSetup.unitCost) -) +getFormattedCurrency( + i18n.language, + currency, + Number(paymentSetup.unitCost) +)The
currencyvariable from the context should be used instead of an empty string to ensure proper currency formatting.src/Donations/PaymentMethods/PaymentMethodTabs.tsx (1)
Line range hint
107-129: Consider refactoring repetitive currency formatting logicThe switch statement contains duplicate currency formatting logic across multiple cases. Consider extracting this common logic to reduce code duplication.
+ const formatAmount = () => getFormattedCurrency( + i18n.language, + currency, + paymentSetup.unitCost * quantity + ); switch (projectDetails && projectDetails.purpose) { case "trees": paymentLabel = t("treesInCountry", { treeCount: quantity, }); break; case "funds": paymentLabel = t("fundingPaymentLabel", { - amount: getFormattedCurrency( - i18n.language, - currency, - paymentSetup.unitCost * quantity - ), + amount: formatAmount(), }); break; // Apply similar changes to other casessrc/Donations/Micros/DonationTypes/FundingDonations.tsx (1)
Line range hint
198-219: Consider enhancing input validation for custom amountsThe current input validation only handles numeric input and length constraints. Consider adding:
- Maximum amount validation
- Decimal point support for precise amounts
- Feedback to users when validation fails
onInput={(e: ChangeEvent<HTMLInputElement>) => { - // replaces any character other than number to blank - e.target.value = e.target.value.replace(/[^0-9]/g, ""); + // Allow numbers and one decimal point + e.target.value = e.target.value.replace(/[^0-9.]/g, "") + .replace(/(\..*)\./g, "$1"); // Allow only one decimal point + + // Validate maximum amount + const amount = parseFloat(e.target.value); + if (amount > 999999) { + e.target.value = "999999"; + } if (e.target.value.toString().length >= 12) { e.target.value = e.target.value.toString().slice(0, 12); } }}src/Donations/Micros/DonationTypes/BouquetDonations.tsx (1)
Line range hint
128-134: Track the rounded costs requirementThe TODO comment indicates a future requirement for showing rounded costs. This should be tracked to ensure it's not forgotten.
Would you like me to create a GitHub issue to track this requirement? I can help outline the necessary structural changes and implementation details.
src/Donations/Components/PaymentsForm.tsx (1)
Line range hint
133-133: Remove console.log and improve error handling.The TODO comment indicates that better error handling is needed. Consider implementing proper error handling instead of using console.log.
- console.log("Missing payment options"); //TODOO - better error handling + setPaymentError(t("errors.missingPaymentOptions")); + return;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
package.json(1 hunks)src/Donations/Components/ContactsForm.tsx(2 hunks)src/Donations/Components/DonationsForm.tsx(5 hunks)src/Donations/Components/PaymentsForm.tsx(2 hunks)src/Donations/LeftPanel/TransactionSummary.tsx(2 hunks)src/Donations/Micros/DonationAmount.tsx(2 hunks)src/Donations/Micros/DonationTypes/BouquetDonations.tsx(4 hunks)src/Donations/Micros/DonationTypes/FundingDonations.tsx(3 hunks)src/Donations/Micros/DonationTypes/TreeDonation.tsx(2 hunks)src/Donations/Micros/PaymentStatus/ImageComponent.tsx(2 hunks)src/Donations/Micros/PaymentStatus/ThankyouMessage.tsx(2 hunks)src/Donations/Micros/PaymentStatus/TransferDetails.tsx(2 hunks)src/Donations/Micros/PlanetCashSelector.tsx(4 hunks)src/Donations/PaymentMethods/PaymentMethodTabs.tsx(4 hunks)src/Utils/getFormattedCurrency.ts(2 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/Utils/getFormattedCurrency.ts
- src/Donations/Components/ContactsForm.tsx
- src/Donations/Micros/PlanetCashSelector.tsx
- src/Donations/Micros/PaymentStatus/ThankyouMessage.tsx
🔇 Additional comments (14)
src/Donations/Micros/DonationAmount.tsx (1)
3-3: LGTM: Consistent function renaming
The renaming from getFormatedCurrency to getFormattedCurrency is applied consistently in both the import statement and function call, maintaining the same functionality.
Also applies to: 18-21
src/Donations/LeftPanel/TransactionSummary.tsx (1)
3-3: LGTM: Consistent function renaming
The renaming from getFormatedCurrency to getFormattedCurrency is applied consistently in both the import statement and function call.
Also applies to: 71-75
package.json (1)
29-29: Verify @planet-sdk/common version update implications
The package version update from ^0.1.42 to ^0.1.47 should be verified for:
- Breaking changes in the changelog
- Impact on the currency formatting functions being modified
✅ Verification successful
Based on the analysis of the codebase, I can now generate the final response:
@planet-sdk/common version update appears safe
The version update from ^0.1.42 to ^0.1.47 appears safe as:
- The package is primarily used for type definitions and error handling utilities
- No breaking changes in dependencies (only requires
@types/geojson) - The codebase uses well-defined imports and interfaces from the package
- Currency formatting functions being modified are local to this codebase (
src/Utils/getFormattedCurrency.ts) and don't depend on @planet-sdk/common
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in @planet-sdk/common between versions
# Fetch package info and changelog
echo "Fetching package information..."
curl -s "https://registry.npmjs.org/@planet-sdk/common" | jq '.versions["0.1.47"].dependencies'
# Check git commits between versions using GitHub API
echo "Checking commits between versions..."
gh api graphql -f query='
{
repository(owner: "Plant-for-the-Planet-org", name: "planet-sdk") {
refs(refPrefix: "refs/tags/", first: 10, orderBy: {field: TAG_COMMIT_DATE, direction: DESC}) {
nodes {
name
target {
... on Tag {
message
tagger { date }
}
}
}
}
}
}'
Length of output: 953
Script:
#!/bin/bash
# Let's check the actual usage of @planet-sdk/common in the codebase
echo "Searching for @planet-sdk/common usage..."
rg "@planet-sdk/common" -A 3
# Check if there are any currency-related imports or usage
echo "Checking currency-related imports..."
rg "currency" --type js --type ts -A 3
# Look for any package-lock.json to see the exact versions
echo "Checking package-lock.json..."
fd "package-lock.json" --exec cat {} | jq '.dependencies["@planet-sdk/common"]'
Length of output: 65436
src/Donations/Micros/PaymentStatus/ImageComponent.tsx (1)
27-27: LGTM: Currency formatting function usage is correct
The renamed getFormattedCurrency function is correctly used with proper parameters.
src/Donations/Micros/PaymentStatus/TransferDetails.tsx (1)
Line range hint 54-58: LGTM: Currency formatting implementation is correct
The renamed getFormattedCurrency function is properly imported and used with correct parameters for formatting the recurrent transfer amount.
src/Donations/Micros/DonationTypes/TreeDonation.tsx (1)
10-10: Verify consistent renaming across the codebase
Let's ensure the function renaming is consistent across all files.
✅ Verification successful
Function renaming is consistent across the codebase
The verification shows that:
- No instances of the old function name "getFormatedCurrency" were found
- The new function name "getFormattedCurrency" is consistently used across 13 files
- No typos or inconsistencies were found in the function naming
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of the old function name
# and verify the new function name is used consistently
echo "Checking for old function name (getFormatedCurrency)..."
rg -l "getFormatedCurrency"
echo -e "\nVerifying usage of new function name (getFormattedCurrency)..."
rg -l "getFormattedCurrency"
echo -e "\nChecking for potential typos in the function name..."
rg -i "get.*formatted.*currency"
Length of output: 5627
src/Donations/PaymentMethods/PaymentMethodTabs.tsx (1)
12-12: LGTM!
The import statement correctly uses the renamed currency formatting function.
src/Donations/Micros/DonationTypes/FundingDonations.tsx (1)
10-12: LGTM!
The import statements correctly use the renamed currency formatting functions.
src/Donations/Micros/DonationTypes/BouquetDonations.tsx (2)
11-13: LGTM!
The import statements correctly use the renamed currency formatting functions.
Line range hint 163-171: Consider handling edge cases in currency formatting
The currency formatting logic should handle edge cases such as:
- Zero or negative amounts
- Very large numbers
- Different currency precision requirements
src/Donations/Components/DonationsForm.tsx (2)
260-263: LGTM! Currency formatting is consistent across all payment labels.
The currency formatting implementation is consistent across funds, planet-cash, and bouquet payment labels.
Also applies to: 269-272, 279-282
469-469: LGTM! Minimum amount currency formatting is consistent.
The currency formatting for minimum amount display matches the pattern used in payment labels.
src/Donations/Components/PaymentsForm.tsx (2)
10-10: LGTM! Import statement is correctly updated.
The import statement for getFormattedCurrency is properly updated.
422-425: LGTM! Currency formatting in CardPayments is consistent.
The currency formatting implementation in CardPayments matches the pattern used throughout the application.
mohitb35
force-pushed
the
hotfix/typos-and-planet-sdk-update
branch
from
9e8b60b to
71554b5
Compare
- removes unnecessary `true &&...` expression - refactors onClick handler for currency selector in TreeDonations
|
Could you address the code factor issue ? |
I have addressed the ones that could be addressed. The pending issue relates to complex code in the |
Summary by CodeRabbit
New Features
Bug Fixes
getFormatedCurrencytogetFormattedCurrencyin multiple components, ensuring consistent functionality.Chores
@planet-sdk/commonpackage to enhance overall application performance and stability.