Merge remote-tracking branch 'origin/develop'

Author: snipe

.ai/rules/tests.md

@@ -24,3 +24,19 @@ Use `$this->actingAsForApi($user)` in API tests and `$this->actingAs($user)` in
 
 ## Feature and unit test placement
 Feature tests live in `tests/Feature/`, grouped by the feature or resource under test. A resource with both API and web controllers splits its tests into `Api/` and `Ui/` subfolders matching the layer under test; cross-cutting feature-area suites stay flat in their folder. Unit tests live in `tests/Unit/`, grouped by the type under test (`Models/`, `Presenters/`, `Transformers/`...).
+
+## Avoid calling truncate() in tests — use delete() or forceDelete()
+
+Avoid using `Model::truncate()` or `DB::table(...)->truncate()` in a test. Use `Model::query()->delete()` (soft delete)
+or `Model::query()->forceDelete()` (hard delete) instead — both are DML and roll back normally.
+
+Why: TRUNCATE is DDL, and MySQL implicitly commits on DDL. That destroys the transaction `LazilyRefreshDatabase` wraps
+each test in. Laravel spots the dead transaction at teardown, clears `RefreshDatabaseState::$migrated`, and re-runs
+all migrations before the next test — roughly 10s per occurrence on MySQL. SQLite hides the problem entirely
+because its DDL is transactional, so this only shows up on a MySQL run.
+
+The same trap applies to any runtime DDL, notably `Schema::table()` — which is why creating a `CustomField` (its
+`created` hook does `ALTER TABLE assets`) is expensive on MySQL.
+
+Tell: the test that runs the DDL looks fast; the *next* test pays. A test suddenly reporting ~1,690 queries is paying
+for a re-migration.

app/Console/Commands/LdapSync.php

@@ -51,8 +51,8 @@ public function handle()
             exit();
         }
 
-        ini_set('max_execution_time', env('LDAP_TIME_LIM', 600)); // 600 seconds = 10 minutes
-        ini_set('memory_limit', env('LDAP_MEM_LIM', '500M'));
+        ini_set('max_execution_time', config('app.ldap_time_limit')); // 600 seconds = 10 minutes
+        ini_set('memory_limit', config('app.ldap_memory_limit'));
 
         // Single source of truth for internal-key => LDAP-attribute-name
         // lives on the Ldap model so parseAndMapLdapAttributes and this

app/Console/Commands/LdapTroubleshooter.php

@@ -627,13 +627,11 @@ private function timed_boolean_execute($function)
                         posix_kill($parent_pid, SIGUSR2);
                     }
                     exit();
-                    break; // yes I know we don't need it.
                 case -1:
                     // couldn't fork
                     $this->error('COULD NOT FORK - assuming failure');
 
                     return false;
-                    break; // I still know that we don't need it
                 default:
                     // we remain the 'parent', $pid is the PID of the forked process.
                     $siginfo = [];
@@ -645,7 +643,6 @@ private function timed_boolean_execute($function)
 
                         return false;
                     }
-                    break; // Yeah I get it already, shush.
             }
         }
 

app/Console/Commands/MoveUploadsToNewDisk.php

@@ -89,7 +89,7 @@ public function handle()
             $type_count++;
             $filename = basename($logo);
             Storage::disk('public')->put('uploads/'.$filename, file_get_contents($logo));
-            $this->info($type_count.'. LOGO: '.$filename.' was copied to '.env('PUBLIC_AWS_URL').'/uploads/'.$filename);
+            $this->info($type_count.'. LOGO: '.$filename.' was copied to '.config('filesystems.disks.public_aws.url').'/uploads/'.$filename);
         }
 
         $private_uploads['assets'] = glob('storage/private_uploads/assets'.'/*.*');

app/Console/Commands/ObjectImportCommand.php

@@ -54,8 +54,8 @@ public function __construct()
      */
     public function handle()
     {
-        ini_set('max_execution_time', env('IMPORT_TIME_LIMIT', 600)); // 600 seconds = 10 minutes
-        ini_set('memory_limit', env('IMPORT_MEMORY_LIMIT', '500M'));
+        ini_set('max_execution_time', config('importer.time_limit')); // 600 seconds = 10 minutes
+        ini_set('memory_limit', config('importer.memory_limit'));
 
         $this->progressIndicator = new ProgressIndicator($this->output);
 

app/Console/Commands/SystemBackup.php

@@ -41,7 +41,7 @@ public function __construct()
      */
     public function handle()
     {
-        ini_set('max_execution_time', env('BACKUP_TIME_LIMIT', 600)); // 600 seconds = 10 minutes
+        ini_set('max_execution_time', config('backup.time_limit')); // 600 seconds = 10 minutes
 
         if ($this->option('filename')) {
             $filename = $this->option('filename');

app/Console/Commands/ValidateAssets.php

@@ -4,7 +4,6 @@
 
 use App\Models\Asset;
 use Illuminate\Console\Command;
-use Illuminate\Support\MessageBag;
 
 class ValidateAssets extends Command
 {
@@ -69,18 +68,7 @@ public function handle()
 
     private function formatValidationErrors(Asset $asset): string
     {
-        $errors = $asset->getErrors();
-        $messages = [];
-
-        if ($errors instanceof MessageBag) {
-            $messages = $errors->all();
-        } elseif (is_array($errors)) {
-            $messages = $errors;
-        } else {
-            $messages = [(string) $errors];
-        }
-
-        $prefixedMessages = collect($messages)
+        $prefixedMessages = collect($asset->getErrors()->all())
             ->map(fn ($message) => trim((string) $message))
             ->filter()
             ->map(fn (string $message) => str_starts_with($message, '✘') ? $message : '✘ '.$message)

app/Exceptions/Handler.php

@@ -15,7 +15,6 @@
 use Illuminate\Http\Response;
 use Illuminate\Session\TokenMismatchException;
 use Illuminate\Support\Facades\Lang;
-use Illuminate\Support\Facades\Log;
 use Illuminate\Validation\ValidationException;
 use Intervention\Image\Exception\NotSupportedException;
 use JsonException;
@@ -60,14 +59,36 @@ class Handler extends ExceptionHandler
     public function report(Throwable $exception)
     {
         if ($this->shouldReport($exception)) {
-            if (class_exists(Log::class)) {
-                Log::error($exception);
-            }
-
             return parent::report($exception);
         }
     }
 
+    /**
+     * Report a caught exception, and rethrow in dev when the underlying
+     * cause is a programmer-error \Error (TypeError, ArgumentCountError,
+     * etc.) so it fails loud with a stack trace in dev instead of hiding behind
+     * a friendly "something went wrong" flash. Plain \Exception (sub)types
+     * (QueryException, ItemStillHasAssets, etc.) *always* report + return so
+     * bulk operations can keep swallowing runtime-data failures per row.
+     *
+     * We should use this as a drop-in for `report($e)` inside the wide-net
+     * catch (\Throwable $e) blocks in the bulk destroy / import paths.
+     */
+    public static function reportOrRethrow(Throwable $e): void
+    {
+        // Both APP_DEBUG must be on AND the environment must
+        // not be production before the raw \Error is allowed to
+        // escape past the friendly user-facing error.
+        if (
+            config('app.debug')
+            && ! app()->environment('production')
+            && $e instanceof \Error
+        ) {
+            throw $e;
+        }
+        report($e);
+    }
+
     /**
      * Render an exception into an HTTP response.
      *
@@ -161,7 +182,7 @@ public function render($request, Throwable $e)
         // This is traaaaash but it handles models that are not found while using route model binding :(
         // The only alternative is to set that at *each* route, which is crazypants
         if ($e instanceof ModelNotFoundException) {
-            $ids = method_exists($e, 'getIds') ? $e->getIds() : [];
+            $ids = $e->getIds();
 
             if (in_array('bulkedit', $ids, true)) {
                 $error_array = session()->get('bulk_asset_errors');
@@ -198,7 +219,7 @@ public function render($request, Throwable $e)
             // Normalize the space-separated derived name to underscore
             // so compound class names (AssetModel -> "asset model" -> "asset_model")
             // resolve to keys that actually exist in general.php.
-            $translationKey = 'general.' . str_replace(' ', '_', $model_name);
+            $translationKey = 'general.'.str_replace(' ', '_', $model_name);
             $translatedName = Lang::has($translationKey) ? trans($translationKey) : $model_name;
 
             return redirect()

app/Helpers/Helper.php

@@ -1642,7 +1642,6 @@ public static function isDemoMode()
     {
         if (config('app.lock_passwords') === true) {
             return true;
-            Log::debug('app locked!');
         }
 
         return false;
@@ -1699,8 +1698,6 @@ public static function getUnitConversionFactor($unit)
                 return (1 / 72) * static::getUnitConversionFactor('in');
             default:
                 throw new \InvalidArgumentException('Unit: '.e($unit).' is not supported');
-
-                return false;
         }
     }
 
@@ -1846,6 +1843,7 @@ public static function getRedirectOption($request, $id, $table, $item_id = null)
                 'Components' => route('components.index'),
                 'Consumables' => route('consumables.index'),
                 'Maintenances' => route('maintenances.index'),
+                default => route('home'),
             };
 
             // #15214: preserve query-string filters when the user came
@@ -1871,6 +1869,7 @@ public static function getRedirectOption($request, $id, $table, $item_id = null)
                 'Accessories' => redirect()->route('accessories.show', $id ?? $item_id),
                 'Components' => redirect()->route('components.show', $id ?? $item_id),
                 'Consumables' => redirect()->route('consumables.show', $id ?? $item_id),
+                default => redirect()->route('home'),
             };
         }
 
@@ -1890,6 +1889,7 @@ public static function getRedirectOption($request, $id, $table, $item_id = null)
                 'asset' => $assetId
                     ? redirect()->route('hardware.show', $assetId)
                     : redirect()->route('hardware.index'),
+                default => redirect()->route('home'),
             };
         }
 
@@ -1898,6 +1898,7 @@ public static function getRedirectOption($request, $id, $table, $item_id = null)
             return match ($other_redirect) {
                 'audit' => redirect()->route('assets.audit.due'),
                 'model' => redirect()->route('models.show', $request->model_id),
+                default => redirect()->route('home'),
             };
 
         }

app/Http/Controllers/Accessories/AccessoryCheckinController.php

@@ -67,6 +67,7 @@ public function store(Request $request, $accessoryCheckoutId = null, $backto = n
             'App\Models\User' => 'user',
             'App\Models\Location' => 'location',
             'App\Models\Asset' => 'asset',
+            default => null,
         });
 
         $checkin_hours = date('H:i:s');