From 1f8be427e5b27f643ef73e25c0cc53fbdc4f13d2 Mon Sep 17 00:00:00 2001 From: Fady Mondy Date: Tue, 15 Sep 2026 09:39:09 +0300 Subject: [PATCH 1/4] Port React table fixes to Vue and sanitize HTML text columns - CardGrid: normalize titles before string ops, replace div-based selection boxes with keyboard-operable labelled Checkboxes (reka-ui v2 model-value API), forward the full column config and recordId to grid columns - DataTable: treat non-OK reorder responses as failures (revert order), add the drag-handle cell to skeleton rows - Table: stop the immediate records watcher duplicating page > 1, reload on AJAX group change instead of calling undefined updateUrl() - ImageGridColumn: show defaultImageUrl for empty values, don't retry a failing default image - TextGridColumn: render non-badge arrays/objects as JSON, avoid Invalid Date - ApiTester: correct YAML quoting/escaping in the OpenAPI export - TextColumn::html(): sanitize values server-side (new DOMDocument-based Support\HtmlSanitizer) so Vue and React both render safe HTML Co-Authored-By: Claude Opus 5 (1M context) --- resources/js/components/ApiTester.vue | 19 ++- resources/js/components/CardGrid.vue | 94 ++++++------ resources/js/components/DataTable.vue | 10 +- resources/js/components/Table.vue | 8 +- .../grid-columns/ImageGridColumn.vue | 9 +- .../grid-columns/TextGridColumn.vue | 45 ++++-- src/Columns/TextColumn.php | 5 + src/Support/HtmlSanitizer.php | 144 ++++++++++++++++++ src/Table.php | 9 ++ tests/Unit/HtmlSanitizerTest.php | 109 +++++++++++++ 10 files changed, 375 insertions(+), 77 deletions(-) create mode 100644 src/Support/HtmlSanitizer.php create mode 100644 tests/Unit/HtmlSanitizerTest.php diff --git a/resources/js/components/ApiTester.vue b/resources/js/components/ApiTester.vue index 3116ec7..5341413 100644 --- a/resources/js/components/ApiTester.vue +++ b/resources/js/components/ApiTester.vue @@ -664,6 +664,17 @@ const exportOpenApiJson = () => { showExportMenu.value = false } +// Quote strings that would break or be re-typed as plain YAML scalars (newlines, indicators, booleans, null, +// numbers). JSON string syntax is a valid double-quoted YAML scalar and escapes newlines and quotes. +const toYamlString = (value: string): string => + value === '' || + /[\n\r:#]/.test(value) || + /^[\s\-?,[\]{}&*!|>'"%@`]|\s$/.test(value) || + /^(?:true|false|yes|no|on|off|null|~)$/i.test(value) || + !isNaN(Number(value)) + ? JSON.stringify(value) + : value + const toYaml = (obj: any, indent = 0): string => { const prefix = ' '.repeat(indent) let yaml = '' @@ -675,11 +686,7 @@ const toYaml = (obj: any, indent = 0): string => { } else if (typeof value === 'number') { yaml += `${prefix}${key}: ${value}\n` } else if (typeof value === 'string') { - if (value.includes('\n') || value.includes(':') || value.includes('#')) { - yaml += `${prefix}${key}: "${value.replace(/"/g, '\\"')}"\n` - } else { - yaml += `${prefix}${key}: ${value}\n` - } + yaml += `${prefix}${key}: ${toYamlString(value)}\n` } else if (Array.isArray(value)) { if (value.length === 0) { yaml += `${prefix}${key}: []\n` @@ -690,7 +697,7 @@ const toYaml = (obj: any, indent = 0): string => { const itemYaml = toYaml(item, indent + 2).trim() yaml += `${prefix}- ${itemYaml.split('\n').join('\n' + prefix + ' ')}\n` } else { - yaml += `${prefix}- ${item}\n` + yaml += `${prefix}- ${typeof item === 'string' ? toYamlString(item) : item}\n` } } } diff --git a/resources/js/components/CardGrid.vue b/resources/js/components/CardGrid.vue index 77624fd..64fd4a3 100644 --- a/resources/js/components/CardGrid.vue +++ b/resources/js/components/CardGrid.vue @@ -87,6 +87,28 @@ watch(() => props.clearSelections, () => { selectAll.value = false }) +// Normalize any value (number, object, null) to a display string before calling string methods on it +const toDisplayString = (value: unknown): string => { + if (value === null || value === undefined) return '' + if (typeof value === 'object') { + try { + return JSON.stringify(value) + } catch { + return String(value) + } + } + return String(value) +} + +// Forward the full column config (limit, wrap, badge, imageWidth, editable, name, ...) to the grid column +// component, restricted to the props it declares so unknown keys don't fall through as DOM attributes. +const getColumnProps = (column: any) => { + const declared = getColumnComponent(column.component)?.props + if (!declared) return {} + const keys = Array.isArray(declared) ? declared : Object.keys(declared) + return Object.fromEntries(Object.entries(column).filter(([key]) => keys.includes(key))) +} + const getColumnComponent = (columnType: string) => { const components: Record = { 'text_grid_column': TextGridColumn, @@ -300,7 +322,8 @@ const getRelativeTime = (record: any) => { // Get avatar initials from title const getAvatarInitials = (record: any) => { - const title = getTitle(record) + // Normalize first: the configured title field may hold a number or object + const title = toDisplayString(getTitle(record)) if (!title) return '?' const words = title.split(' ') if (words.length >= 2) { @@ -560,30 +583,13 @@ const handleCardClick = (event: MouseEvent, record: any) => {
-
-
- - - -
-
+ :model-value="isSelected(record.id)" + @update:model-value="() => handleSelectRecord(record.id)" + :aria-label="`Select record #${toDisplayString(record.id)}`" + class="size-5 rounded border-2 border-muted-foreground/40 bg-background cursor-pointer hover:border-primary/60" + /> #{{ record.id }}
@@ -696,7 +702,7 @@ const handleCardClick = (event: MouseEvent, record: any) => { class="absolute inset-0 w-full h-full object-cover transition-transform duration-500 group-hover:scale-110" />
- {{ getTitle(record)?.charAt(0) || '?' }} + {{ toDisplayString(getTitle(record)).charAt(0) || '?' }}
@@ -705,8 +711,9 @@ const handleCardClick = (event: MouseEvent, record: any) => {
@@ -772,26 +779,12 @@ const handleCardClick = (event: MouseEvent, record: any) => { >
-
- - - -
+
@@ -931,8 +924,9 @@ const handleCardClick = (event: MouseEvent, record: any) => {
@@ -951,8 +945,10 @@ const handleCardClick = (event: MouseEvent, record: any) => { v-for="column in gridColumns" :key="column.name" :is="getColumnComponent(column.component)" + v-bind="getColumnProps(column)" :column="column" :record="record" + :record-id="record.id" :value="record[column.name]" :color="record._colors?.[column.name]" :icon="record._icons?.[column.name]" diff --git a/resources/js/components/DataTable.vue b/resources/js/components/DataTable.vue index 4cfa8bb..5fed450 100644 --- a/resources/js/components/DataTable.vue +++ b/resources/js/components/DataTable.vue @@ -191,7 +191,7 @@ const saveReorder = async (items: { id: number | string, order: number }[]) => { isReordering.value = true try { const url = props.reorderRoute || `/admin/${props.resourceSlug}/reorder` - await fetch(url, { + const response = await fetch(url, { method: 'POST', headers: { 'Content-Type': 'application/json', @@ -204,6 +204,11 @@ const saveReorder = async (items: { id: number | string, order: number }[]) => { column: props.reorderableColumn, }), }) + + // fetch only rejects on network failure; treat 4xx/5xx as a failed save too + if (!response.ok) { + throw new Error(`Reorder request failed with status ${response.status}`) + } } catch (error) { console.error('Failed to save reorder:', error) // Revert to original order on error @@ -548,6 +553,9 @@ const getColumnWidthClass = (column: Column, index: number): string => { :key="`skeleton-${i}`" :class="[striped && i % 2 !== 0 ? 'bg-muted' : 'bg-card']" > + + + diff --git a/resources/js/components/Table.vue b/resources/js/components/Table.vue index 4bc76eb..f14ece0 100644 --- a/resources/js/components/Table.vue +++ b/resources/js/components/Table.vue @@ -284,7 +284,9 @@ watch(() => props.records, (newRecords, oldRecords) => { } else { allRecords.value = newRecords } -}, { immediate: true, deep: true }) + // Not immediate: allRecords is already seeded with props.records. Running immediately on a deep link to + // page > 1 appended the initial page to itself, duplicating every record. +}, { deep: true }) // Watch for search, filter, sort changes to reset records (only after initialization) watch([searchQuery, activeFilters, sortColumn, sortDirection], (newValues, oldValues) => { @@ -382,9 +384,9 @@ const handleGroupChange = (group: string | null) => { urlParams.delete('group') } - // If using AJAX mode, reload data + // If using AJAX mode, reload data (reloadData() sends the new group from activeGroup). + // AJAX mode keeps search/filter/group state out of the URL, so there is nothing to push here. if (props.useAjax) { - updateUrl({ group: group || undefined }) reloadData() } else { // For Inertia, do a full navigation diff --git a/resources/js/components/grid-columns/ImageGridColumn.vue b/resources/js/components/grid-columns/ImageGridColumn.vue index 4f2a50d..cc6ed64 100644 --- a/resources/js/components/grid-columns/ImageGridColumn.vue +++ b/resources/js/components/grid-columns/ImageGridColumn.vue @@ -45,7 +45,8 @@ const props = withDefaults(defineProps(), { }) const images = computed(() => { - if (!props.value) return [] + // Show default image if no value but defaultImageUrl is set + if (!props.value) return props.defaultImageUrl ? [props.defaultImageUrl] : [] return Array.isArray(props.value) ? props.value : [props.value] }) @@ -144,8 +145,10 @@ const getImageUrl = (image: string): string => { } const handleImageError = (event: Event) => { - if (props.defaultImageUrl) { - (event.target as HTMLImageElement).src = props.defaultImageUrl + const imgElement = event.target as HTMLImageElement + // Only fall back once: if the default image itself fails, stop (prevents an error/reload loop) + if (props.defaultImageUrl && imgElement.getAttribute('src') !== props.defaultImageUrl) { + imgElement.src = props.defaultImageUrl } } diff --git a/resources/js/components/grid-columns/TextGridColumn.vue b/resources/js/components/grid-columns/TextGridColumn.vue index 76de6cb..30337a3 100644 --- a/resources/js/components/grid-columns/TextGridColumn.vue +++ b/resources/js/components/grid-columns/TextGridColumn.vue @@ -64,36 +64,51 @@ const formattedValue = computed(() => { return '' } - // If it's an array, don't format it here - we'll render badges individually - if (isArray.value) { + // If it's an array for badges, don't format it here - we'll render badges individually + if (isArray.value && props.badge) { return '' } + // If it's an object or array (not for badges), format as JSON (same as TextColumn) + if (typeof props.value === 'object' && !props.badge) { + try { + return JSON.stringify(props.value, null, 2) + } catch (e) { + return String(props.value) + } + } + let result = String(props.value) // Format as date/datetime if (props.dateTimeFormat && props.value) { try { const date = new Date(props.value) - result = date.toLocaleString('en-US', { - year: 'numeric', - month: '2-digit', - day: '2-digit', - hour: '2-digit', - minute: '2-digit', - second: '2-digit', - }) + // Check if date is valid + if (!isNaN(date.getTime())) { + result = date.toLocaleString('en-US', { + year: 'numeric', + month: '2-digit', + day: '2-digit', + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + }) + } } catch (e) { result = String(props.value) } } else if (props.dateFormat && props.value) { try { const date = new Date(props.value) - result = date.toLocaleDateString('en-US', { - year: 'numeric', - month: '2-digit', - day: '2-digit', - }) + // Check if date is valid + if (!isNaN(date.getTime())) { + result = date.toLocaleDateString('en-US', { + year: 'numeric', + month: '2-digit', + day: '2-digit', + }) + } } catch (e) { result = String(props.value) } diff --git a/src/Columns/TextColumn.php b/src/Columns/TextColumn.php index 76fcf09..f7e131d 100644 --- a/src/Columns/TextColumn.php +++ b/src/Columns/TextColumn.php @@ -227,6 +227,11 @@ public function html(bool $condition = true): static return $this; } + public function isHtml(): bool + { + return $this->html; + } + public function separator(?string $separator = ','): static { $this->separator = $separator; diff --git a/src/Support/HtmlSanitizer.php b/src/Support/HtmlSanitizer.php new file mode 100644 index 0000000..260f083 --- /dev/null +++ b/src/Support/HtmlSanitizer.php @@ -0,0 +1,144 @@ + + */ + protected const REMOVED_ELEMENTS = [ + 'script', 'style', 'iframe', 'object', 'embed', 'applet', 'frame', 'frameset', + 'base', 'link', 'meta', 'noscript', 'template', 'form', + ]; + + /** + * Attributes that may carry a URL and must not use a script-capable scheme. + * + * @var array + */ + protected const URL_ATTRIBUTES = [ + 'href', 'src', 'srcset', 'action', 'formaction', 'xlink:href', 'poster', + 'background', 'data', 'cite', 'lowsrc', 'dynsrc', 'longdesc', 'ping', + ]; + + /** + * URL schemes that are never allowed. + * + * @var array + */ + protected const BLOCKED_SCHEMES = ['javascript', 'vbscript', 'data']; + + public static function sanitize(?string $html): string + { + if ($html === null || trim($html) === '') { + return (string) $html; + } + + $document = new DOMDocument('1.0', 'UTF-8'); + $previous = libxml_use_internal_errors(true); + + // The XML encoding hint makes libxml parse the fragment as UTF-8 instead of ISO-8859-1 + $document->loadHTML( + '
'.$html.'
', + LIBXML_HTML_NOIMPLIED | LIBXML_HTML_NODEFDTD | LIBXML_NONET + ); + + libxml_clear_errors(); + libxml_use_internal_errors($previous); + + $root = $document->getElementById('__laravilt_sanitizer_root'); + + if (! $root) { + // Could not parse: fall back to fully escaped text + return e($html); + } + + static::cleanNode($root); + + $output = ''; + foreach ($root->childNodes as $child) { + $output .= $document->saveHTML($child); + } + + return $output; + } + + protected static function cleanNode(DOMNode $node): void + { + // Iterate over a copy: removing nodes while iterating a live DOMNodeList skips siblings + foreach (iterator_to_array($node->childNodes) as $child) { + if ($child->nodeType === XML_COMMENT_NODE || $child->nodeType === XML_PI_NODE) { + $node->removeChild($child); + + continue; + } + + if (! $child instanceof DOMElement) { + continue; + } + + if (in_array(strtolower($child->nodeName), static::REMOVED_ELEMENTS, true)) { + $node->removeChild($child); + + continue; + } + + static::cleanAttributes($child); + static::cleanNode($child); + } + } + + protected static function cleanAttributes(DOMElement $element): void + { + foreach (iterator_to_array($element->attributes) as $attribute) { + $name = strtolower($attribute->nodeName); + $value = $attribute->nodeValue ?? ''; + + $remove = str_starts_with($name, 'on') + || (in_array($name, static::URL_ATTRIBUTES, true) && static::hasBlockedScheme($value)) + || ($name === 'style' && static::hasDangerousStyle($value)); + + if ($remove) { + $element->removeAttributeNode($attribute); + } + } + } + + protected static function hasBlockedScheme(string $value): bool + { + // Browsers ignore whitespace/control characters inside the scheme ("java\tscript:") + $normalized = strtolower(preg_replace('/[\x00-\x20\x7F]+/', '', html_entity_decode($value, ENT_QUOTES | ENT_HTML5, 'UTF-8')) ?? ''); + + foreach (static::BLOCKED_SCHEMES as $scheme) { + // srcset/ping can hold several URLs separated by commas or spaces + if (preg_match('/(^|[,\s])'.$scheme.':/', $normalized) === 1) { + return true; + } + } + + return false; + } + + protected static function hasDangerousStyle(string $value): bool + { + $normalized = strtolower(preg_replace('/[\x00-\x20\x7F\\\\]+/', '', $value) ?? ''); + + return str_contains($normalized, 'expression(') + || str_contains($normalized, 'javascript:') + || str_contains($normalized, 'vbscript:') + || str_contains($normalized, 'url(data:'); + } +} diff --git a/src/Table.php b/src/Table.php index 565cefa..5edb63e 100644 --- a/src/Table.php +++ b/src/Table.php @@ -1320,6 +1320,15 @@ protected function processRecords(array $records, ?Grouping\Group $activeGroup = $recordArray[$columnName] = $formattedValue; } } + + // HTML columns are rendered with v-html / dangerouslySetInnerHTML: sanitize server-side + // so both frontends receive safe markup (runs after formatUsing, which may build HTML) + if ($column instanceof Columns\TextColumn && $column->isHtml()) { + $htmlValue = $recordArray[$columnName] ?? $value; + if (is_string($htmlValue)) { + $recordArray[$columnName] = Support\HtmlSanitizer::sanitize($htmlValue); + } + } } // Evaluate card badge color if card has badge color callback diff --git a/tests/Unit/HtmlSanitizerTest.php b/tests/Unit/HtmlSanitizerTest.php new file mode 100644 index 0000000..ec37b89 --- /dev/null +++ b/tests/Unit/HtmlSanitizerTest.php @@ -0,0 +1,109 @@ +Hello world link

'; + + expect(HtmlSanitizer::sanitize($html))->toBe($html); +}); + +it('preserves utf-8 text', function () { + expect(HtmlSanitizer::sanitize('مرحبا café'))->toBe('مرحبا café'); +}); + +it('returns empty and null input unchanged', function () { + expect(HtmlSanitizer::sanitize(''))->toBe('') + ->and(HtmlSanitizer::sanitize(null))->toBe(''); +}); + +it('strips dangerous elements with their content', function (string $tag) { + $output = HtmlSanitizer::sanitize("

ok

<{$tag}>alert(1)"); + + expect($output)->toBe('

ok

') + ->and(strtolower($output))->not->toContain($tag); +})->with(['script', 'style', 'iframe', 'object', 'embed']); + +it('strips self-closing embed and nested scripts', function () { + $output = HtmlSanitizer::sanitize('
text
'); + + expect($output)->toBe('
text
'); +}); + +it('strips event handler attributes', function () { + $output = HtmlSanitizer::sanitize('b'); + + expect($output)->not->toContain('onerror') + ->and(strtolower($output))->not->toContain('onload') + ->and($output)->not->toContain('onclick') + ->and($output)->toContain('src="a.png"') + ->and($output)->toContain('b'); +}); + +it('strips javascript and data urls', function (string $html) { + $output = strtolower(HtmlSanitizer::sanitize($html)); + + expect($output)->not->toContain('javascript') + ->and($output)->not->toContain('data:') + ->and($output)->not->toContain('vbscript'); +})->with([ + 'x', + 'x', + "x", + 'x', + '', + 'x', + '
', + 'x', +]); + +it('keeps relative and mailto urls', function () { + $html = 'um'; + + expect(HtmlSanitizer::sanitize($html))->toBe($html); +}); + +it('strips dangerous inline styles but keeps safe ones', function () { + expect(HtmlSanitizer::sanitize('x'))->toBe('x') + ->and(HtmlSanitizer::sanitize('x'))->toBe('x'); +}); + +it('strips html comments', function () { + expect(HtmlSanitizer::sanitize('

a

'))->toBe('

a

'); +}); + +it('exposes the html flag on text columns', function () { + expect(TextColumn::make('bio')->isHtml())->toBeFalse() + ->and(TextColumn::make('bio')->html()->isHtml())->toBeTrue(); +}); + +it('sanitizes html text column values when processing records', function () { + $table = Table::make()->columns([ + TextColumn::make('bio')->html(), + TextColumn::make('name'), + ]); + + $method = new ReflectionMethod($table, 'processRecords'); + $records = $method->invoke($table, [[ + 'id' => 1, + 'bio' => '

Hi

', + 'name' => 'plain', + ]]); + + // HTML column is sanitized; non-HTML columns are left alone (the frontend escapes them) + expect($records[0]['bio'])->toBe('

Hi

') + ->and($records[0]['name'])->toBe('plain'); +}); + +it('sanitizes html produced by formatStateUsing', function () { + $table = Table::make()->columns([ + TextColumn::make('bio')->html()->formatStateUsing(fn ($state) => $state.''), + ]); + + $method = new ReflectionMethod($table, 'processRecords'); + $records = $method->invoke($table, [['id' => 1, 'bio' => 'Hi']]); + + expect($records[0]['bio'])->toBe('Hi'); +}); From daaae3b9f5066509ea36761d3b6c429d983d6d8a Mon Sep 17 00:00:00 2001 From: Fady Mondy Date: Tue, 15 Sep 2026 09:40:26 +0300 Subject: [PATCH 2/4] Allow base64 raster data URLs in img src in HtmlSanitizer Inline base64 png/jpeg/gif/webp/avif images in are kept; data: URLs stay blocked everywhere else (SVG, text/html, non-base64, srcset, href, other elements/attributes). Co-Authored-By: Claude Opus 5 (1M context) --- src/Support/HtmlSanitizer.php | 15 ++++++++++++++- tests/Unit/HtmlSanitizerTest.php | 21 +++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/Support/HtmlSanitizer.php b/src/Support/HtmlSanitizer.php index 260f083..b2328f0 100644 --- a/src/Support/HtmlSanitizer.php +++ b/src/Support/HtmlSanitizer.php @@ -108,7 +108,9 @@ protected static function cleanAttributes(DOMElement $element): void $value = $attribute->nodeValue ?? ''; $remove = str_starts_with($name, 'on') - || (in_array($name, static::URL_ATTRIBUTES, true) && static::hasBlockedScheme($value)) + || (in_array($name, static::URL_ATTRIBUTES, true) + && ! static::isAllowedImageDataUrl($element, $name, $value) + && static::hasBlockedScheme($value)) || ($name === 'style' && static::hasDangerousStyle($value)); if ($remove) { @@ -117,6 +119,17 @@ protected static function cleanAttributes(DOMElement $element): void } } + /** + * Inline base64 raster images are allowed in only: raster formats can't execute script. + * SVG (can embed script), other media types, and data: URLs on any other element/attribute stay blocked. + */ + protected static function isAllowedImageDataUrl(DOMElement $element, string $attribute, string $value): bool + { + return strtolower($element->nodeName) === 'img' + && $attribute === 'src' + && preg_match('#^\s*data:image/(png|jpe?g|gif|webp|avif);base64,[a-z0-9+/=\s]*$#i', $value) === 1; + } + protected static function hasBlockedScheme(string $value): bool { // Browsers ignore whitespace/control characters inside the scheme ("java\tscript:") diff --git a/tests/Unit/HtmlSanitizerTest.php b/tests/Unit/HtmlSanitizerTest.php index ec37b89..573ad89 100644 --- a/tests/Unit/HtmlSanitizerTest.php +++ b/tests/Unit/HtmlSanitizerTest.php @@ -59,6 +59,27 @@ 'x', ]); +it('allows base64 raster data urls in img src', function (string $mime) { + $html = 'x'; + + expect(HtmlSanitizer::sanitize($html))->toBe($html); +})->with(['png', 'jpeg', 'jpg', 'gif', 'webp', 'avif', 'PNG']); + +it('blocks data urls that are not base64 raster images in img src', function (string $html) { + expect(strtolower(HtmlSanitizer::sanitize($html)))->not->toContain('data:'); +})->with([ + 'svg' => '', + 'svg plain' => '', + 'text/html' => '', + 'non-base64 png' => '', + 'trailing payload' => '', + 'img srcset' => '', + 'a href' => 'x', + 'a href html' => 'x', + 'source src' => '', + 'iframe-like object data' => '
x
', +]); + it('keeps relative and mailto urls', function () { $html = 'um'; From f59e6d2666e09896c59dddfb718aee37671eca59 Mon Sep 17 00:00:00 2001 From: Fady Mondy Date: Tue, 15 Sep 2026 10:05:30 +0300 Subject: [PATCH 3/4] Strip SVG animation elements in HtmlSanitizer and resolve grid image fallback URL - HtmlSanitizer: remove animate, set, animateMotion and animateTransform, which can animate an href/xlink:href to a javascript: URL through values/to. - ImageGridColumn (Vue + React): resolve the default image through getImageUrl before falling back and compare against that normalized URL, so the one-shot loop guard holds and no document-relative URL is requested. Co-Authored-By: Claude Opus 5 (1M context) --- .../grid-columns/ImageGridColumn.vue | 9 +++++--- .../grid-columns/ImageGridColumn.tsx | 9 +++++--- src/Support/HtmlSanitizer.php | 2 ++ tests/Unit/HtmlSanitizerTest.php | 21 +++++++++++++++++++ 4 files changed, 35 insertions(+), 6 deletions(-) diff --git a/resources/js/components/grid-columns/ImageGridColumn.vue b/resources/js/components/grid-columns/ImageGridColumn.vue index cc6ed64..a7a96e5 100644 --- a/resources/js/components/grid-columns/ImageGridColumn.vue +++ b/resources/js/components/grid-columns/ImageGridColumn.vue @@ -146,9 +146,12 @@ const getImageUrl = (image: string): string => { const handleImageError = (event: Event) => { const imgElement = event.target as HTMLImageElement - // Only fall back once: if the default image itself fails, stop (prevents an error/reload loop) - if (props.defaultImageUrl && imgElement.getAttribute('src') !== props.defaultImageUrl) { - imgElement.src = props.defaultImageUrl + if (!props.defaultImageUrl) return + // Resolve the fallback the same way as the rendered src, then compare normalized URLs: + // only fall back once, so a failing default image can't cause an error/reload loop + const fallbackUrl = getImageUrl(props.defaultImageUrl) + if (imgElement.getAttribute('src') !== fallbackUrl) { + imgElement.src = fallbackUrl } } diff --git a/resources/react/components/grid-columns/ImageGridColumn.tsx b/resources/react/components/grid-columns/ImageGridColumn.tsx index 452d1b5..9eadb44 100644 --- a/resources/react/components/grid-columns/ImageGridColumn.tsx +++ b/resources/react/components/grid-columns/ImageGridColumn.tsx @@ -143,9 +143,12 @@ export default function ImageGridColumn({ const handleImageError = (event: SyntheticEvent) => { const imgElement = event.currentTarget; - // Only fall back once: if the default image itself fails, stop (prevents an error/reload loop) - if (defaultImageUrl && imgElement.getAttribute('src') !== defaultImageUrl) { - imgElement.src = defaultImageUrl; + if (!defaultImageUrl) return; + // Resolve the fallback the same way as the rendered src, then compare normalized URLs: + // only fall back once, so a failing default image can't cause an error/reload loop + const fallbackUrl = getImageUrl(defaultImageUrl); + if (imgElement.getAttribute('src') !== fallbackUrl) { + imgElement.src = fallbackUrl; } }; diff --git a/src/Support/HtmlSanitizer.php b/src/Support/HtmlSanitizer.php index b2328f0..2dc6f15 100644 --- a/src/Support/HtmlSanitizer.php +++ b/src/Support/HtmlSanitizer.php @@ -22,6 +22,8 @@ class HtmlSanitizer protected const REMOVED_ELEMENTS = [ 'script', 'style', 'iframe', 'object', 'embed', 'applet', 'frame', 'frameset', 'base', 'link', 'meta', 'noscript', 'template', 'form', + // SVG animation elements can animate href/xlink:href to a javascript: URL via values/to/from/by + 'animate', 'set', 'animatemotion', 'animatetransform', ]; /** diff --git a/tests/Unit/HtmlSanitizerTest.php b/tests/Unit/HtmlSanitizerTest.php index 573ad89..cb6375f 100644 --- a/tests/Unit/HtmlSanitizerTest.php +++ b/tests/Unit/HtmlSanitizerTest.php @@ -32,6 +32,27 @@ expect($output)->toBe('
text
'); }); +it('strips svg animation elements that can retarget href to javascript urls', function (string $html) { + $output = strtolower(HtmlSanitizer::sanitize($html)); + + expect($output)->not->toContain('javascript:') + ->and($output)->not->toContain('and($output)->not->toContain('and($output)->toContain('with([ + 'animate values' => ['click'], + 'animate to' => ['click'], + 'set to' => ['click'], + 'set xlink:href' => ['click'], +]); + +it('strips all four svg animation elements', function (string $tag) { + $output = HtmlSanitizer::sanitize("<{$tag} attributeName=\"r\" to=\"10\">"); + + expect(strtolower($output))->not->toContain(strtolower($tag)) + ->and($output)->toContain('with(['animate', 'set', 'animateMotion', 'animateTransform']); + it('strips event handler attributes', function () { $output = HtmlSanitizer::sanitize('b'); From 442c96537ee69693619358a301d6e6e5f6beac6b Mon Sep 17 00:00:00 2001 From: Fady Mondy Date: Tue, 15 Sep 2026 10:15:45 +0300 Subject: [PATCH 4/4] Add inline editable Select/TextInput/Checkbox columns with an authorized update endpoint SelectColumn, TextInputColumn and CheckboxColumn had no renderer in either stack and fell back to plain text. ToggleColumn saved through the panel's column route, which does not authorize, does not validate, and accepts any table column. - New PATCH {panel}/_tables/{resource}/{record}/column endpoint (Http\ColumnStateRoutes + UpdateColumnStateController), registered per panel with the panel's middleware stack. It rebuilds the table from the resource, accepts only EditableColumn columns (not disabled, not relationship paths), loads the record via getEloquentQuery() (tenant scoping), authorizes with Resource::canUpdate() (policy "update" when $usePolicies), validates with the column rules plus type rules (select options / boolean / string|numeric), and writes only that attribute. before/afterStateUpdated callbacks run for all editable columns. - Table serializes columnUpdateRoute; columns expose getRules() and getStateValidationRules(); SelectColumn now serializes editable. - Vue + React cell renderers (shadcn Select, Input, Checkbox, Switch) share a useColumnUpdate helper: optimistic update, revert + notification on failure, disabled while saving, aria-label/aria-busy. ToggleColumn uses the new endpoint when available and keeps the legacy route for relation manager tables. - Pest feature tests for the endpoint and serialization. Co-Authored-By: Claude Opus 5 (1M context) --- resources/js/components/DataTable.vue | 12 + resources/js/components/Table.vue | 1 + .../js/components/columns/CheckboxColumn.vue | 60 +++++ .../js/components/columns/SelectColumn.vue | 90 +++++++ .../js/components/columns/TextInputColumn.vue | 94 +++++++ .../js/components/columns/ToggleColumn.vue | 33 +++ resources/js/composables/useColumnUpdate.ts | 93 +++++++ resources/react/components/DataTable.tsx | 12 + resources/react/components/Table.tsx | 1 + .../components/columns/CheckboxColumn.tsx | 60 +++++ .../react/components/columns/SelectColumn.tsx | 89 +++++++ .../components/columns/TextInputColumn.tsx | 98 +++++++ .../react/components/columns/ToggleColumn.tsx | 36 ++- .../react/composables/useColumnUpdate.ts | 92 +++++++ src/Columns/CheckboxColumn.php | 18 +- src/Columns/Contracts/EditableColumn.php | 38 +++ src/Columns/SelectColumn.php | 24 +- src/Columns/TextInputColumn.php | 27 +- src/Columns/ToggleColumn.php | 18 +- src/Http/ColumnStateRoutes.php | 60 +++++ .../UpdateColumnStateController.php | 130 ++++++++++ src/Table.php | 23 ++ src/TablesServiceProvider.php | 5 + tests/Feature/UpdateColumnStateTest.php | 241 ++++++++++++++++++ 24 files changed, 1345 insertions(+), 10 deletions(-) create mode 100644 resources/js/components/columns/CheckboxColumn.vue create mode 100644 resources/js/components/columns/SelectColumn.vue create mode 100644 resources/js/components/columns/TextInputColumn.vue create mode 100644 resources/js/composables/useColumnUpdate.ts create mode 100644 resources/react/components/columns/CheckboxColumn.tsx create mode 100644 resources/react/components/columns/SelectColumn.tsx create mode 100644 resources/react/components/columns/TextInputColumn.tsx create mode 100644 resources/react/composables/useColumnUpdate.ts create mode 100644 src/Columns/Contracts/EditableColumn.php create mode 100644 src/Http/ColumnStateRoutes.php create mode 100644 src/Http/Controllers/UpdateColumnStateController.php create mode 100644 tests/Feature/UpdateColumnStateTest.php diff --git a/resources/js/components/DataTable.vue b/resources/js/components/DataTable.vue index 5fed450..e097899 100644 --- a/resources/js/components/DataTable.vue +++ b/resources/js/components/DataTable.vue @@ -6,6 +6,9 @@ import IconColumn from './columns/IconColumn.vue' import ImageColumn from './columns/ImageColumn.vue' import ColorColumn from './columns/ColorColumn.vue' import ToggleColumn from './columns/ToggleColumn.vue' +import SelectColumn from './columns/SelectColumn.vue' +import TextInputColumn from './columns/TextInputColumn.vue' +import CheckboxColumn from './columns/CheckboxColumn.vue' import RecordActions from '@laravilt/actions/components/RecordActions.vue' import { Skeleton } from '@/components/ui/skeleton' import { Checkbox } from '@/components/ui/checkbox' @@ -64,6 +67,7 @@ interface DataTableProps { bulkActionsAvailable?: boolean resourceSlug?: string columnExecutionRoute?: string + columnUpdateRoute?: string | null modelClass?: string recordActions?: Action[] executionRoute?: string @@ -90,6 +94,7 @@ const props = withDefaults(defineProps(), { bulkActionsAvailable: false, resourceSlug: '', columnExecutionRoute: undefined, + columnUpdateRoute: null, modelClass: undefined, recordActions: () => [], executionRoute: undefined, @@ -394,6 +399,12 @@ const getColumnComponent = (columnType: string) => { return ColorColumn case 'ToggleColumn': return ToggleColumn + case 'SelectColumn': + return SelectColumn + case 'TextInputColumn': + return TextInputColumn + case 'CheckboxColumn': + return CheckboxColumn default: return TextColumn } @@ -650,6 +661,7 @@ const getColumnWidthClass = (column: Column, index: number): string => { :record-id="record.id" :resource-slug="resourceSlug" :column-execution-route="columnExecutionRoute" + :column-update-route="columnUpdateRoute" v-bind="column" :default-image-url="record._defaultImageUrls?.[column.name] ?? column.defaultImageUrl" /> diff --git a/resources/js/components/Table.vue b/resources/js/components/Table.vue index f14ece0..8f60043 100644 --- a/resources/js/components/Table.vue +++ b/resources/js/components/Table.vue @@ -945,6 +945,7 @@ onUnmounted(() => { :bulk-actions-available="extractedBulkActions.length > 0" :resource-slug="resourceSlug" :column-execution-route="relationContext?.columnExecutionRoute || table.columnExecutionRoute" + :column-update-route="relationContext ? null : table.columnUpdateRoute" :model-class="table.model" :clear-selections="clearSelectionsKey" :fixed-actions="table.fixedActions" diff --git a/resources/js/components/columns/CheckboxColumn.vue b/resources/js/components/columns/CheckboxColumn.vue new file mode 100644 index 0000000..72ad0a9 --- /dev/null +++ b/resources/js/components/columns/CheckboxColumn.vue @@ -0,0 +1,60 @@ + + + diff --git a/resources/js/components/columns/SelectColumn.vue b/resources/js/components/columns/SelectColumn.vue new file mode 100644 index 0000000..49f9a5a --- /dev/null +++ b/resources/js/components/columns/SelectColumn.vue @@ -0,0 +1,90 @@ + + + diff --git a/resources/js/components/columns/TextInputColumn.vue b/resources/js/components/columns/TextInputColumn.vue new file mode 100644 index 0000000..350b071 --- /dev/null +++ b/resources/js/components/columns/TextInputColumn.vue @@ -0,0 +1,94 @@ + + + diff --git a/resources/js/components/columns/ToggleColumn.vue b/resources/js/components/columns/ToggleColumn.vue index a08ea93..d4a949b 100644 --- a/resources/js/components/columns/ToggleColumn.vue +++ b/resources/js/components/columns/ToggleColumn.vue @@ -4,6 +4,7 @@ import { router } from '@inertiajs/vue3' import { Switch } from '@/components/ui/switch' import { useNotification } from '@laravilt/notifications/composables/useNotification' import { useLocalization } from '@/composables/useLocalization' +import { useColumnUpdate } from '../../composables/useColumnUpdate' // Initialize localization const { trans } = useLocalization() @@ -11,8 +12,12 @@ const { trans } = useLocalization() interface ToggleColumnProps { value: any name: string + label?: string | null recordId: number | string resourceSlug?: string + /** Authorized, validated column update endpoint (preferred when present) */ + columnUpdateRoute?: string | null + /** Legacy panel endpoint, still used by relation manager tables */ columnExecutionRoute?: string editable?: boolean disabled?: boolean @@ -30,7 +35,9 @@ const props = withDefaults(defineProps(), { disabled: false, description: null, descriptionPosition: 'below', + label: null, resourceSlug: '', + columnUpdateRoute: null, columnExecutionRoute: undefined, successNotificationTitle: 'Updated', successNotificationMessage: 'Value updated successfully', @@ -40,6 +47,20 @@ const props = withDefaults(defineProps(), { const { notify } = useNotification() +// Optimistic save through the column update endpoint (reverts on failure) +const update = useColumnUpdate( + () => Boolean(props.value), + () => ({ + name: props.name, + recordId: props.recordId, + columnUpdateRoute: props.columnUpdateRoute, + editable: props.editable, + disabled: props.disabled, + }), +) +const { localValue: updateValue, isSaving: updateIsSaving, isDisabled: updateIsDisabled } = update +const usesUpdateRoute = computed(() => Boolean(props.columnUpdateRoute)) + // Compute the execution URL - replace __ID__ placeholder with actual record ID const executionUrl = computed(() => { if (props.columnExecutionRoute) { @@ -114,6 +135,18 @@ const isChecked = computed({
+ ({})) + + if (!response.ok) { + throw new Error(data?.errors?.value?.[0] || data?.message || trans('tables::tables.toggle_column.error_notification_message')) + } + + if (data && 'state' in data) localValue.value = data.state as T + + notify( + trans('tables::tables.toggle_column.success_notification_title'), + trans('tables::tables.toggle_column.success_notification_message'), + 'success', + { duration: 2000 }, + ) + + return true + } catch (error) { + localValue.value = previous + notify( + trans('tables::tables.toggle_column.error_notification_title'), + error instanceof Error && error.message ? error.message : trans('tables::tables.toggle_column.error_notification_message'), + 'error', + { duration: 3000 }, + ) + + return false + } finally { + isSaving.value = false + } + } + + return { localValue, isSaving, isDisabled, url, save } +} diff --git a/resources/react/components/DataTable.tsx b/resources/react/components/DataTable.tsx index 6235ee7..f5f538b 100644 --- a/resources/react/components/DataTable.tsx +++ b/resources/react/components/DataTable.tsx @@ -11,6 +11,9 @@ import ColorColumn from './columns/ColorColumn'; import IconColumn from './columns/IconColumn'; import ImageColumn from './columns/ImageColumn'; import TextColumn from './columns/TextColumn'; +import CheckboxColumn from './columns/CheckboxColumn'; +import SelectColumn from './columns/SelectColumn'; +import TextInputColumn from './columns/TextInputColumn'; import ToggleColumn from './columns/ToggleColumn'; import './DataTable.css'; @@ -63,6 +66,7 @@ export interface DataTableProps { bulkActionsAvailable?: boolean; resourceSlug?: string; columnExecutionRoute?: string; + columnUpdateRoute?: string | null; modelClass?: string; recordActions?: Action[]; executionRoute?: string; @@ -128,6 +132,7 @@ export default function DataTable({ bulkActionsAvailable = false, resourceSlug = '', columnExecutionRoute, + columnUpdateRoute = null, modelClass, executionRoute, clearSelections = 0, @@ -405,6 +410,12 @@ export default function DataTable({ return ColorColumn; case 'ToggleColumn': return ToggleColumn; + case 'SelectColumn': + return SelectColumn; + case 'TextInputColumn': + return TextInputColumn; + case 'CheckboxColumn': + return CheckboxColumn; default: return TextColumn; } @@ -662,6 +673,7 @@ export default function DataTable({ recordId={record.id} resourceSlug={resourceSlug} columnExecutionRoute={columnExecutionRoute} + columnUpdateRoute={columnUpdateRoute} {...column} defaultImageUrl={record._defaultImageUrls?.[column.name] ?? column.defaultImageUrl} /> diff --git a/resources/react/components/Table.tsx b/resources/react/components/Table.tsx index 5410bc3..06d9236 100644 --- a/resources/react/components/Table.tsx +++ b/resources/react/components/Table.tsx @@ -886,6 +886,7 @@ export default function Table({ bulkActionsAvailable={extractedBulkActions.length > 0} resourceSlug={resourceSlug} columnExecutionRoute={relationContext?.columnExecutionRoute || table.columnExecutionRoute} + columnUpdateRoute={relationContext ? null : table.columnUpdateRoute} modelClass={table.model} clearSelections={clearSelectionsKey} fixedActions={table.fixedActions} diff --git a/resources/react/components/columns/CheckboxColumn.tsx b/resources/react/components/columns/CheckboxColumn.tsx new file mode 100644 index 0000000..c6ef265 --- /dev/null +++ b/resources/react/components/columns/CheckboxColumn.tsx @@ -0,0 +1,60 @@ +import { Checkbox } from '@/components/ui/checkbox'; +import { cn } from '@/lib/utils'; +import type { MouseEvent } from 'react'; +import { useColumnUpdate } from '../../composables/useColumnUpdate'; + +export interface CheckboxColumnProps { + value: any; + name: string; + label?: string | null; + recordId: number | string; + columnUpdateRoute?: string | null; + editable?: boolean; + disabled?: boolean; + description?: string | null; + descriptionPosition?: 'above' | 'below'; +} + +export default function CheckboxColumn({ + value, + name, + label = null, + recordId, + columnUpdateRoute = null, + editable = true, + disabled = false, + description = null, + descriptionPosition = 'below', +}: CheckboxColumnProps) { + const { localValue, isSaving, isDisabled, save } = useColumnUpdate(Boolean(value), { + name, + recordId, + columnUpdateRoute, + editable, + disabled, + }); + + return ( + // Editing a cell must not trigger the row's click-to-open behaviour +
event.stopPropagation()}> + {description && descriptionPosition === 'above' && ( +
{description}
+ )} + +
+ void save(checked === true)} + /> +
+ + {description && descriptionPosition === 'below' && ( +
{description}
+ )} +
+ ); +} diff --git a/resources/react/components/columns/SelectColumn.tsx b/resources/react/components/columns/SelectColumn.tsx new file mode 100644 index 0000000..4d753d0 --- /dev/null +++ b/resources/react/components/columns/SelectColumn.tsx @@ -0,0 +1,89 @@ +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select'; +import { cn } from '@/lib/utils'; +import { Loader2 } from 'lucide-react'; +import type { MouseEvent } from 'react'; +import { useColumnUpdate } from '../../composables/useColumnUpdate'; + +export interface SelectColumnProps { + value: any; + name: string; + label?: string | null; + placeholder?: string | null; + recordId: number | string; + columnUpdateRoute?: string | null; + editable?: boolean; + disabled?: boolean; + options?: Record | string[]; + selectablePlaceholder?: boolean; + description?: string | null; + descriptionPosition?: 'above' | 'below'; +} + +// Radix/Reka select items can't use an empty value, so "no value" gets a sentinel item +const NULL_VALUE = '__laravilt_null__'; + +const toKey = (value: any): string | null => (value === null || value === undefined || value === '' ? null : String(value)); + +export default function SelectColumn({ + value, + name, + label = null, + placeholder = null, + recordId, + columnUpdateRoute = null, + editable = true, + disabled = false, + options, + selectablePlaceholder = true, + description = null, + descriptionPosition = 'below', +}: SelectColumnProps) { + const { localValue, isSaving, isDisabled, save } = useColumnUpdate(toKey(value), { + name, + recordId, + columnUpdateRoute, + editable, + disabled, + }); + + const entries = Object.entries(options ?? {}).map(([key, optionLabel]) => ({ key, label: String(optionLabel) })); + + const onValueChange = (next: string) => { + const newValue = next === NULL_VALUE ? null : next; + if (newValue !== localValue) void save(newValue); + }; + + return ( + // Editing a cell must not trigger the row's click-to-open behaviour +
event.stopPropagation()}> + {description && descriptionPosition === 'above' && ( +
{description}
+ )} + +
+ + {isSaving &&
+ + {description && descriptionPosition === 'below' && ( +
{description}
+ )} +
+ ); +} diff --git a/resources/react/components/columns/TextInputColumn.tsx b/resources/react/components/columns/TextInputColumn.tsx new file mode 100644 index 0000000..8264c43 --- /dev/null +++ b/resources/react/components/columns/TextInputColumn.tsx @@ -0,0 +1,98 @@ +import { Input } from '@/components/ui/input'; +import { cn } from '@/lib/utils'; +import { Loader2 } from 'lucide-react'; +import { useEffect, useState, type KeyboardEvent, type MouseEvent } from 'react'; +import { useColumnUpdate } from '../../composables/useColumnUpdate'; + +export interface TextInputColumnProps { + value: any; + name: string; + label?: string | null; + placeholder?: string | null; + recordId: number | string; + columnUpdateRoute?: string | null; + editable?: boolean; + disabled?: boolean; + type?: string; + prefix?: string | null; + suffix?: string | null; + description?: string | null; + descriptionPosition?: 'above' | 'below'; +} + +const toText = (value: any): string => (value === null || value === undefined ? '' : String(value)); + +export default function TextInputColumn({ + value, + name, + label = null, + placeholder = null, + recordId, + columnUpdateRoute = null, + editable = true, + disabled = false, + type = 'text', + prefix = null, + suffix = null, + description = null, + descriptionPosition = 'below', +}: TextInputColumnProps) { + const { localValue, isSaving, isDisabled, save } = useColumnUpdate(value ?? null, { + name, + recordId, + columnUpdateRoute, + editable, + disabled, + }); + + // The draft is what the user is typing; it's saved on Enter/blur and resynced when the stored value changes + const [draft, setDraft] = useState(() => toText(value)); + useEffect(() => { + setDraft(toText(localValue)); + }, [localValue]); + + const commit = () => { + if (draft === toText(localValue)) return; + void save(draft === '' ? null : type === 'number' ? Number(draft) : draft); + }; + + const onKeyDown = (event: KeyboardEvent) => { + if (event.key === 'Enter') { + event.preventDefault(); + commit(); + } else if (event.key === 'Escape') { + setDraft(toText(localValue)); + } + }; + + return ( + // Editing a cell must not trigger the row's click-to-open behaviour +
event.stopPropagation()}> + {description && descriptionPosition === 'above' && ( +
{description}
+ )} + +
+ {prefix && {prefix}} + setDraft(event.target.value)} + onBlur={commit} + onKeyDown={onKeyDown} + /> + {suffix && {suffix}} + {isSaving &&
+ + {description && descriptionPosition === 'below' && ( +
{description}
+ )} +
+ ); +} diff --git a/resources/react/components/columns/ToggleColumn.tsx b/resources/react/components/columns/ToggleColumn.tsx index 84a76b2..c968f62 100644 --- a/resources/react/components/columns/ToggleColumn.tsx +++ b/resources/react/components/columns/ToggleColumn.tsx @@ -4,13 +4,18 @@ import { router } from '@inertiajs/react'; import { useNotification } from '@laravilt/notifications/composables/useNotification'; import { useLocalization } from '@laravilt/support/composables/useLocalization'; import { useEffect, useState } from 'react'; +import { useColumnUpdate } from '../../composables/useColumnUpdate'; import { useStateRef } from '../../composables/useStateRef'; export interface ToggleColumnProps { value: any; name: string; + label?: string | null; recordId: number | string; resourceSlug?: string; + /** Authorized, validated column update endpoint (preferred when present) */ + columnUpdateRoute?: string | null; + /** Legacy panel endpoint, still used by relation manager tables */ columnExecutionRoute?: string; editable?: boolean; disabled?: boolean; @@ -26,7 +31,9 @@ export interface ToggleColumnProps { export default function ToggleColumn({ value, name, + label = null, recordId, + columnUpdateRoute = null, columnExecutionRoute, editable = true, disabled = false, @@ -40,6 +47,10 @@ export default function ToggleColumn({ const { trans } = useLocalization(); const { notify } = useNotification(); + // Optimistic save through the column update endpoint (reverts on failure) + const update = useColumnUpdate(Boolean(value), { name, recordId, columnUpdateRoute, editable, disabled }); + const usesUpdateRoute = Boolean(columnUpdateRoute); + // Compute the execution URL - replace __ID__ placeholder with actual record ID const executionUrl = columnExecutionRoute ? columnExecutionRoute.replace('__ID__', String(recordId)) : null; @@ -112,12 +123,25 @@ export default function ToggleColumn({ {/* Main content */}
- setChecked(checked)} - /> + {usesUpdateRoute ? ( + void update.save(checked)} + /> + ) : ( + setChecked(checked)} + /> + )}
{/* Description below */} diff --git a/resources/react/composables/useColumnUpdate.ts b/resources/react/composables/useColumnUpdate.ts new file mode 100644 index 0000000..db40788 --- /dev/null +++ b/resources/react/composables/useColumnUpdate.ts @@ -0,0 +1,92 @@ +import { useNotification } from '@laravilt/notifications/composables/useNotification'; +import { useLocalization } from '@laravilt/support/composables/useLocalization'; +import { useCallback, useEffect } from 'react'; +import { useStateRef } from './useStateRef'; + +export interface ColumnUpdateOptions { + name: string; + recordId: number | string; + columnUpdateRoute?: string | null; + editable?: boolean; + disabled?: boolean; +} + +/** + * Shared inline-edit state for editable table columns (Select/TextInput/Checkbox/Toggle). + * + * Optimistic: the local value changes immediately, is sent to the column update endpoint, and is + * reverted (with an error notification) if the server rejects it. Keep in sync with the Vue composable. + */ +export function useColumnUpdate(value: T, { name, recordId, columnUpdateRoute, editable = true, disabled = false }: ColumnUpdateOptions) { + const { notify } = useNotification(); + const { trans } = useLocalization(); + + const [localValue, setLocalValue, localValueRef] = useStateRef(value); + const [isSaving, setIsSaving, isSavingRef] = useStateRef(false); + + // Follow server-side changes, but never clobber an in-flight optimistic value + useEffect(() => { + if (!isSavingRef.current) { + setLocalValue(value); + } + }, [value, isSavingRef, setLocalValue]); + + const url = columnUpdateRoute ? columnUpdateRoute.replace('__ID__', encodeURIComponent(String(recordId))) : null; + const isDisabled = disabled || !editable || !url || isSaving; + + const save = useCallback( + async (newValue: T): Promise => { + if (disabled || !editable || !url || isSavingRef.current) return false; + + const previous = localValueRef.current; + setLocalValue(newValue); + setIsSaving(true); + + try { + const response = await fetch(url, { + method: 'PATCH', + credentials: 'same-origin', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json', + 'X-CSRF-TOKEN': document.querySelector('meta[name="csrf-token"]')?.content || '', + 'X-Requested-With': 'XMLHttpRequest', + }, + body: JSON.stringify({ column: name, value: newValue }), + }); + + const data = await response.json().catch(() => ({})); + + if (!response.ok) { + throw new Error(data?.errors?.value?.[0] || data?.message || trans('tables::tables.toggle_column.error_notification_message')); + } + + if (data && 'state' in data) setLocalValue(data.state as T); + + notify( + trans('tables::tables.toggle_column.success_notification_title'), + trans('tables::tables.toggle_column.success_notification_message'), + 'success', + { duration: 2000 }, + ); + + return true; + } catch (error) { + setLocalValue(previous); + notify( + trans('tables::tables.toggle_column.error_notification_title'), + error instanceof Error && error.message ? error.message : trans('tables::tables.toggle_column.error_notification_message'), + 'error', + { duration: 3000 }, + ); + + return false; + } finally { + setIsSaving(false); + } + }, + [disabled, editable, url, name, isSavingRef, localValueRef, setLocalValue, setIsSaving, notify, trans], + ); + + return { localValue, isSaving, isDisabled, url, save }; +} diff --git a/src/Columns/CheckboxColumn.php b/src/Columns/CheckboxColumn.php index f7135a2..85fbca1 100644 --- a/src/Columns/CheckboxColumn.php +++ b/src/Columns/CheckboxColumn.php @@ -3,8 +3,9 @@ namespace Laravilt\Tables\Columns; use Closure; +use Laravilt\Tables\Columns\Contracts\EditableColumn; -class CheckboxColumn extends Column +class CheckboxColumn extends Column implements EditableColumn { protected ?Closure $beforeStateUpdated = null; @@ -33,6 +34,21 @@ public function rules(array $rules): static return $this; } + public function getRules(): array + { + return $this->rules; + } + + public function getStateValidationRules(): array + { + return ['required', 'boolean', ...$this->rules]; + } + + public function dehydrateState(mixed $state): mixed + { + return filter_var($state, FILTER_VALIDATE_BOOLEAN); + } + public function getBeforeStateUpdated(): ?Closure { return $this->beforeStateUpdated; diff --git a/src/Columns/Contracts/EditableColumn.php b/src/Columns/Contracts/EditableColumn.php new file mode 100644 index 0000000..bd1b0ab --- /dev/null +++ b/src/Columns/Contracts/EditableColumn.php @@ -0,0 +1,38 @@ + + */ + public function getRules(): array; + + /** + * The full rule set used to validate an incoming state value: type rules plus getRules(). + * + * @return array + */ + public function getStateValidationRules(): array; + + /** + * Convert a validated incoming value to what is stored on the model. + */ + public function dehydrateState(mixed $state): mixed; + + public function getBeforeStateUpdated(): ?Closure; + + public function getAfterStateUpdated(): ?Closure; +} diff --git a/src/Columns/SelectColumn.php b/src/Columns/SelectColumn.php index 3269a9b..eae050d 100644 --- a/src/Columns/SelectColumn.php +++ b/src/Columns/SelectColumn.php @@ -3,8 +3,10 @@ namespace Laravilt\Tables\Columns; use Closure; +use Illuminate\Validation\Rule; +use Laravilt\Tables\Columns\Contracts\EditableColumn; -class SelectColumn extends Column +class SelectColumn extends Column implements EditableColumn { protected array|Closure $options = []; @@ -87,6 +89,25 @@ public function getOptions(): array return $this->options; } + public function getRules(): array + { + return $this->rules; + } + + public function getStateValidationRules(): array + { + return [ + $this->selectablePlaceholder ? 'nullable' : 'required', + Rule::in(array_map('strval', array_keys($this->getOptions()))), + ...$this->rules, + ]; + } + + public function dehydrateState(mixed $state): mixed + { + return $state === '' ? null : $state; + } + public function getBeforeStateUpdated(): ?Closure { return $this->beforeStateUpdated; @@ -111,6 +132,7 @@ protected function getVueProps(): array 'native' => $this->native, 'optionsSearchable' => $this->optionsSearchable, 'selectablePlaceholder' => $this->selectablePlaceholder, + 'editable' => true, ]; } diff --git a/src/Columns/TextInputColumn.php b/src/Columns/TextInputColumn.php index 65c170d..b43f52c 100644 --- a/src/Columns/TextInputColumn.php +++ b/src/Columns/TextInputColumn.php @@ -3,8 +3,9 @@ namespace Laravilt\Tables\Columns; use Closure; +use Laravilt\Tables\Columns\Contracts\EditableColumn; -class TextInputColumn extends Column +class TextInputColumn extends Column implements EditableColumn { protected ?Closure $beforeStateUpdated = null; @@ -96,6 +97,30 @@ public function inputSuffixIconColor(string|Closure $color): static return $this; } + public function getRules(): array + { + return $this->rules; + } + + public function getType(): string + { + return $this->type; + } + + public function getStateValidationRules(): array + { + return [ + 'nullable', + $this->type === 'number' ? 'numeric' : 'string', + ...$this->rules, + ]; + } + + public function dehydrateState(mixed $state): mixed + { + return $state === '' ? null : $state; + } + public function getBeforeStateUpdated(): ?Closure { return $this->beforeStateUpdated; diff --git a/src/Columns/ToggleColumn.php b/src/Columns/ToggleColumn.php index f958f15..044bdbd 100644 --- a/src/Columns/ToggleColumn.php +++ b/src/Columns/ToggleColumn.php @@ -3,8 +3,9 @@ namespace Laravilt\Tables\Columns; use Closure; +use Laravilt\Tables\Columns\Contracts\EditableColumn; -class ToggleColumn extends Column +class ToggleColumn extends Column implements EditableColumn { protected ?Closure $beforeStateUpdated = null; @@ -33,6 +34,21 @@ public function rules(array $rules): static return $this; } + public function getRules(): array + { + return $this->rules; + } + + public function getStateValidationRules(): array + { + return ['required', 'boolean', ...$this->rules]; + } + + public function dehydrateState(mixed $state): mixed + { + return filter_var($state, FILTER_VALIDATE_BOOLEAN); + } + public function getBeforeStateUpdated(): ?Closure { return $this->beforeStateUpdated; diff --git a/src/Http/ColumnStateRoutes.php b/src/Http/ColumnStateRoutes.php new file mode 100644 index 0000000..a12ba91 --- /dev/null +++ b/src/Http/ColumnStateRoutes.php @@ -0,0 +1,60 @@ +all() as $panel) { + Route::middleware(static::middlewareFor($panel)) + ->prefix($panel->getPath()) + ->name($panel->getId().'.') + ->group(function () use ($panel) { + Route::patch('_tables/{resource}/{record}/column', UpdateColumnStateController::class) + ->defaults('laraviltPanel', $panel->getId()) + ->name(static::ROUTE_NAME); + }); + } + } + + /** + * Mirrors the panel's path-based route middleware: session + panel identification + auth + tenant scoping. + * + * @return array + */ + public static function middlewareFor(Panel $panel): array + { + $toPanelAuth = fn ($middleware) => $middleware === 'auth' ? 'panel.auth' : $middleware; + + $middleware = array_filter( + array_map($toPanelAuth, $panel->getMiddleware()), + fn ($middleware) => $middleware !== 'panel.auth', + ); + + return array_values(array_unique(array_merge( + $middleware, + [IdentifyPanel::class.':'.$panel->getId()], + array_map($toPanelAuth, $panel->getAuthMiddleware()), + [HandleLocalization::class, IdentifyTenant::class], + ), SORT_REGULAR)); + } +} diff --git a/src/Http/Controllers/UpdateColumnStateController.php b/src/Http/Controllers/UpdateColumnStateController.php new file mode 100644 index 0000000..245f078 --- /dev/null +++ b/src/Http/Controllers/UpdateColumnStateController.php @@ -0,0 +1,130 @@ +resolveResource($request, $resource); + + $column = $this->resolveColumn($resourceClass, (string) $request->input('column')); + + $model = $this->resolveRecord($resourceClass, $record); + + $this->authorize($resourceClass, $model); + + $name = $column->getName(); + + $validated = Validator::make( + ['value' => $request->input('value')], + ['value' => $column->getStateValidationRules()], + [], + ['value' => method_exists($column, 'getLabel') && $column->getLabel() ? $column->getLabel() : $name], + )->validate(); + + $value = $column->dehydrateState($validated['value'] ?? null); + + if ($before = $column->getBeforeStateUpdated()) { + $before($model, $name, $value); + } + + // Only this column's attribute is written, regardless of what else the request contains + $model->setAttribute($name, $value); + $model->save(); + + if ($after = $column->getAfterStateUpdated()) { + $after($model, $name, $value); + } + + return response()->json([ + 'column' => $name, + 'state' => $model->getAttribute($name), + ]); + } + + /** + * @return class-string + */ + protected function resolveResource(Request $request, string $slug): string + { + $registry = app(PanelRegistry::class); + $panelId = $request->route('laraviltPanel'); + $panel = $panelId ? $registry->get($panelId) : $registry->getCurrent(); + + foreach ($panel?->getResources() ?? [] as $resourceClass) { + if ($resourceClass::getSlug() === $slug) { + return $resourceClass; + } + } + + abort(404); + } + + protected function resolveColumn(string $resourceClass, string $name): EditableColumn + { + abort_if($name === '', 422, 'The column field is required.'); + + $table = $resourceClass::table(new Table); + + foreach ($table->getColumns() as $column) { + if ($column->getName() !== $name) { + continue; + } + + // Plain display columns (TextColumn, ...) can never be written through this endpoint + abort_unless($column instanceof EditableColumn, 403, 'This column is not editable.'); + // Relationship paths ("author.name") are display-only: only the record's own attribute is updated + abort_if(str_contains($name, '.'), 403, 'Relationship columns cannot be edited inline.'); + abort_if($column->isDisabled(), 403, 'This column is disabled.'); + + return $column; + } + + abort(404, 'Column not found.'); + } + + protected function resolveRecord(string $resourceClass, string $key): Model + { + // getEloquentQuery() applies the resource's tenant scoping + $query = method_exists($resourceClass, 'getEloquentQuery') + ? $resourceClass::getEloquentQuery() + : $resourceClass::getModel()::query(); + + return $query->whereKey($key)->firstOrFail(); + } + + protected function authorize(string $resourceClass, Model $model): void + { + // Resource authorization honours $usePolicies (policy "update") or the panel's permission system + if (method_exists($resourceClass, 'canUpdate')) { + abort_unless($resourceClass::canUpdate($model), 403); + + return; + } + + if (Gate::getPolicyFor($model)) { + Gate::authorize('update', $model); + + return; + } + + abort_unless(auth()->check(), 403); + } +} diff --git a/src/Table.php b/src/Table.php index 5edb63e..9e8bd73 100644 --- a/src/Table.php +++ b/src/Table.php @@ -5,6 +5,7 @@ use Closure; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasOne; +use Illuminate\Support\Facades\Route; use Laravilt\Panel\PanelRegistry; use Laravilt\Support\Contracts\InertiaSerializable; use Laravilt\Tables\Columns\Column; @@ -690,6 +691,27 @@ public function getColumnExecutionRouteName(): string return $panelId.'.resources.'.$this->resourceSlug.'.column.update'; } + /** + * URL of the authorized inline column update endpoint (see Http\ColumnStateRoutes), with an + * `__ID__` placeholder for the record key. Null when the table isn't bound to a panel resource. + */ + public function getColumnUpdateRoute(): ?string + { + if (! $this->resourceSlug) { + return null; + } + + $registry = app(PanelRegistry::class); + $panel = $registry->getCurrent() ?? $registry->getDefault(); + $name = ($panel?->getId() ?? 'admin').'.'.Http\ColumnStateRoutes::ROUTE_NAME; + + if (! Route::has($name)) { + return null; + } + + return route($name, ['resource' => $this->resourceSlug, 'record' => '__ID__']); + } + /** * Get the reorder route name. */ @@ -1470,6 +1492,7 @@ function (Filter $filter) { 'queryRoute' => $this->queryRoute ?? request()->url(), 'resourceSlug' => $this->resourceSlug ?? '', 'columnExecutionRoute' => $this->resourceSlug ? route($this->getColumnExecutionRouteName(), ['id' => '__ID__']) : null, + 'columnUpdateRoute' => $this->getColumnUpdateRoute(), 'model' => $this->model, // API-specific properties 'apiEnabled' => $this->apiEnabled, diff --git a/src/TablesServiceProvider.php b/src/TablesServiceProvider.php index 271fdcc..8b7b5c6 100644 --- a/src/TablesServiceProvider.php +++ b/src/TablesServiceProvider.php @@ -32,6 +32,11 @@ public function boot(): void // Load web routes $this->loadRoutesFrom(__DIR__.'/../routes/web.php'); + // Inline column update endpoint: registered per panel once every panel has been registered + if (! $this->app->routesAreCached()) { + $this->app->booted(fn () => Http\ColumnStateRoutes::register()); + } + if ($this->app->runningInConsole()) { // Publish config $this->publishes([ diff --git a/tests/Feature/UpdateColumnStateTest.php b/tests/Feature/UpdateColumnStateTest.php new file mode 100644 index 0000000..1926bc4 --- /dev/null +++ b/tests/Feature/UpdateColumnStateTest.php @@ -0,0 +1,241 @@ + 'boolean', 'is_pinned' => 'boolean', 'locked' => 'boolean']; +} + +class ColumnStateUser extends AuthUser +{ + protected $table = 'column_state_users'; + + protected $guarded = []; +} + +class ColumnStateTaskResource extends Resource +{ + protected static string $model = ColumnStateTask::class; + + protected static ?string $slug = 'tasks'; + + /** @var array */ + public static array $afterUpdates = []; + + public static function table(Table $table): Table + { + return $table->columns([ + TextColumn::make('title'), + TextInputColumn::make('notes')->rules(['max:10']), + SelectColumn::make('status') + ->options(['draft' => 'Draft', 'published' => 'Published']) + ->selectablePlaceholder(false), + ToggleColumn::make('is_done') + ->afterStateUpdated(function ($record, $column, $value) { + static::$afterUpdates[] = [$column, $value]; + }), + CheckboxColumn::make('is_pinned'), + ToggleColumn::make('locked')->disabled(), + TextInputColumn::make('author.name'), + ]); + } +} + +class ColumnStatePolicyTaskResource extends ColumnStateTaskResource +{ + protected static ?string $slug = 'policy-tasks'; + + protected static bool $usePolicies = true; +} + +class ColumnStateTaskPolicy +{ + public function update(ColumnStateUser $user, ColumnStateTask $task): bool + { + return $task->title !== 'forbidden'; + } +} + +beforeEach(function () { + // The web middleware group encrypts cookies/sessions + config()->set('app.key', 'base64:'.base64_encode(str_repeat('a', 32))); + + Schema::create('column_state_users', function (Blueprint $table) { + $table->id(); + $table->string('name')->nullable(); + $table->timestamps(); + }); + + Schema::create('column_state_tasks', function (Blueprint $table) { + $table->id(); + $table->string('title')->nullable(); + $table->string('notes')->nullable(); + $table->string('status')->nullable(); + $table->boolean('is_done')->default(false); + $table->boolean('is_pinned')->default(false); + $table->boolean('locked')->default(false); + $table->timestamps(); + }); + + ColumnStateTaskResource::$afterUpdates = []; + + $this->app->singleton(PanelRegistry::class); + app(PanelRegistry::class)->register( + Panel::make('admin') + ->path('admin') + ->middleware(['web']) + ->authMiddleware(['auth']) + ->resources([ColumnStateTaskResource::class, ColumnStatePolicyTaskResource::class]) + ); + + app('router')->aliasMiddleware('panel.auth', Authenticate::class); + ColumnStateRoutes::register(); + app('router')->getRoutes()->refreshNameLookups(); + + Gate::policy(ColumnStateTask::class, ColumnStateTaskPolicy::class); + + $this->user = ColumnStateUser::create(['name' => 'Admin']); + $this->task = ColumnStateTask::create(['title' => 'Task', 'notes' => 'old', 'status' => 'draft']); +}); + +function columnStateUrl(string $resource, int|string $id): string +{ + return "/admin/_tables/{$resource}/{$id}/column"; +} + +it('serializes editable columns with their component, rules and disabled state', function () { + $select = SelectColumn::make('status')->options(['a' => 'A'])->rules(['required'])->disabled()->toInertiaProps(); + $input = TextInputColumn::make('notes')->type('number')->rules(['min:1'])->toInertiaProps(); + $checkbox = CheckboxColumn::make('is_pinned')->toInertiaProps(); + $toggle = ToggleColumn::make('is_done')->toInertiaProps(); + + expect($select)->toMatchArray(['component' => 'SelectColumn', 'options' => ['a' => 'A'], 'rules' => ['required'], 'disabled' => true, 'editable' => true]) + ->and($input)->toMatchArray(['component' => 'TextInputColumn', 'type' => 'number', 'rules' => ['min:1'], 'disabled' => false, 'editable' => true]) + ->and($checkbox)->toMatchArray(['component' => 'CheckboxColumn', 'editable' => true]) + ->and($toggle)->toMatchArray(['component' => 'ToggleColumn', 'editable' => true]); +}); + +it('exposes the column update route for resource tables only', function () { + app(PanelRegistry::class)->setCurrent('admin'); + + expect(Table::make()->resourceSlug('tasks')->getColumnUpdateRoute()) + ->toEndWith('/admin/_tables/tasks/__ID__/column') + ->and(Table::make()->getColumnUpdateRoute())->toBeNull(); +}); + +it('updates a text input column and only that attribute', function () { + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => 'notes', 'value' => 'new', 'title' => 'hacked']) + ->assertOk() + ->assertJson(['column' => 'notes', 'state' => 'new']); + + $this->task->refresh(); + expect($this->task->notes)->toBe('new') + ->and($this->task->title)->toBe('Task'); +}); + +it('validates using the column rules', function () { + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => 'notes', 'value' => 'way too long for ten']) + ->assertUnprocessable() + ->assertJsonValidationErrors('value'); + + expect($this->task->refresh()->notes)->toBe('old'); +}); + +it('only accepts declared select options', function () { + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => 'status', 'value' => 'archived']) + ->assertUnprocessable(); + + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => 'status', 'value' => 'published']) + ->assertOk(); + + expect($this->task->refresh()->status)->toBe('published'); +}); + +it('updates toggle and checkbox columns as booleans and runs callbacks', function () { + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => 'is_done', 'value' => true]) + ->assertOk() + ->assertJson(['state' => true]); + + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => 'is_pinned', 'value' => 1]) + ->assertOk(); + + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => 'is_pinned', 'value' => 'nope']) + ->assertUnprocessable(); + + $this->task->refresh(); + expect($this->task->is_done)->toBeTrue() + ->and($this->task->is_pinned)->toBeTrue() + ->and(ColumnStateTaskResource::$afterUpdates)->toBe([['is_done', true]]); +}); + +it('rejects columns that are not editable', function (string $column, int $status) { + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => $column, 'value' => 'x']) + ->assertStatus($status); + + expect($this->task->refresh()->title)->toBe('Task'); +})->with([ + 'display column' => ['title', 403], + 'disabled column' => ['locked', 403], + 'relationship column' => ['author.name', 403], + 'unknown column' => ['password', 404], +]); + +it('requires authentication', function () { + $this->patchJson(columnStateUrl('tasks', $this->task->id), ['column' => 'notes', 'value' => 'new']) + ->assertUnauthorized(); + + expect($this->task->refresh()->notes)->toBe('old'); +}); + +it('authorizes the record update through the resource policy', function () { + $forbidden = ColumnStateTask::create(['title' => 'forbidden', 'notes' => 'old']); + + $this->actingAs($this->user) + ->patchJson(columnStateUrl('policy-tasks', $forbidden->id), ['column' => 'notes', 'value' => 'new']) + ->assertForbidden(); + + $this->actingAs($this->user) + ->patchJson(columnStateUrl('policy-tasks', $this->task->id), ['column' => 'notes', 'value' => 'new']) + ->assertOk(); + + expect($forbidden->refresh()->notes)->toBe('old') + ->and($this->task->refresh()->notes)->toBe('new'); +}); + +it('returns not found for unknown resources and records', function () { + $this->actingAs($this->user) + ->patchJson(columnStateUrl('missing', $this->task->id), ['column' => 'notes', 'value' => 'new']) + ->assertNotFound(); + + $this->actingAs($this->user) + ->patchJson(columnStateUrl('tasks', 999), ['column' => 'notes', 'value' => 'new']) + ->assertNotFound(); +});