Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
- Give every custom field a public id, so an API that hands one out names it the way the rest of the platform names a resource rather than exposing an internal uuid. `CustomField` takes `HasPublicId` with the `custom_field` prefix, and `public_id` becomes fillable.
- Mint an id on the one path that would otherwise miss it: `HasCustomFields::setCustomField()` saves a field it creates on the fly with `saveQuietly()`, which skips the hook that assigns the id.

## Fixes

- Let an observer's refusal reach the caller on the update and bulk-delete paths. An observer that refused a write by throwing `FleetbaseRequestValidationException` had its explanation discarded: `HasApiModelBehavior::updateRecordFromRequest()` rewrapped every exception from the save as a plain `\Exception`, and `HasApiControllerBehavior::bulkDelete()` caught `\Exception` ahead of its dedicated handler, so callers saw `Invalid request` or a generic update error instead of the message the observer wrote. The exception now passes through untouched on both paths and is rendered with `getErrors()`, as it already was on create and single delete. Every other exception is wrapped exactly as before. Reported in [#256](https://github.com/fleetbase/core-api/issues/256).

## Reliability

- Backfill existing rows in the migration, and add the column as nullable and indexed rather than unique-and-required, so it is safe on an already-populated `custom_fields` table.
Expand All @@ -14,4 +18,4 @@ This is platform-wide: every custom field gains a public id, not only those used

A database migration is required. No configuration change is needed.

