Skip to content
This repository was archived by the owner on Aug 28, 2025. It is now read-only.

Conversation

polyfloyd
Copy link
Contributor

This makes deluser-like plugins possible.

@Juerd
Copy link
Contributor

Juerd commented Jun 15, 2025

Thanks for your PR.

As discussed on IRC, I won't be merging it. Specifically, I think the plugin is not a good idea, because users are likely to expect a deluser command to delete a user account with everything associated with it. However, since the account name is the primary key used everywhere, and because a user's transaction log is a literal grep of the log file, deleting a user account from the accounts file and then using adduser again combined, is almost a noop. All it does is reset the datetime metadata.

There's a usecase for deleting accounts that have expired - those are unlikely to be used again, but I wouldn't expose the functionality to end users for that.

If you do choose to continue to use the deluser plugin, I'd strongly advise against the current implementation which uses RevBank::Accounts::update directly. Like the documentation for RevBank::Accounts describes, a transaction should be used instead, because otherwise there's a discrepancy between the ledger and the account balances.

I will cherry-pick your first commit, though, as it makes it easier to inactivate an account (that has expired) with the proper global lockfile.

@Juerd Juerd closed this Jun 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants