Skip to content

Commit 9b8768d

Browse files
uberbradyuberbrady
committed
Tighten everything up, remove logging, fix last bits of functionality
1 parent 3cd1912 commit 9b8768d

File tree

6 files changed

+112
-121
lines changed

6 files changed

+112
-121
lines changed

‎app/Http/Controllers/Assets/AssetsController.php‎

Lines changed: 104 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use App\Helpers\Helper;
77
use App\Http\Controllers\Controller;
88
use App\Http\Requests\ImageUploadRequest;
9-
use App\Http\Requests\StoreMultipleAssetRequest;
9+
use App\Http\Requests\CreateMultipleAssetRequest;
1010
use App\Http\Requests\UpdateAssetRequest;
1111
use App\Models\Actionlog;
1212
use App\Http\Requests\UploadFileRequest;
@@ -99,7 +99,7 @@ public function create(Request $request) : View
9999
* @author [A. Gianotto] [<[email protected]>]
100100
* @since [v1.0]
101101
*/
102-
public function store(StoreMultipleAssetRequest $request): RedirectResponse
102+
public function store(CreateMultipleAssetRequest $request): RedirectResponse
103103
{
104104
$this->authorize(Asset::class);
105105

@@ -136,123 +136,136 @@ public function store(StoreMultipleAssetRequest $request): RedirectResponse
136136
$successes = [];
137137
$failures = [];
138138

139-
for ($a = 1, $aMax = count($asset_tags); $a <= $aMax; $a++) {
140-
$asset = new Asset();
139+
try {
140+
DB::beginTransaction();
141+
for ($a = 1, $aMax = count($asset_tags); $a <= $aMax; $a++) {
142+
$asset = new Asset();
141143

142-
$asset->model()->associate($model);
143-
$asset->name = $request->input('name');
144+
$asset->model()->associate($model);
145+
$asset->name = $request->input('name');
144146

145-
// Check for a corresponding serial
146-
if (($serials) && (array_key_exists($a, $serials))) {
147-
$asset->serial = $serials[$a];
148-
}
149-
150-
if (($asset_tags) && (array_key_exists($a, $asset_tags))) {
151-
$asset->asset_tag = $asset_tags[$a];
152-
}
147+
// Check for a corresponding serial
148+
if (($serials) && (array_key_exists($a, $serials))) {
149+
$asset->serial = $serials[$a];
150+
}
153151

154-
$asset->company_id = $companyId;
155-
$asset->model_id = $request->input('model_id');
156-
$asset->order_number = $request->input('order_number');
157-
$asset->notes = $request->input('notes');
158-
$asset->created_by = auth()->id();
159-
$asset->status_id = request('status_id');
160-
$asset->warranty_months = request('warranty_months', null);
161-
$asset->purchase_cost = request('purchase_cost');
162-
$asset->purchase_date = request('purchase_date', null);
163-
$asset->asset_eol_date = request('asset_eol_date', null);
164-
$asset->assigned_to = request('assigned_to', null);
165-
$asset->supplier_id = request('supplier_id', null);
166-
$asset->requestable = request('requestable', 0);
167-
$asset->rtd_location_id = request('rtd_location_id', null);
168-
$asset->byod = request('byod', 0);
169-
170-
if (! empty($settings->audit_interval)) {
171-
$asset->next_audit_date = Carbon::now()->addMonths((int) $settings->audit_interval)->toDateString();
172-
}
152+
if (($asset_tags) && (array_key_exists($a, $asset_tags))) {
153+
$asset->asset_tag = $asset_tags[$a];
154+
}
173155

174-
// Set location_id to rtd_location_id ONLY if the asset isn't being checked out
175-
if (!request('assigned_user') && !request('assigned_asset') && !request('assigned_location')) {
176-
$asset->location_id = $request->input('rtd_location_id', null);
177-
}
156+
$asset->company_id = $companyId;
157+
$asset->model_id = $request->input('model_id');
158+
$asset->order_number = $request->input('order_number');
159+
$asset->notes = $request->input('notes');
160+
$asset->created_by = auth()->id();
161+
$asset->status_id = request('status_id');
162+
$asset->warranty_months = request('warranty_months', null);
163+
$asset->purchase_cost = request('purchase_cost');
164+
$asset->purchase_date = request('purchase_date', null);
165+
$asset->asset_eol_date = request('asset_eol_date', null);
166+
$asset->assigned_to = request('assigned_to', null);
167+
$asset->supplier_id = request('supplier_id', null);
168+
$asset->requestable = request('requestable', 0);
169+
$asset->rtd_location_id = request('rtd_location_id', null);
170+
$asset->byod = request('byod', 0);
171+
172+
if (!empty($settings->audit_interval)) {
173+
$asset->next_audit_date = Carbon::now()->addMonths((int)$settings->audit_interval)->toDateString();
174+
}
178175

179-
if ($request->has('use_cloned_image')) {
180-
$cloned_model_img = Asset::select('image')->find($request->input('clone_image_from_id'));
181-
if ($cloned_model_img) {
182-
$new_image_name = 'clone-'.date('U').'-'.$cloned_model_img->image;
183-
$new_image = 'assets/'.$new_image_name;
184-
Storage::disk('public')->copy('assets/'.$cloned_model_img->image, $new_image);
185-
$asset->image = $new_image_name;
176+
// Set location_id to rtd_location_id ONLY if the asset isn't being checked out
177+
if (!request('assigned_user') && !request('assigned_asset') && !request('assigned_location')) {
178+
$asset->location_id = $request->input('rtd_location_id', null);
186179
}
187180

188-
} else {
189-
$asset = $request->handleImages($asset);
190-
}
181+
if ($request->has('use_cloned_image')) {
182+
$cloned_model_img = Asset::select('image')->find($request->input('clone_image_from_id'));
183+
if ($cloned_model_img) {
184+
$new_image_name = 'clone-' . date('U') . '-' . $cloned_model_img->image;
185+
$new_image = 'assets/' . $new_image_name;
186+
Storage::disk('public')->copy('assets/' . $cloned_model_img->image, $new_image);
187+
$asset->image = $new_image_name;
188+
}
191189

192-
// Update custom fields in the database.
193-
// Validation for these fields is handled through the AssetRequest form request
190+
} else {
191+
$asset = $request->handleImages($asset);
192+
}
194193

195-
if (($model) && ($model->fieldset)) {
196-
foreach ($model->fieldset->fields as $field) {
197-
if ($field->field_encrypted == '1') {
198-
if (Gate::allows('assets.view.encrypted_custom_fields')) {
194+
// Update custom fields in the database.
195+
// Validation for these fields is handled through the AssetRequest form request
196+
197+
if (($model) && ($model->fieldset)) {
198+
foreach ($model->fieldset->fields as $field) {
199+
if ($field->field_encrypted == '1') {
200+
if (Gate::allows('assets.view.encrypted_custom_fields')) {
201+
if (is_array($request->input($field->db_column))) {
202+
$asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column)));
203+
} else {
204+
$asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column));
205+
}
206+
}
207+
} else {
199208
if (is_array($request->input($field->db_column))) {
200-
$asset->{$field->db_column} = Crypt::encrypt(implode(', ', $request->input($field->db_column)));
209+
$asset->{$field->db_column} = implode(', ', $request->input($field->db_column));
201210
} else {
202-
$asset->{$field->db_column} = Crypt::encrypt($request->input($field->db_column));
211+
$asset->{$field->db_column} = $request->input($field->db_column);
203212
}
204213
}
205-
} else {
206-
if (is_array($request->input($field->db_column))) {
207-
$asset->{$field->db_column} = implode(', ', $request->input($field->db_column));
208-
} else {
209-
$asset->{$field->db_column} = $request->input($field->db_column);
210-
}
211214
}
212215
}
213-
}
214216

215-
\Log::error("About to check validity and, possibly, save...");
216-
// Validate the asset before saving
217-
if ($asset->isValid() && $asset->save()) {
218-
$target = null;
219-
$location = null;
217+
// Validate the asset before saving
218+
// Note - it can be tempting to instead want to call saveOrFail(), to automatically throw when an object
219+
// is invalid (and can't save). But this won't work, because Custom Fields _overrides_ the save() method
220+
// to inject the Custom Field Rules into the $rules property right before invoking the _real_ save.
221+
// so, instead, we have to catch failures on the 'else' clause and throw there.
222+
if ($asset->isValid() && $asset->save()) {
223+
$target = null;
224+
$location = null;
220225

221-
if ($userId = request('assigned_user')) {
222-
$target = User::find($userId);
226+
if ($userId = request('assigned_user')) {
227+
$target = User::find($userId);
223228

224-
if (!$target) {
225-
return redirect()->back()->withInput()->with('error', trans('admin/hardware/message.create.target_not_found.user'));
226-
}
227-
$location = $target->location_id;
229+
if (!$target) {
230+
return redirect()->back()->withInput()->with('error', trans('admin/hardware/message.create.target_not_found.user'));
231+
}
232+
$location = $target->location_id;
228233

229-
} elseif ($assetId = request('assigned_asset')) {
230-
$target = Asset::find($assetId);
234+
} elseif ($assetId = request('assigned_asset')) {
235+
$target = Asset::find($assetId);
231236

232-
if (!$target) {
233-
return redirect()->back()->withInput()->with('error', trans('admin/hardware/message.create.target_not_found.asset'));
234-
}
235-
$location = $target->location_id;
237+
if (!$target) {
238+
return redirect()->back()->withInput()->with('error', trans('admin/hardware/message.create.target_not_found.asset'));
239+
}
240+
$location = $target->location_id;
236241

237-
} elseif ($locationId = request('assigned_location')) {
238-
$target = Location::find($locationId);
242+
} elseif ($locationId = request('assigned_location')) {
243+
$target = Location::find($locationId);
239244

240-
if (!$target) {
241-
return redirect()->back()->withInput()->with('error', trans('admin/hardware/message.create.target_not_found.location'));
245+
if (!$target) {
246+
return redirect()->back()->withInput()->with('error', trans('admin/hardware/message.create.target_not_found.location'));
247+
}
248+
$location = $target->id;
242249
}
243-
$location = $target->id;
244-
}
245250

246-
if (isset($target)) {
247-
$asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location);
248-
}
251+
if (isset($target)) {
252+
$asset->checkOut($target, auth()->user(), date('Y-m-d H:i:s'), $request->input('expected_checkin', null), 'Checked out on asset creation', $request->get('name'), $location);
253+
}
249254

250-
$successes[] = "<a href='" . route('hardware.show', $asset) . "' style='color: white;'>" . e($asset->asset_tag) . "</a>";
255+
$successes[] = "<a href='" . route('hardware.show', $asset) . "' style='color: white;'>" . e($asset->asset_tag) . "</a>";
251256

252-
} else {
253-
$failures[] = join(",", $asset->getErrors()->all());
257+
} else {
258+
$asset->throwValidationException(); // we have to do this for the reason listed above - can't use saveOrFail()
259+
$failures[] = join(",", $asset->getErrors()->all()); //TODO - this can probably go away soon
260+
}
254261
}
262+
} catch (\Throwable $e) {
263+
\Log::debug("Caught exception in multi-create - rolling back: " . $e->getMessage());
264+
DB::rollBack();
265+
throw $e;
255266
}
267+
DB::commit();
268+
256269
if($request->get('redirect_option') === 'back'){
257270
session()->put(['redirect_option' => 'index']);
258271
} else {

‎app/Http/Requests/StoreMultipleAssetRequest.php‎ renamed to ‎app/Http/Requests/CreateMultipleAssetRequest.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use App\Rules\UniqueUndeleted;
1212
use Illuminate\Support\Str;
1313

14-
class StoreMultipleAssetRequest extends ImageUploadRequest //should I extend from StoreAssetRequest? FIXME OR TODO OR THINKME
14+
class CreateMultipleAssetRequest extends ImageUploadRequest //should I extend from StoreAssetRequest? FIXME OR TODO OR THINKME
1515
{
1616
use MayContainCustomFields;
1717

@@ -20,7 +20,7 @@ class StoreMultipleAssetRequest extends ImageUploadRequest //should I extend fro
2020
*/
2121
public function authorize(): bool
2222
{
23-
return true; //FIXME - should I do the auth check here?
23+
return true; //TODO - should I do the auth check here?
2424
}
2525

2626
/**
@@ -59,8 +59,6 @@ public function rules(): array
5959
} else {
6060
$serial_rules[] = 'nullable';
6161
}
62-
\Log::error("Serial Rules are: " . print_r($serial_rules, true));
63-
\Log::error("Asset Tag Rules are: " . print_r($asset_tag_rules, true));
6462

6563
return array_merge($modelRules, [
6664
'serials.*' => $serial_rules,

‎app/Http/Traits/UniqueUndeletedTrait.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ trait UniqueUndeletedTrait
1313
*/
1414
protected function prepareUniqueUndeletedRule($parameters, $field)
1515
{
16-
\Log::error("Preparing unique undeleted rule for $field");
1716
// Only perform a replacement if the model has been persisted.
1817
if ($this->exists) {
1918
return 'unique_undeleted:'.$this->table.','.$this->getKey();

‎app/Models/Asset.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public function declinedCheckout(User $declinedBy, $signature)
115115
'location_id' => ['nullable', 'exists:locations,id', 'fmcs_location'],
116116
'rtd_location_id' => ['nullable', 'exists:locations,id', 'fmcs_location'],
117117
'purchase_date' => ['nullable', 'date', 'date_format:Y-m-d'],
118-
'serial' => ['nullable', 'string', 'unique_undeleted:assets,serial'], // FIXME - doesn't this require unique serials *ALWAYS*?!?
118+
'serial' => ['nullable', 'string', 'unique_undeleted:assets,serial'],
119119
'purchase_cost' => ['nullable', 'numeric', 'gte:0', 'max:99999999999999999.99'],
120120
'supplier_id' => ['nullable', 'exists:suppliers,id'],
121121
'asset_eol_date' => ['nullable', 'date'],

‎app/Rules/UniqueUndeleted.php‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,11 @@ public function __construct(
2525

2626
public function setValidator(Validator $validator): static
2727
{
28-
// \Log::error("Validator has been SET and it is: ".print_r(get_object_vars($validator), true));
2928
$this->validator = $validator;
3029
$this->data = $validator->getData();
3130
//TODO - can we somehow grab the ID of the route-model-bound object, and omit its ID?
3231
// to do that, we'd have to know _which_ parameter in the validator is actually the R-M-B'ed
33-
// parameter
32+
// parameter. Or maybe we just change the function signature to let you specify it.
3433

3534
return $this;
3635
}
@@ -42,19 +41,16 @@ public function setValidator(Validator $validator): static
4241
*/
4342
public function validate(string $attribute, mixed $value, Closure $fail): void
4443
{
45-
\Log::error("Going to validate via RULE for attribute: $attribute which has value: " . print_r($value, true));
46-
// $this->validator->{$something} should maybe get the object?
47-
// but, so far, we're only using this on creating a 'new thing' so there's just a simple check needed, I guess?
4844
$query = DB::table($this->table)->whereNull('deleted_at');
49-
$query->where($this->columns[0], '!=', $value); //the first column to check
45+
$query->where($this->columns[0], '==', $value); //the first column to check
5046
$translation_string = 'validation.unique_undeleted'; //the normal validation string for a single-column check
5147
foreach (array_slice($this->columns, 1) as $column) {
5248
$translation_string = 'validation.two_column_unique_undeleted';
53-
$query->where($column, '!=', $this->data[$column]);
49+
$query->where($column, '==', $this->data[$column]);
5450
}
55-
// \Log::error("Here is what you get if you ask for the ID directly: " . $this->validator->id);
51+
5652
if ($query->count() > 0) {
57-
$fail()->translate($translation_string);
53+
$fail($translation_string)->translate();
5854
}
5955
}
6056
}

‎resources/views/hardware/edit.blade.php‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,6 @@
7676
@foreach(array_keys($old_tags) as $i)
7777
{{-- This is mostly stolen from the HTML that we add via javascript on the 'add_field_button' handler in the embedded JS below --}}
7878
{{-- @include ('partials.forms.edit.serial', ['fieldname'=> 'serials['.$loop->iteration.']', 'old_val_name' => 'serials.'.$loop->iteration, 'translated_serial' => trans('admin/hardware/form.serial')])--}}
79-
@php
80-
\Log::error("I is: $i");
81-
@endphp
8279
<span class="fields_wrapper">
8380
<div class="form-group {{ $errors->has('asset_tags.'.$i) ? ' has-error' : '' }}"><label for="asset_tag"
8481
class="col-md-3 control-label">{{ trans('admin/hardware/form.tag') }} {{ $i }}</label>
@@ -93,18 +90,6 @@ class="col-md-3 control-label">{{ trans('admin/hardware/form.tag') }} {{ $i }}</
9390
</div>
9491
</div>
9592
@include ('partials.forms.edit.serial', ['fieldname'=> 'serials['.$i.']', 'old_val_name' => 'serials.'.$i, 'translated_serial' => trans('admin/hardware/form.serial')])
96-
{{-- <div class="form-group {{ $errors->has('serials.'.$i) ? ' has-error': '' }}"><label for="serial"
97-
class="col-md-3 control-label">{{ trans('admin/hardware/form.serial') }} {{ $i }}</label>
98-
<div class="col-md-7 col-sm-12">
99-
<input type="text" class="form-control" name="serials[{{ $i }}]"
100-
value="{{ old('serials.'.$i) }}">
101-
@error('serials.'.$i )
102-
<span class="alert-msg" aria-hidden="true">
103-
<i class="fas fa-times" aria-hidden="true"></i> {{ $message }}
104-
</span>
105-
@enderror
106-
</div>
107-
</div> --}}
10893
</span>
10994
@endforeach
11095
</div>

0 commit comments

Comments
 (0)