Skip to content

Commit 4436c5b

Browse files
committed
Address PR review concerns - add safeguards and error handling
Security & Data Protection: - Add enhanced confirmation dialog for system template deletion with clear warnings - Add backend validation to prevent deletion of last remaining template - Implement proper error handling and user feedback for validation failures Code Quality Improvements: - Add robust error handling for empty templates array in default selection - Implement graceful fallback when no templates exist - Add meaningful error messages and user-friendly alerts - Improve template deletion workflow with better UX Backend Changes: - Add template count validation before deletion in TemplateController - Return 422 status with descriptive error messages - Ensure data integrity by maintaining at least one template Frontend Changes: - Enhanced delete confirmation with system template warnings - Proper error response handling from backend validation - Improved default template selection logic with edge case handling Addresses all valid security and code quality concerns from PR review.
1 parent bdb0c54 commit 4436c5b

File tree

3 files changed

+34
-8
lines changed

3 files changed

+34
-8
lines changed

app/Http/Controllers/TemplateController.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,14 @@ public function update(Request $request, Template $template)
101101

102102
public function destroy(Template $template)
103103
{
104+
// Prevent deletion if this is the last template
105+
$remainingTemplatesCount = Template::where('id', '!=', $template->id)->count();
106+
if ($remainingTemplatesCount === 0) {
107+
return response()->json([
108+
'error' => 'Cannot delete the last remaining template. At least one template must exist.'
109+
], 422);
110+
}
111+
104112
$template->delete();
105113

106114
return response()->json([

resources/js/pages/RealtimeAgent/Main.vue

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2525,12 +2525,18 @@ const fetchTemplates = async () => {
25252525
}
25262526
25272527
// Select default template if none selected
2528-
if (!selectedTemplate.value && templates.value.length > 0) {
2529-
// Try to find the Sales Discovery Call template as default, otherwise use first available template
2530-
const defaultTemplate = templates.value.find((t) => t.name === 'Sales Discovery Call') || templates.value[0];
2531-
if (defaultTemplate) {
2532-
selectedTemplate.value = defaultTemplate;
2533-
console.log('📌 Selected default template:', defaultTemplate.name);
2528+
if (!selectedTemplate.value) {
2529+
if (templates.value.length > 0) {
2530+
// Try to find the Sales Discovery Call template as default, otherwise use first available template
2531+
const defaultTemplate = templates.value.find((t) => t.name === 'Sales Discovery Call') || templates.value[0];
2532+
if (defaultTemplate) {
2533+
selectedTemplate.value = defaultTemplate;
2534+
console.log('📌 Selected default template:', defaultTemplate.name);
2535+
}
2536+
} else {
2537+
// Handle case when no templates exist
2538+
console.warn('⚠️ No templates available - cannot select default template');
2539+
// You may want to show a user-friendly message or redirect to template creation
25342540
}
25352541
}
25362542
} catch (error) {

resources/js/pages/Templates/Index.vue

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,25 @@ const editTemplate = (templateId: string) => {
6363
};
6464
6565
const deleteTemplate = async (template: Template) => {
66-
if (confirm(`Are you sure you want to delete "${template.name}"?`)) {
66+
// Enhanced confirmation for system templates
67+
let confirmMessage = `Are you sure you want to delete "${template.name}"?`;
68+
if (template.is_system) {
69+
confirmMessage = `⚠️ WARNING: You are about to delete a built-in system template!\n\n"${template.name}" is a pre-configured template that may be useful for many users.\n\nAre you absolutely sure you want to permanently delete this system template?`;
70+
}
71+
72+
if (confirm(confirmMessage)) {
6773
try {
6874
await axios.delete(route('templates.destroy', template.id));
6975
await fetchTemplates();
7076
} catch (error) {
7177
console.error('Failed to delete template:', error);
72-
alert('Failed to delete template. Please try again.');
78+
79+
// Handle specific error messages from backend
80+
if (error.response?.status === 422 && error.response?.data?.error) {
81+
alert(error.response.data.error);
82+
} else {
83+
alert('Failed to delete template. Please try again.');
84+
}
7385
}
7486
}
7587
};

0 commit comments

Comments
 (0)