Page MenuHomekolab.org

Wallet balance checks/notifications/actions
ClosedPublic

Authored by machniak on Sep 16 2020, 3:12 PM.

Details

Reviewers
vanmeeuwen
Group Reviewers
Restricted Project
Commits
rKc7aa3fdf4590: Wallet balance checks/notifications/actions
Summary

Work in progress

Diff Detail

Repository
rK kolab
Branch
dev/negative-balance
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 30751
Build 11422: arc lint + arc unit

Event Timeline

machniak requested review of this revision.Sep 16 2020, 3:12 PM
machniak created this revision.
vanmeeuwen requested changes to this revision.Sep 16 2020, 4:28 PM
vanmeeuwen added a subscriber: vanmeeuwen.
vanmeeuwen added inline comments.
src/app/Console/Commands/WalletCharge.php
43

Should we also say, something like "where number of entitlements > 0"?

Because the thing is, everyone gets a wallet, but at least currently, most users have no entitlements charged to their wallets.

src/app/Jobs/WalletCheck.php
58

For consistency, should we use balance_negative_since?

73

"comming" -> "upcoming"

This revision now requires changes to proceed.Sep 16 2020, 4:28 PM
machniak planned changes to this revision.Sep 16 2020, 6:39 PM
machniak added inline comments.
src/app/Console/Commands/WalletCharge.php
43

We could, but it would not give us much improvement. I'd not do this.

And I wouldn't be so sure that it's most users.

src/app/Jobs/WalletCheck.php
58

Sure.

175

@vanmeeuwen what do you think about storing entries like these in transactions table? I mean we'd have "for free" a wallet history including sent mail notifications. Of course, it would appear in UI for admins only.

vanmeeuwen added inline comments.Sep 17 2020, 8:42 AM
src/app/Console/Commands/WalletCharge.php
43

For the sake of memory consumption then, does it make sense to, instead of ->get(), do an ->each() over a closure?

src/app/Jobs/WalletCheck.php
175

I would suppose this could get in as a future development, such that it would more generically be a log of "what happened". In this case, we can use system logs and possibly even nginx imap/smtp proxy requests, as well as actions such as these and the actions of users, in a more complete timeline.

machniak updated this revision to Diff 4147.Sep 17 2020, 2:12 PM
  • Small fixes
  • Add job:walletcheck command
  • Add templates for negative balance emails
  • Negative balance mail improvements
  • Code improvements
  • Add tests for negative balance mails
  • Wallet check tests
machniak updated this revision to Diff 4153.Sep 17 2020, 2:58 PM
  • Add more tests
machniak updated this revision to Diff 4159.Sep 17 2020, 3:41 PM
  • Fix automatic unsuspending, add test for this case
machniak updated this revision to Diff 4165.Sep 17 2020, 4:04 PM
  • Fix test cleanup
machniak updated this revision to Diff 4171.Sep 17 2020, 7:43 PM
  • Add wallet:charge tests and possibility to run it for a specified wallet only
machniak updated this revision to Diff 4177.Sep 17 2020, 8:11 PM
  • Fix migration case so first run of WalletCheck will sent notifications for negative-balance wallets
vanmeeuwen accepted this revision.Sep 18 2020, 10:23 AM
This revision is now accepted and ready to land.Sep 18 2020, 10:23 AM
This revision was automatically updated to reflect the committed changes.