feat: add type, first 6, last 4 fields to the wallet cc form#1
feat: add type, first 6, last 4 fields to the wallet cc form#1chrismshea wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe update refines the payment processing flow by improving code readability and extending data handling for credit card details. In the backend, both the card save controller and wallet payment builder now extract and pass additional credit card information. In the frontend, the JavaScript component managing vault cards is enhanced to observe and submit these new properties. No public entity declarations were modified. Changes
Sequence Diagram(s)sequenceDiagram
participant JS as JS Payment Component
participant CTRL as Save Controller
participant WPB as WalletPaymentBuilder
participant PG as Payment Gateway
JS->>CTRL: submitPayment() with credit card details
CTRL->>CTRL: execute() & validate form key
CTRL->>CTRL: prepareVerifyCommandSubject() extracts credit card data
CTRL->>WPB: Pass enhanced payment profile data
WPB->>WPB: build() returns array with new credit card fields
WPB->>PG: Send detailed payment information
Poem
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 review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
view/frontend/web/js/view/vault/card.js (1)
74-78: Ensure consistency in data handling.The
submitPaymentmethod only includeslast_digitsandfirst_digitsbut notcreditcard_type, unlikegetPaymentData. Consider addingcreditcard_typefor consistency.submitPayment: function () { var cartData = $(this.formSelector).serializeJSON(); cartData.creditcard_last_digits = this.creditCardLastDigits(); cartData.creditcard_first_digits = this.creditCardFirstDigits(); + cartData.creditcard_type = this.selectedCardType();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Controller/Cards/Save.php(2 hunks)Gateway/Request/WalletPaymentBuilder.php(1 hunks)view/frontend/web/js/view/vault/card.js(2 hunks)
🔇 Additional comments (3)
view/frontend/web/js/view/vault/card.js (1)
22-26: LGTM! Good use of observables for new card properties.The new properties are properly initialized and made observable.
Controller/Cards/Save.php (2)
93-100: LGTM! Improved readability of form validation conditions.The multi-line formatting of validation conditions improves code readability.
151-153:Details
❓ Verification inconclusive
Verify validation coverage for new credit card fields.
The code relies on
vaultFormValidator->validate()for field validation. Please ensure that the validator properly handles these new sensitive fields.
🏁 Script executed:
#!/bin/bash # Check if vaultFormValidator includes validation for new fields ast-grep --pattern 'class Validator { $$$ validate($profileData) { $$$ PaymentProfileInterface::CREDITCARD_TYPE $$$ } }' # Also check for any existing validation patterns rg -A 5 "CREDITCARD_(TYPE|FIRST_DIGITS|LAST_DIGITS)" --type phpLength of output: 3572
Action Required: Validate New Credit Card Fields in the Validator
The new fields (
creditcard_type,creditcard_first_digits, andcreditcard_last_digits) are now being passed through the vault form validation, but our search didn’t reveal any explicit validation rules within the validator for these fields. Please double-check that thevaultFormValidator->validate()method adequately covers these sensitive fields. If not, consider adding explicit validation logic to handle them securely.
| PaymentProfileInterface::CREDITCARD_TYPE => $buildSubject['creditcard_type'] ?? null, | ||
| PaymentProfileInterface::CREDITCARD_LAST_DIGITS => $buildSubject['creditcard_last_digits'] ?? null, | ||
| PaymentProfileInterface::CREDITCARD_FIRST_DIGITS => $buildSubject['creditcard_first_digits'] ?? null, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for sensitive credit card fields.
While using the null coalescing operator is good for safety, consider adding validation for these sensitive credit card fields similar to how other fields are validated. This ensures data integrity and security.
Consider adding validation methods similar to existing ones:
public function build(array $buildSubject)
{
return [
PaymentProfileInterface::CREDITCARD_MONTH => $this->getCreditCardMonth($buildSubject),
PaymentProfileInterface::CREDITCARD_YEAR => $this->getCreditCardYear($buildSubject),
- PaymentProfileInterface::CREDITCARD_TYPE => $buildSubject['creditcard_type'] ?? null,
- PaymentProfileInterface::CREDITCARD_LAST_DIGITS => $buildSubject['creditcard_last_digits'] ?? null,
- PaymentProfileInterface::CREDITCARD_FIRST_DIGITS => $buildSubject['creditcard_first_digits'] ?? null,
+ PaymentProfileInterface::CREDITCARD_TYPE => $this->getCreditCardType($buildSubject),
+ PaymentProfileInterface::CREDITCARD_LAST_DIGITS => $this->getCreditCardLastDigits($buildSubject),
+ PaymentProfileInterface::CREDITCARD_FIRST_DIGITS => $this->getCreditCardFirstDigits($buildSubject),
PaymentProfileInterface::BILLING_ADDRESS => $this->getBillingAddress($buildSubject),Add these validation methods:
private function getCreditCardType(array $buildSubject)
{
if (empty($buildSubject['creditcard_type'])) {
throw new \InvalidArgumentException('Credit card type is not passed.');
}
return $buildSubject['creditcard_type'];
}
private function getCreditCardLastDigits(array $buildSubject)
{
if (empty($buildSubject['creditcard_last_digits'])) {
throw new \InvalidArgumentException('Credit card last digits are not passed.');
}
return $buildSubject['creditcard_last_digits'];
}
private function getCreditCardFirstDigits(array $buildSubject)
{
if (empty($buildSubject['creditcard_first_digits'])) {
throw new \InvalidArgumentException('Credit card first digits are not passed.');
}
return $buildSubject['creditcard_first_digits'];
}| 'month': this.creditCardExpMonth(), | ||
| 'creditcard_first_digits': this.creditCardFirstDigits(), | ||
| 'creditcard_last_digits': this.creditCardLastDigits(), | ||
| 'creditcard_type': this.selectedCardType(), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for credit card data.
Consider adding validation to ensure the credit card data follows expected patterns:
- First 6 digits should be exactly 6 digits
- Last 4 digits should be exactly 4 digits
- Card type should be from a valid set of card types
Add validation before returning the data:
getPaymentData: function () {
+ if (this.creditCardFirstDigits() && !/^\d{6}$/.test(this.creditCardFirstDigits())) {
+ throw new Error('Invalid credit card first digits');
+ }
+ if (this.creditCardLastDigits() && !/^\d{4}$/.test(this.creditCardLastDigits())) {
+ throw new Error('Invalid credit card last digits');
+ }
return {
'first_name': $("#first_name").val(),
// ... other fields ...
'creditcard_first_digits': this.creditCardFirstDigits(),
'creditcard_last_digits': this.creditCardLastDigits(),
'creditcard_type': this.selectedCardType(),
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'month': this.creditCardExpMonth(), | |
| 'creditcard_first_digits': this.creditCardFirstDigits(), | |
| 'creditcard_last_digits': this.creditCardLastDigits(), | |
| 'creditcard_type': this.selectedCardType(), | |
| getPaymentData: function () { | |
| if (this.creditCardFirstDigits() && !/^\d{6}$/.test(this.creditCardFirstDigits())) { | |
| throw new Error('Invalid credit card first digits'); | |
| } | |
| if (this.creditCardLastDigits() && !/^\d{4}$/.test(this.creditCardLastDigits())) { | |
| throw new Error('Invalid credit card last digits'); | |
| } | |
| return { | |
| 'month': this.creditCardExpMonth(), | |
| 'creditcard_first_digits': this.creditCardFirstDigits(), | |
| 'creditcard_last_digits': this.creditCardLastDigits(), | |
| 'creditcard_type': this.selectedCardType(), | |
| }; | |
| } |
1 participant
Building on the 1.5.17 release to support SP Vault in the customer wallet card form.
Summary by CodeRabbit