# PRD: Branding Module Bug Fixes

## Overview
Four QA test cases are failing in the Branding module. This PRD defines the root cause and exact code changes required to fix each one.

## Failing Tests Summary

| Test ID | Sub-module       | Scenario                                      |
|---------|------------------|-----------------------------------------------|
| BRD-012 | Calendar Colors  | Blank/cleared color silently ignored — no validation feedback |
| BRD-025 | Logos & Domains  | Invalid domain format accepted — no format validation |
| BRD-026 | Logos & Domains  | Duplicate domain allowed per company — no uniqueness check |
| BRD-032 | Logos & Domains  | Unsupported file type not rejected — errors swallowed by catch block |

---

## Root Cause Analysis

### BRD-012 — Calendar Colors: Blank Color Behavior

**File:** `public/assets/js/calendar-color.js` (line 57)

**Root Cause:** The JS save handler contains a guard `if (!color) { return; }` that silently aborts execution when the color value is blank, providing zero feedback to the user. The backend validation rule `'calendar_color' => 'required|string|min:4|max:40'` is correct but is never reached because the JS exits early without posting.

**Expected (per test):** System should restore default color or show required validation message.

---

### BRD-025 — Logos & Domains: Invalid Domain Format Not Rejected

**File:** `app/Http/Controllers/Configuration/BrandingAssetController.php`, method `__formPost()` (line 85)

**Root Cause:** Domain validation rule is `'required|max:255'` — no format check. Any arbitrary string (spaces, special characters, non-domain strings) up to 255 characters is accepted as a valid domain.

---

### BRD-026 — Logos & Domains: Duplicate Domain Not Prevented

**File:** `app/Http/Controllers/Configuration/BrandingAssetController.php`, method `__formPost()` (line 85)

**Root Cause:** No `unique` constraint on `domain` in either the validation rules or the database. Multiple records with identical domains can be created for the same company. The `MasterBrandingAsset::store()` method performs a plain insert/update with no duplicate detection.

---

### BRD-032 — Logos & Domains: Unsupported File Type Not Rejected

**File:** `app/Http/Controllers/Configuration/BrandingAssetController.php`, method `__formPost()` (lines 80–95)

**Root Cause:** The `$this->validate()` call is placed **inside** a `try { ... } catch (\Exception $e)` block. Laravel's `ValidationException` (which extends `\Exception`) is caught by that block and converted to `Helper::rj($e->getMessage(), 500)`. This returns an unstructured 500-style error response instead of a proper 422 JSON response with field-level messages. The form's AJAX handler never receives per-field validation errors, so no validation message is displayed to the user when an unsupported file type is uploaded.

The file validation rules themselves are structurally correct:
```php
'site_logo'   => 'nullable|image|mimes:jpeg,png,jpg,gif,svg|max:2048',
'new_ui_logo' => 'nullable|image|mimes:jpeg,png,jpg,gif,svg|max:2048',
'icon'        => 'nullable|image|mimes:jpeg,png,jpg,gif,svg|max:1024',
```
The fix is purely about ensuring `ValidationException` propagates rather than being swallowed.

---

## Fixes

### Fix 1 — BRD-012: Show Error Toast on Blank Calendar Color

**File:** `public/assets/js/calendar-color.js`

Replace the silent `return` with an error toast so the user gets required-field feedback.

```js
// BEFORE (line 57)
if (!color) { return; }

// AFTER
if (!color) {
    showErrorToast('Please select a color before saving.');
    return;
}
```

---

### Fix 2 — BRD-025, BRD-026, BRD-032: Rewrite `__formPost()` Validation

**File:** `app/Http/Controllers/Configuration/BrandingAssetController.php`

Three changes in one method:
1. **Move `$this->validate()` BEFORE the `try-catch`** so `ValidationException` propagates naturally and Laravel emits a proper 422 JSON response with per-field errors (fixes BRD-032).
2. **Add domain format regex** (fixes BRD-025).
3. **Add unique domain rule scoped to `company_id`** with soft-delete awareness and self-ignore on edit (fixes BRD-026).

**Full rewritten method:**

```php
use Illuminate\Validation\Rule;

protected function __formPost(Request $request, $id = 0)
{
    $companyId = (int) \Auth::user()->company_id;

    // Validation is intentionally outside try-catch so ValidationException
    // propagates as a proper 422 response instead of being swallowed as 500.
    $this->validate($request, [
        'site_title'  => 'required|max:255',
        'domain'      => [
            'required',
            'max:255',
            'regex:/^([a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?\.)+[a-zA-Z]{2,}$/',
            Rule::unique('master_branding_assets', 'domain')
                ->where(fn ($q) => $q->where('company_id', $companyId)->whereNull('deleted_at'))
                ->ignore($id),
        ],
        'site_logo'   => 'nullable|image|mimes:jpeg,png,jpg,gif,svg|max:2048',
        'new_ui_logo' => 'nullable|image|mimes:jpeg,png,jpg,gif,svg|max:2048',
        'icon'        => 'nullable|image|mimes:jpeg,png,jpg,gif,svg|max:1024',
    ], [
        'domain.regex'  => 'The domain must be a valid domain name (e.g. example.com or sub.example.com).',
        'domain.unique' => 'This domain is already registered for your company.',
    ]);

    try {
        $input    = $request->all();
        $response = $this->_model->store($input, $id, $request);
        return $this->redirectToContext($request, 'branding-assets')
            ->with(in_array($response['status'], [200, 201]) ? 'success' : 'error', $response['message']);
    } catch (\Exception $e) {
        \App\Models\ErrorLog::Log($e);
        return Helper::rj($e->getMessage(), 500);
    }
}
```

---

## Files to Modify

| File | Lines Affected | Test IDs |
|------|---------------|----------|
| `public/assets/js/calendar-color.js` | ~57 | BRD-012 |
| `app/Http/Controllers/Configuration/BrandingAssetController.php` | `__formPost()` ~80–95 | BRD-025, BRD-026, BRD-032 |

---

## Test Coverage Map

| Test ID | Scenario | Fix Applied |
|---------|----------|-------------|
| BRD-012 | Blank calendar color — no feedback | Fix 1: JS error toast on blank color |
| BRD-025 | Invalid domain format accepted | Fix 2: `regex` validation rule on `domain` |
| BRD-026 | Duplicate domain allowed | Fix 2: `Rule::unique` scoped to `company_id` + soft-delete |
| BRD-032 | Unsupported file type not rejected | Fix 2: `$this->validate()` moved outside `try-catch` |

---

## No DB Migration Required

No schema changes are needed. The uniqueness constraint is enforced at the application layer via `Rule::unique()`. The `master_branding_assets` table already has a `deleted_at` column (via `SoftDeletes`), and the unique rule filters on `whereNull('deleted_at')` to exclude soft-deleted rows.