captableinc / captable

#1 Open-Source Captable, an alternative to Carta, Pully, Angelist and others.
https://captable.inc
GNU Affero General Public License v3.0
430 stars 73 forks source link

Add bank accounts modal and table #452

Open dahal opened 1 month ago

dahal commented 1 month ago

We currently have placeholder modal for bank accounts, we need to complete this feature. Completing this feature means:-

image

You can get to this page by going to Settings > Bank accounts after you login

Raju-kadel-27 commented 1 month ago

Just to inform, @Alfredoeb9 is working on this issue.

rushikeshg25 commented 1 month ago

@Raju-kadel-27 I can help in with this. Please assign me this if there is inactivity for a long time. Sry for the ping

dahal commented 1 month ago

@Alfredoeb9 are you picking this up or can @rushikeshg25 help? Please confirm.

Alfredoeb9 commented 1 month ago

@dahal I am picking this up just waiting for my other PR to merge before I continue on this task.

Alfredoeb9 commented 1 month ago

@dahal on the new bank modal screen, do I include the respected BankAccount Table schema as inputs? (beneficiaryName, beneficiaryAddress, bankName, bankAddress, companyId etc.)

dahal commented 1 month ago

Yes please. Company id is only to establish association.

Alfredoeb9 commented 1 month ago

Some other edge cases for the routing numbers and account number will there be a maxLength limit? When doing a search looks like Routing numbers are always 9 digits long. Account numbers may be up to 17 digits long.

dahal commented 1 month ago

Some other edge cases for the routing numbers and account number will there be a maxLength limit? When doing a search looks like Routing numbers are always 9 digits long. Account numbers may be up to 17 digits long.

Is this universal or only specific to US accounts, if its universal we can have that validation. @Alfredoeb9

Alfredoeb9 commented 1 month ago

Some other edge cases for the routing numbers and account number will there be a maxLength limit? When doing a search looks like Routing numbers are always 9 digits long. Account numbers may be up to 17 digits long.

Is this universal or only specific to US accounts, if its universal we can have that validation. @Alfredoeb9

Let dig deeper on that, it didnt specify the country

Edit: After some research looks like every country has different max digits. ex. In Germany and Austria, Germany has 8 digits, Austria has 5. In Canada max digits are 8. In India 11-character code with the first four characters representing the bank name, and the last six characters representing the branch.

With this I won't be putting a maxLength on the fields

Alfredoeb9 commented 1 month ago

@dahal Before I continue to connecting to backend, is this UI approved?

I added in a confirm routing number as that seems to be standard across other 'Add Bank Account' services.

image

dahal commented 1 month ago

@dahal Before I continue to connecting to backend, is this UI approved?

I added in a confirm routing number as that seems to be standard across other 'Add Bank Account' services.

image

Looks good, we dont need the company name though. @Alfredoeb9

Alfredoeb9 commented 1 month ago

@dahal are users able to create infinite amount of bank accounts, with the exception that only one can be true for primary and all others as false?

With the current schema only two accounts can be created, one for primary and one for non-primary, I can't create any other primary=false bank accounts.

I don't think prisma @@unique allows for one true and all others as false.

Solution: On the server I can run a check to see if any primary column is true and if there are then return back an error.

dahal commented 1 month ago

@dahal are users able to create infinite amount of bank accounts, with the exception that only one can be true for primary and all others as false?

With the current schema only two accounts can be created, one for primary and one for non-primary, I can't create any other primary=false bank accounts.

I don't think prisma @@unique allows for one true and all others as false.

Solution: On the server I can run a check to see if any primary column is true and if there are then return back an error.

I think one primary and secondary is fine for now with the proper validation in place. When editing the existing account, and if user decide to toggle primary to secondary and vice versa, please update other record to toggle as accordingly. Meaning

Show a ConfirmDialog with the message Please note that other primary/secondary account will be set as secondary/primary, are you sure you want to proceed? @Alfredoeb9, please order primary/secondary and secondary/primary as according to the action you are taking.

Alfredoeb9 commented 1 month ago

@dahal I am not able to auto set primary -> secondary and secondary -> primary getting Error: Unique constraint failed on the fields: (companyId, primary)

Coming from BankAccount schema: @@unique([companyId, primary], name: "unique_primary_account")

How should we proceed? I could remove the unique constraints and implement the auto setting from primary -> seconary and vise-versa but then users are going to able to create infinite about of accounts unless custom code is placed on backend to check if any other accouts are primary and return an error in that case.

dahal commented 1 month ago

@dahal I am not able to auto set primary -> secondary and secondary -> primary getting Error: Unique constraint failed on the fields: (companyId, primary)

Coming from BankAccount schema: @@unique([companyId, primary], name: "unique_primary_account")

How should we proceed? I could remove the unique constraints and implement the auto setting from primary -> seconary and vise-versa but then users are going to able to create infinite about of accounts unless custom code is placed on backend to check if any other accouts are primary and return an error in that case.

@Alfredoeb9 we probably can run a raw sql query for this, something like

BEGIN;

-- Step 1: Select the current primary account and set it to false
UPDATE BankAccount
SET primary = FALSE
WHERE companyId = :companyId
AND primary = TRUE;

-- Step 2: Select the current non-primary account and set it to true
UPDATE BankAccount
SET primary = TRUE
WHERE companyId = :companyId
AND primary = FALSE
LIMIT 1;

COMMIT;
Alfredoeb9 commented 2 weeks ago

can you reassign please @dahal

Alfredoeb9 commented 2 weeks ago

@dahal I tried running a raw sql string using queryRaw and executeRaw as provided in the docs but no avail. I recieve the same error message as above Unique constraint failed on the fields: (companyId, primary)

I also tried putting query in a $transaction but did not work.

I also tried setting the primary value to null then switching to respected value but would get a different error as null cannot be assigned to boolean.

This feature is basically finished just need to find a solution for this last automatic primary switch but I'm not sure it is possible due to the @@unique constriant on a boolean column.

dahal commented 2 weeks ago

@dahal I tried running a raw sql string using queryRaw and executeRaw as provided in the docs but no avail. I recieve the same error message as above Unique constraint failed on the fields: (companyId, primary)

I also tried putting query in a $transaction but did not work.

I also tried setting the primary value to null then switching to respected value but would get a different error as null cannot be assigned to boolean.

This feature is basically finished just need to find a solution for this last automatic primary switch but I'm not sure it is possible due to the @@unique constriant on a boolean column.

Lets remove the db level validation in that case. @Alfredoeb9