Page MenuHomekolab.org

Wallet balance checks/notifications/actions
ClosedPublic

Authored by machniak on Wed, Sep 16, 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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

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

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.Wed, Sep 16, 4:28 PM
machniak planned changes to this revision.Wed, Sep 16, 6:39 PM
machniak added inline comments.
src/app/Console/Commands/WalletCharge.php
44

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.Thu, Sep 17, 8:42 AM
src/app/Console/Commands/WalletCharge.php
44

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.Thu, Sep 17, 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.Thu, Sep 17, 2:58 PM
  • Add more tests
machniak updated this revision to Diff 4159.Thu, Sep 17, 3:41 PM
  • Fix automatic unsuspending, add test for this case
machniak updated this revision to Diff 4165.Thu, Sep 17, 4:04 PM
  • Fix test cleanup
machniak updated this revision to Diff 4171.Thu, Sep 17, 7:43 PM
  • Add wallet:charge tests and possibility to run it for a specified wallet only
machniak updated this revision to Diff 4177.Thu, Sep 17, 8:11 PM
  • Fix migration case so first run of WalletCheck will sent notifications for negative-balance wallets
vanmeeuwen accepted this revision.Fri, Sep 18, 10:23 AM
This revision is now accepted and ready to land.Fri, Sep 18, 10:23 AM
This revision was automatically updated to reflect the committed changes.