./phpunit
Details
Details
Diff Detail
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
Comment Actions
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." | |
Comment Actions
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. | |