Skip to content
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

Add the ability to transfer ownership of a bank account #65

Open
Nexabra opened this issue Dec 12, 2023 · 13 comments · May be fixed by #82
Open

Add the ability to transfer ownership of a bank account #65

Nexabra opened this issue Dec 12, 2023 · 13 comments · May be fixed by #82
Assignees
Labels
feature New features

Comments

@Nexabra
Copy link

Nexabra commented Dec 12, 2023

/bank transfer <ID> <username>

This will change the ownership in the database from the initiator of the command to the player specified. Must check if bank account is owned by that person and send a confirmation message

@zefir-git
Copy link
Member

Why when instead you can just send all your money?

Also /bank transfer is an existing command for sending bank transfers

@zefir-git zefir-git added the question Further information is requested label Dec 24, 2023
@WoofWolfGG
Copy link

Why when instead you can just send all your money?

Also /bank transfer is an existing command for sending bank transfers

On my server we have RP ministries and government organizations. And each ministry has its own account, which is managed by the minister. It would be more convenient to change the owner of the account by command, instead of having to go to the database every time to do it

@zefir-git zefir-git added the feature New features label Dec 28, 2023
@zefir-git
Copy link
Member

Seeing that this is of interest, I have added this feature to the to-do list.

Can you suggest what command we should use? We could always use changeowner or something like that

@WoofWolfGG
Copy link

I think something like changeowner would good. Maybe something like newowner, newholder, changeholder etc. And maybe this feature should only be added with special permission, or something like that :)

@zefir-git zefir-git removed the question Further information is requested label Dec 28, 2023
@zefir-git zefir-git self-assigned this Dec 29, 2023
@zefir-git
Copy link
Member

Should there be confirmation whether you want to accept the account?

Alternatively, to prevent spam I could add a limitation that accounts with a $0 balance and no transaction history cannot be transferred.

@Nissoco23
Copy link

Yes there should definitely be a confirmation

And yes I think the minimum balance for transfer should be $5,000

@zefir-git
Copy link
Member

Yes there should definitely be a confirmation

And yes I think the minimum balance for transfer should be $5,000

I can add a config option for min balance to transfer ownership.

I will make a command like /bank acceptaccount <id> for confirmations. Pending confirmations will be stored in memory or in the db. When you receive a "Would you like to accept this account?" message you will be able to click a "button" that will run the accept command for you.

@zefir-git
Copy link
Member

Yes there should definitely be a confirmation

And yes I think the minimum balance for transfer should be $5,000

Another question:

What if the account has the required balance and a "change owner" request is created, but then, by the time the new owner confirms the request, the balance has changed (and is now below the required minimum)?

I had a few solutions in mind:

  1. Remove requirement for min balance;
  2. When accepting, check conditions again and invalidate with an error if needed (may cause confusion as the new owner had nothing to do with this failure); OR
  3. Freeze account while there is a pending owner change request. Adds lots of new complexity: needs a way to cancel the request (and see requests sent), needs to be correctly unfrozen once the request expires, etc.

I think (from these options at least) that 1. is the best. If you have better ideas let me know. Waiting for response to continue working on this.

@WoofWolfGG
Copy link

I agree that a minimum balance is not necessary to transfer an account to another player (As for me)

@Nissoco23
Copy link

I personally think you should only check the minimum balance when the transfer request is initiated and if it goes below the minimum threshold by the time the person accepts it is just ignored/does not matter

@zefir-git
Copy link
Member

I personally think you should only check the minimum balance when the transfer request is initiated and if it goes below the minimum threshold by the time the person accepts it is just ignored/does not matter

Then what stops one from sending $5K to the account just to changeowner and then immediately getting it back? What if the account needs to be transferred but it just so happens it was at $3543 at the time?

@zefir-git zefir-git linked a pull request Jan 14, 2024 that will close this issue
8 tasks
@zefir-git
Copy link
Member

zefir-git commented Jan 14, 2024

Hi! I have another question.

When accepting an account, should the new owner just have a permission like bank.accept-ownership or should I also enforce the same requirements as account creation (max accounts, per account-type permissions, etc.)?

Otherwise, someone could technically bypass their max accounts limit by having someone else create the account and then transfer it to them.

Note

When a player with the admin permission bank.change.owner.skip-confirmation initiates the account owner change, there will be no confirmation needed and no checks will be performed, allowing admins to transfer accounts instantly.

@WoofWolfGG
Copy link

Hello! Yes, I think it is necessary to make bank.accept-ownership authorization, it will not be unnecessary for sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New features
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants