You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes
deluser-like plugins possible.