Changes: [#254](https://github.com/fleetbase/core-api/pull/254).
Changes: [#254](https://github.com/fleetbase/core-api/pull/254), [#259](https://github.com/fleetbase/core-api/pull/259).
12 changes: 4 additions & 8 deletions src/Traits/HasApiControllerBehavior.php
Original file line number Diff line number Diff line change
Expand Up @@ -627,17 +627,13 @@ public function bulkDelete(BulkDeleteRequest $request)

try {
$count = $this->model->bulkRemove($ids);
} catch (\Exception $e) {
return response()->error($e->getMessage());
// QueryException and FleetbaseRequestValidationException are covered by
// the preceding Exception catch in PHP's current catch order.
// @codeCoverageIgnoreStart
} catch (QueryException $e) {
return response()->error($e->getMessage());
} catch (FleetbaseRequestValidationException $e) {
return response()->error($e->getErrors());
} catch (QueryException $e) {
return response()->error($e->getMessage());
} catch (\Exception $e) {
return response()->error($e->getMessage());
}
// @codeCoverageIgnoreEnd

return response()->json(
[
Expand Down
5 changes: 5 additions & 0 deletions src/Traits/HasApiModelBehavior.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Fleetbase\Traits;

use Fleetbase\Exceptions\FleetbaseRequestValidationException;
use Fleetbase\Support\ApiModelCache;
use Fleetbase\Support\Auth;
use Fleetbase\Support\Http;
Expand Down Expand Up @@ -416,6 +417,10 @@ public function updateRecordFromRequest(Request $request, $id, ?callable $onBefo
$input = Arr::except($input, ['uuid', 'public_id', 'deleted_at', 'updated_at', 'created_at']);
try {
$record->update($input);
} catch (FleetbaseRequestValidationException $e) {
// An observer refusing the write is user-facing feedback, not an internal failure.
// Let it through untouched so the controller and global handler can render getErrors().
throw $e;
} catch (\Exception $e) {
throw new \Exception(app()->hasDebugModeEnabled() ? $e->getMessage() : 'Failed to update ' . $this->getApiHumanReadableName());
}
Expand Down
32 changes: 32 additions & 0 deletions tests/Unit/Traits/HasApiControllerBehaviorTest.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

use Fleetbase\Exceptions\FleetbaseRequestValidationException;
use Fleetbase\Http\Requests\Internal\BulkDeleteRequest;
use Fleetbase\Http\Resources\FleetbaseResource;
use Fleetbase\Traits\HasApiControllerBehavior;
Expand Down Expand Up @@ -506,6 +507,37 @@ public function updateRecordFromRequest(Request $request, $id, ?callable $onBefo
->and($updateQueryFailure->getData(true))->toBe(['errors' => ['Error occurred while trying to update a Widget']]);
});

test('api controller behavior surfaces observer refusals on update and bulk delete', function () {
$refusingModel = new class extends HasApiControllerBehaviorModel {
public function updateRecordFromRequest(Request $request, $id, ?callable $onBefore = null, ?callable $onAfter = null): self
{
throw new FleetbaseRequestValidationException(['Widget is locked and cannot be changed.']);
}

public function bulkRemove(array $ids): int
{
throw new FleetbaseRequestValidationException(['Widget is locked and cannot be deleted.']);
}
};
$queryFailingModel = new class extends HasApiControllerBehaviorModel {
public function bulkRemove(array $ids): int
{
throw new QueryException('mysql', 'delete from widgets', [], new RuntimeException('database unavailable'));
}
};

$refusingController = new HasApiControllerBehaviorController($refusingModel);
$updateRefusal = $refusingController->updateRecord(has_api_controller_behavior_request('/v1/widgets/widget-1', 'PATCH', ['name' => 'Nope']), 'widget-1');
$bulkDeleteRefusal = $refusingController->bulkDelete(BulkDeleteRequest::create('/v1/widgets/bulk-delete', 'DELETE', ['ids' => ['widget-1']]));
$bulkDeleteQuery = (new HasApiControllerBehaviorController($queryFailingModel))->bulkDelete(
BulkDeleteRequest::create('/v1/widgets/bulk-delete', 'DELETE', ['ids' => ['widget-1']])
);

expect($updateRefusal->getData(true))->toBe(['errors' => ['Widget is locked and cannot be changed.']])
->and($bulkDeleteRefusal->getData(true))->toBe(['errors' => ['Widget is locked and cannot be deleted.']])
->and($bulkDeleteQuery->getData(true)['errors'][0])->toContain('database unavailable');
});

test('api controller behavior validates fallback rule contracts before writing', function () {
$controller = new HasApiControllerBehaviorController();
$controller->rules = ['name' => ['required']];
Expand Down
38 changes: 38 additions & 0 deletions tests/Unit/Traits/HasApiModelBehaviorTest.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

use Fleetbase\Exceptions\FleetbaseRequestValidationException;
use Fleetbase\Http\Filter\Filter;
use Fleetbase\Models\Model;
use Fleetbase\Traits\HasApiModelBehavior;
Expand Down Expand Up @@ -299,6 +300,16 @@ public function update(array $attributes = [], array $options = [])
}
}

class HasApiModelBehaviorRefusingUpdateRecord extends HasApiModelBehaviorRecord
{
protected static function booted(): void
{
static::updating(function () {
throw new FleetbaseRequestValidationException(['Record is locked and cannot be updated.']);
});
}
}

class HasApiModelBehaviorFailingBulkDeleteRecord extends HasApiModelBehaviorRecord
{
public function where($column, $operator = null, $value = null, $boolean = 'and')
Expand Down Expand Up @@ -883,6 +894,33 @@ public function total(): int
->and(fn () => (new HasApiModelBehaviorRecord())->remove('record_alpha'))->toThrow(Exception::class);
});

test('api model behavior propagates observer update refusals without rewrapping them', function () {
$capsule = has_api_model_behavior_database();
has_api_model_behavior_seed_records($capsule);
session(['company' => 'company-a']);

$refusals = [];
foreach ([true, false] as $debug) {
config(['app.debug' => $debug]);

try {
(new HasApiModelBehaviorRefusingUpdateRecord())->updateRecordFromRequest(
has_api_model_behavior_request(['name' => 'Refused update'], method: 'PATCH'),
'record_alpha'
);
} catch (Exception $exception) {
$refusals[] = $exception;
}
}

expect($refusals)->toHaveCount(2)
->and($refusals[0])->toBeInstanceOf(FleetbaseRequestValidationException::class)
->and($refusals[0]->getErrors())->toBe(['Record is locked and cannot be updated.'])
->and($refusals[1])->toBeInstanceOf(FleetbaseRequestValidationException::class)
->and($refusals[1]->getErrors())->toBe(['Record is locked and cannot be updated.'])
->and($capsule->getConnection('mysql')->table('api_model_behavior_records')->where('public_id', 'record_alpha')->value('name'))->toBe('Alpha Dispatch');
});

test('api model behavior exposes default searchable fields options and no-op query branches', function () {
$capsule = has_api_model_behavior_database();
has_api_model_behavior_seed_records($capsule);
Expand Down
Loading