Bulk Api user actions #57
Conversation
- Bulk Create Users - Bulk Update Users - Bulk Delete Users - Bulk Restore deleted Users
There was a problem hiding this comment.
@zashton nice, looks good. asked a few questions and left a few comments.
| 'users.*.address' => ['required', 'string', 'size:42', 'distinct', | ||
| // validate id is unique within circle | ||
| Rule::unique('users')->where(function ($query) use ($circle_id) { | ||
| return $query->where('circle_id', $circle_id)->whereNull('deleted_at'); |
There was a problem hiding this comment.
Does Laravel have support for default scopes or some way that we can more easily verify " deleted_at is NULL" all the time?
There was a problem hiding this comment.
added withoutThrashed
| Rule::unique('users')->where(function ($query) use ($circle_id) { | ||
| return $query->where('circle_id', $circle_id)->whereNull('deleted_at'); | ||
| })], | ||
| 'users.*.name' => ['required', 'string', 'max:255'], |
There was a problem hiding this comment.
Not sure if easy to do, but ideally I'd think all these validation rules are defined in exactly one spot not across multiple places (https://github.com/coordinape/coordinape-backend/blob/main/app/Http/Requests/UserRequest.php#L40) but may not be easy to do especially as this is wrapped within the "users" context here vs not in the UserRequest file.
There was a problem hiding this comment.
idk maybe i can make a constant for every rule. but might result in a lot of constants created
| Rule::unique('users')->where(function ($query) use ($circle_id, $user_id) { | ||
| return $query->where('circle_id', $circle_id)->whereNotIn('id', $user_id)->whereNull('deleted_at'); | ||
| })], | ||
| 'users.*.name' => ['nullable', 'string', 'max:255'], |
There was a problem hiding this comment.
noticing this is nullable during Update, but required during Create. Am I understanding that right?
There was a problem hiding this comment.
changed to required, but should never trigger since if not provided will be refilled with existing data
| $users = $request->get('users'); | ||
| $address_array = $request->get('address_array'); | ||
| $circle_id = $request->route('circle_id'); | ||
| return DB::transaction(function () use ($users, $address_array, $circle_id) { |
There was a problem hiding this comment.
what will the error messages look like returned from the DB transaction blocks? If there's a validation error will we pass that to the FE or will it be hidden behind another transaction error log?
There was a problem hiding this comment.
The validation is performed before this code block in RequestForm,
message: e.g {value} is required etc , if you need it changed then provide a list of error messages i'll update
There was a problem hiding this comment.
Let's merge for now and can update later.
Add cleanup when bulk deleting users with allocations
| $users = $request->get('users'); | ||
| $address_array = $request->get('address_array'); | ||
| $circle_id = $request->route('circle_id'); | ||
| return DB::transaction(function () use ($users, $address_array, $circle_id) { |
There was a problem hiding this comment.
Let's merge for now and can update later.
levity
deleted the
feature/dev-343-bulk-createupdate-user-list-api-endpoint
branch
Token authenticated endpoints for