-
Notifications
You must be signed in to change notification settings - Fork 78
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
Show bank a/c details #1913
base: show-bank-account-details-on-invoices
Are you sure you want to change the base?
Show bank a/c details #1913
Conversation
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.
@Shruti-Apte Please rebase from backend branch, fix review comments and test the changes. Also add a loom video with bank account details and without bank account details.
app/javascript/src/components/ClientInvoices/Details/InvoiceDetails.tsx
Outdated
Show resolved
Hide resolved
…b.com/saeloun/miru-web into show_bank_a/c_details Merge with backend branch
…b.com/saeloun/miru-web into show_bank_a/c_details pulled backend changes
.min(12, "Account number must have 12 characters"), | ||
accountType: Yup.string().required("Account type cannot be blank"), | ||
}); | ||
|
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.
@Shruti-Apte Please confirm the validations for routing number and account number with @sonamvg
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
Closes #1890 (UI)
Preview:
https://www.loom.com/share/0e1539e1a1d644b99342e187fac945b6?sid=67cf9645-833d-4ad3-8e19-19a6f455b4b9
Backend changes from PR are also included