-
Notifications
You must be signed in to change notification settings - Fork 83
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
Updated customer addbank #268
Conversation
semgrep integration
Doc api correction
create order params updated
fix: Removed unused test.py file
add pull request template
Create pull_request_template.md
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #268 +/- ##
==========================================
+ Coverage 97.06% 97.12% +0.05%
==========================================
Files 62 62
Lines 2009 2049 +40
==========================================
+ Hits 1950 1990 +40
Misses 59 59 ☔ View full report in Codecov by Sentry. |
documents/customer.md
Outdated
|
||
| Name | Type | Description | | ||
|---------------|-------------|---------------------------------------------| | ||
| customerId* | string | Customer's bank account number | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure about customerId? based on the example above, it seems to be the public customer id, but here in the description it is mentioned as bank account number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be The unique identifier linked to the customer.
i will change the description
documents/customer.md
Outdated
| account_number | integer | The id of the customer to be fetched | | ||
| account_number | string | The name of the beneficiary associated with the bank account. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repeated, check for proper description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed the first one
razorpay/resources/customer.py
Outdated
Dictionary of Customers data | ||
""" | ||
url = "{}/{}/bank_account".format(self.base_url, customer_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use
url = f"{self.base_url}/{customer_id}/bank_account"
seems to be much elegant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same goes for the other code in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, including in unit tests
razorpay/resources/customer.py
Outdated
""" | ||
url = "{}/eligibility/{}".format(self.base_url, eligibility_id) | ||
return self.get_url(url, data, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add new line at the end of the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
razorpay/resources/payment.py
Outdated
@@ -288,4 +288,4 @@ def otpResend(self, payment_id, data={}, **kwargs): | |||
Otp Dict which was created | |||
""" | |||
url = "{}/{}/otp/resend".format(self.base_url, payment_id) | |||
return self.post_url(url, data, **kwargs) | |||
return self.post_url(url, data, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add new line at the end of the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
tests/mocks/bank_account.json
Outdated
"name": "Gaurav Kumar", | ||
"notes": [], | ||
"account_number": "XXXXXXXXXXXXXXX0434" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add new line at the end of the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
tests/mocks/eligibility_check.json
Outdated
} | ||
} | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add new line at the end of the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
tests/test_client_customer.py
Outdated
responses.add(responses.GET, url, status=200, | ||
body=json.dumps(result), match_querystring=True) | ||
self.assertEqual(self.client.customer.fetchEligibility('fake_eligibility_id'), result) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add new line at the end of the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
documents/customer.md
Outdated
| Name | Type | Description | | ||
|---------------|-------------|---------------------------------------------| | ||
| customerId* | string | Unique identifier of the customer. | | ||
| account_number | string | The name of the beneficiary associated with the bank account. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is account_number
name of the beneficiary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made correction in account_number
documents/customer.md
Outdated
| beneficiary_address1 | string | The id of the customer to be fetched | | ||
| beneficiary_email | string | Email address of the beneficiary. | | ||
| beneficiary_mobile | integer | Mobile number of the beneficiary. | | ||
| beneficiary_city | string | The name of the city of the beneficiary. | | ||
| beneficiary_state | string | The state of the beneficiary. | | ||
| beneficiary_pin | interger | The pin code of the beneficiary's address. | | ||
| ifsc_code | string | The IFSC code of the bank branch associated with the account. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check the description of each of the parameters and make sure they are proper
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go through the entire file and make sure the information is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made all doc correction in new added endpoint
| customerId* | string | Unique identifier of the customer. | | ||
| account_number | string | Customer's bank account number. | | ||
| beneficiary_name | string | The name of the beneficiary associated with the bank account. | | ||
| beneficiary_address1 | string | The virtual payment address. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? should it be VPA, VPA is used for upi right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documents/customer.md
Outdated
| inquiry | string | List of methods or instruments on which eligibility check is required. | | ||
| amount* | string | The amount for which the order was created, in currency subunits. | | ||
| currency* | string | A three-letter ISO code for the currency in which you want to accept the payment. | | ||
| customer* | object | All keys listed [here](https://razorpay.com/docs/payments/payment-gateway/affordability/eligibility-check/#eligibility-check-api) | | ||
| instruments | object | All keys listed [here](https://razorpay.com/docs/payments/payment-gateway/affordability/eligibility-check/#eligibility-check-api) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description does not seem right. Please add proper description. Kindly go through this file and make sure all text that is being written are proper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reference doc
Add Bank Account
Delete Bank Account
Fetch Eligibility- Affordability
Eligibility Check