[IP-389]: Notes template/selection option in quotations and invoices#612
Conversation
📝 WalkthroughWalkthroughAdds company-scoped note templates with database persistence, Filament management, permission checks, and reusable insertion actions for invoice and quote note fields. Feature tests cover CRUD, tenancy isolation, validation, and editor insertion. ChangesNote template workflow
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MarkdownEditor
participant InsertNoteTemplateAction
participant NoteTemplate
participant FormState
MarkdownEditor->>InsertNoteTemplateAction: submit selected template and replacement mode
InsertNoteTemplateAction->>NoteTemplate: load company-scoped template
NoteTemplate-->>InsertNoteTemplateAction: return template body
InsertNoteTemplateAction->>FormState: replace or append body
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
Modules/Core/Tests/Feature/NoteTemplatesTest.php (1)
148-154: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssign Livewire result to a variable for consistency.
Assigning the Livewire component execution to a
$componentvariable and asserting$component->assertSuccessful()verifies the action succeeded without internal errors and matches the structure of your other tests. As per coding guidelines, structure tests to define all variables in the Act section before asserting.♻️ Proposed refactor
- Livewire::actingAs($this->user) + $component = Livewire::actingAs($this->user) ->test(ListNoteTemplates::class) ->mountAction(TestAction::make('delete')->table($template)) ->callMountedAction(); /* Assert */ + $component->assertSuccessful(); $this->assertDatabaseMissing('note_templates', ['id' => $template->id]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Core/Tests/Feature/NoteTemplatesTest.php` around lines 148 - 154, Update the delete-action test using ListNoteTemplates to assign the Livewire test chain to a $component variable in the Act section, then assert $component->assertSuccessful() before verifying the note template is absent from the database.Source: Coding guidelines
Modules/Core/Filament/Company/Resources/NoteTemplates/Tables/NoteTemplatesTable.php (1)
42-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEnsure bulk deletions use the service layer.
The single
DeleteActionon line 37 correctly delegates toNoteTemplateService::deleteNoteTemplate(). However,DeleteBulkActionrelies on the default Eloquent deletion. If the service layer performs necessary business logic (such as logging or cascading cleanup), bulk deletions will bypass it. Consider overriding the action forDeleteBulkActionto use the service as well.♻️ Proposed refactor
->toolbarActions([ BulkActionGroup::make([ - DeleteBulkAction::make(), + DeleteBulkAction::make() + ->action(function (\Illuminate\Database\Eloquent\Collection $records) { + $records->each(fn ($record) => app(NoteTemplateService::class)->deleteNoteTemplate($record, [])); + }), ]), ]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Core/Filament/Company/Resources/NoteTemplates/Tables/NoteTemplatesTable.php` around lines 42 - 46, Update the DeleteBulkAction within NoteTemplatesTable to delete each selected NoteTemplate through NoteTemplateService::deleteNoteTemplate(), matching the existing single DeleteAction behavior, instead of relying on default Eloquent deletion.Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php (1)
228-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid restricting the Markdown toolbar for fields receiving rich templates.
The PR successfully expands the template editor's toolbar to support headings, lists, and blockquotes. However, the destination fields on the invoice form restrict the toolbar to only bold and italic. When a user inserts a rich template into these fields, they will lack the UI controls necessary to properly edit the imported formatting.
Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php#L228-L229: Remove the->toolbarButtons(['bold', 'italic'])restriction on thenotesfield or expand it to match the template editor.Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php#L249-L250: Apply the same adjustment to theinvoice_termsfield.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php` around lines 228 - 229, The notes field at Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php lines 228-229 and the invoice_terms field at lines 249-250 restrict their Markdown toolbar to bold and italic; remove that toolbarButtons restriction or expand both fields to match the rich template editor’s supported formatting while preserving their existing hintAction configuration.Modules/Core/Services/NoteTemplateService.php (1)
35-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused parameter.
The
$dataparameter is not used anywhere within this method. You can safely remove it to clean up the signature.♻️ Proposed refactor
- public function deleteNoteTemplate(NoteTemplate $noteTemplate, array $data = []): NoteTemplate + public function deleteNoteTemplate(NoteTemplate $noteTemplate): NoteTemplate🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Core/Services/NoteTemplateService.php` at line 35, Remove the unused $data parameter from the deleteNoteTemplate method signature, preserving the NoteTemplate argument and return type. Update any callers to invoke deleteNoteTemplate with only the NoteTemplate instance.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Modules/Core/Filament/Company/Actions/InsertNoteTemplateAction.php`:
- Around line 32-34: Update the NoteTemplate lookup inside the action closure of
InsertNoteTemplateAction to scope the query to the current company/tenant before
resolving data['note_template_id']; preserve the existing template-selection
flow while ensuring cross-company template IDs cannot be accessed.
- Line 5: Update the Action import used by InsertNoteTemplateAction to
Filament\Forms\Components\Actions\Action, and align any method return types or
type references in InsertNoteTemplateAction with that form-component action
class so hintAction() receives the expected type.
In `@Modules/Quotes/Filament/Company/Resources/Quotes/Schemas/QuoteForm.php`:
- Around line 221-222: Update the notes editor configuration in the Quote form
schema by removing the restrictive toolbarButtons(['bold', 'italic']) call from
the field using InsertNoteTemplateAction::make('notes), allowing Filament’s
comprehensive default Markdown toolbar to provide the intended formatting
options.
---
Nitpick comments:
In
`@Modules/Core/Filament/Company/Resources/NoteTemplates/Tables/NoteTemplatesTable.php`:
- Around line 42-46: Update the DeleteBulkAction within NoteTemplatesTable to
delete each selected NoteTemplate through
NoteTemplateService::deleteNoteTemplate(), matching the existing single
DeleteAction behavior, instead of relying on default Eloquent deletion.
In `@Modules/Core/Services/NoteTemplateService.php`:
- Line 35: Remove the unused $data parameter from the deleteNoteTemplate method
signature, preserving the NoteTemplate argument and return type. Update any
callers to invoke deleteNoteTemplate with only the NoteTemplate instance.
In `@Modules/Core/Tests/Feature/NoteTemplatesTest.php`:
- Around line 148-154: Update the delete-action test using ListNoteTemplates to
assign the Livewire test chain to a $component variable in the Act section, then
assert $component->assertSuccessful() before verifying the note template is
absent from the database.
In
`@Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php`:
- Around line 228-229: The notes field at
Modules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.php
lines 228-229 and the invoice_terms field at lines 249-250 restrict their
Markdown toolbar to bold and italic; remove that toolbarButtons restriction or
expand both fields to match the rich template editor’s supported formatting
while preserving their existing hintAction configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b205cc88-0c53-4fd6-b37f-74055d60f11c
📒 Files selected for processing (15)
Modules/Core/Database/Factories/NoteTemplateFactory.phpModules/Core/Database/Migrations/2026_07_14_000001_create_note_templates_table.phpModules/Core/Filament/Company/Actions/InsertNoteTemplateAction.phpModules/Core/Filament/Company/Resources/NoteTemplates/NoteTemplateResource.phpModules/Core/Filament/Company/Resources/NoteTemplates/Pages/ListNoteTemplates.phpModules/Core/Filament/Company/Resources/NoteTemplates/Schemas/NoteTemplateForm.phpModules/Core/Filament/Company/Resources/NoteTemplates/Tables/NoteTemplatesTable.phpModules/Core/Models/NoteTemplate.phpModules/Core/Providers/CompanyPanelProvider.phpModules/Core/Services/NoteTemplateService.phpModules/Core/Tests/Feature/NoteTemplatesTest.phpModules/Invoices/Filament/Company/Resources/Invoices/Schemas/InvoiceForm.phpModules/Invoices/Tests/Feature/InvoicesTest.phpModules/Quotes/Filament/Company/Resources/Quotes/Schemas/QuoteForm.phpModules/Quotes/Tests/Feature/QuotesTest.php
Pull Request Checklist
Checklist
Description
Adds reusable, company-scoped note/terms templates so admins no longer have to retype the same boilerplate legal/payment text on every invoice or quote.
NoteTemplatemodel (tablenote_templates, fieldstemplate_title/template_body), scoped via the existingBelongsToCompanytrait.manage-company-settingspermission (same asNumberingResource).InsertNoteTemplateAction— an "Insert Template" hint-action onMarkdownEditorfields that opens a searchable template picker and either appends or replaces the field's content.notes+invoice_terms, and Quotenotes.Related Issue(s)
Closes #389
Motivation and Context
Company admins were retyping the same standard terms (e.g. "SEO Project Terms", "Web Dev Payment Terms") into every invoice/quote. This adds a small reusable template library plus a one-click insert action on the notes/terms fields, removing that repetitive work.
Issue Type (Check one or more)
Screenshots (If Applicable)
New Navigation Menu
Summary by CodeRabbit
New Features
Tests