./phpunit
Details
Diff Detail
- Repository
- rK kolab
- Branch
- dev/signup-invitations
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 34057 Build 13552: arc lint + arc unit
Event Timeline
Looks good in general.
Notes:
- There's no expiry for signup codes, but I suppose that's ok (we have the timestamps, so we can add that later)
- UUID management could be a bit more generalized I think. Perhaps using a trait: https://dev.to/cverster/uuids-in-laravel-7-4kke. Duplicate detection is not necessary IMO with a UUIDv4 algorithm, the collision probability is low enough and a unique constraint can be added on the database level (for the once in a lifetime event of a collision).
- I don't think it's a performance critical database table, so I'm not worried about that aspect too much.
src/app/Http/Controllers/API/V4/Reseller/InvitationsController.php | ||
---|---|---|
156 | Personally I would rather abort if any of the addresses fails. It seems easier to me to provide a valid file than to try after the fact to figure out which ones failed/succeeded. | |
src/app/SignupInvitation.php | ||
13 | It seems this can be removed? | |
28 | This could be shifted by 3 instead, did you leave space on purpose? | |
50 | Can data be removed? | |
57 | Can data be removed? | |
src/database/migrations/2021_03_26_080000_create_signup_invitations_table.php | ||
21 | Is there a reason for not using uuid('id'); ? | |
24 | Remove? | |
32 | Are we using this index? InvitationsController::index does not seem to use status (but created_at) | |
src/resources/lang/en/mail.php | ||
79 | Probably needs to happen before merge? | |
src/resources/vue/Reseller/Dashboard.vue | ||
8 | I think this isn't here on purpose? | |
src/resources/vue/Reseller/Invitations.vue | ||
83 | As a suggestion: "To send multiple invitations at once, provide a CSV (comma separated value) file containing one email address per value, or alternatively a plain-text file with one email address per line." |
Just the final fix, you can then ship it.
src/app/Http/Controllers/API/V4/Reseller/InvitationsController.php | ||
---|---|---|
183 | Same as above, abort on error. |
src/app/Http/Controllers/API/V4/Reseller/InvitationsController.php | ||
---|---|---|
183 | There's no loop here. So, it will abort on error. |