Page MenuHomekolab.org

Cherry-picked contents from hkccp-import
ClosedPublic

Authored by vanmeeuwen on May 27 2020, 9:50 AM.

Details

Reviewers
machniak
Group Reviewers
Restricted Project
Commits
rK534d00000831: Cherry-picked contents from hkccp-import
Summary
  • Add commands pulled from hkccp import
  • Ensure 0 <= discount <= 100
  • Ensure a domain already assigned is not assigned again, and lower-case domain names
Test Plan

No particular plan

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

vanmeeuwen requested review of this revision.May 27 2020, 9:50 AM
vanmeeuwen created this revision.
machniak requested changes to this revision.May 28 2020, 10:28 AM
machniak added a subscriber: machniak.
machniak added inline comments.
src/app/Console/Commands/WalletBalances.php
55

How about "<id>: <balance> (<email>: <url>)"?

58

The path will be '/user/<id>'. I think we need \App\Utils::serviceAdminUrl() (as we have serviceUrl() already) or add 2nd argument to serviceUrl().

src/app/Console/Commands/WalletGetBalance.php
40–46

\App\Wallet::find($this->argument('wallet'));

src/app/Console/Commands/WalletSetBalance.php
14

Should this command be available in production? Maybe not, then move it to app/Console/Development/DomainList.php

40–47

\App\Wallet::find($this->argument('wallet'));

src/app/Discount.php
50

We already have tests that expected an exception to be thrown. I think an exception would be better.

3) Tests\Feature\DiscountTest::testDiscountValueLessThanZero
Failed asserting that exception of type "Exception" is thrown.

4) Tests\Feature\DiscountTest::testDiscountValueMoreThanHundred
Failed asserting that exception of type "Exception" is thrown.
src/app/Domain.php
65

We need a test for this change. I also propose to throw an exception here.

73

I propose to use an exception here. Would be good to test this too.

This revision now requires changes to proceed.May 28 2020, 10:28 AM
vanmeeuwen updated this revision to Diff 3382.May 28 2020, 11:22 AM
  • Simplify getting to the actual wallet
  • Simplify getting to the wallet
  • Fix test to meet expectations
vanmeeuwen added inline comments.May 28 2020, 11:23 AM
src/app/Console/Commands/WalletBalances.php
58

This is, for now, for our troubleshooting in the migration/transition though.

What I want here is that the credit_debit_remaining and other such from the current HKCCP quickly refers to something I can click so as to get to the current HKCCP account page.

src/app/Discount.php
50

There's values in the HKCCP that go out of bounds, an exception would either need to be addressed there, as well as in every future interface, or just here.

I'll fix the test though.

machniak accepted this revision.Jun 5 2020, 9:01 AM

My comments to src/app/Domain.php were not addressed, but it's not critical.

This revision is now accepted and ready to land.Jun 5 2020, 9:01 AM
This revision was automatically updated to reflect the committed changes.