Skip to content

Just develop to master#282

Draft
nielsdrost7 wants to merge 182 commits into
masterfrom
develop
Draft

Just develop to master#282
nielsdrost7 wants to merge 182 commits into
masterfrom
develop

Conversation

@nielsdrost7

@nielsdrost7 nielsdrost7 commented Dec 29, 2025

Copy link
Copy Markdown
Collaborator

┌─────┬─────────────────────────────────────────────────────────────────────┬──────────────────────┐
│ # │ Issue │ Fix │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ │ database/migrations/2025_06_03_142408_create_sessions_table.php │ Deleted the │
│ 1 │ duplicated Modules/Core's sessions migration — crashed │ duplicate │
│ │ RefreshDatabase and cascaded 300 failures │ │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ │ UserService::updateUser(array $validatedInput, $userToUpdate) — │ Swapped to │
│ 2 │ parameter order was reversed from every caller (updateUser($record, │ updateUser(User │
│ │ $data)). Would throw "call to fill() on array" at runtime │ $user, array $data) │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ 3 │ Invoice::mailQueue() declared return type Builder but returns │ Fixed to HasMany │
│ │ HasMany │ │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ │ Invoice::activities(), attachments(), clientAttachments() all │ Removed all three │
│ 4 │ returned null. clientAttachments() also referenced non-existent │ dead methods │
│ │ $this->status_text and unqualified 'Attachment' string │ │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ 5 │ Same dead methods in Quote │ Removed │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ │ DeleteBulkAction on UsersTable had no super_admin guard — bulk │ Added │
│ 6 │ delete could wipe all super admins │ reject(super_admin) │
│ │ │ before delete │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ 7 │ Modules/Quotes/Listeners/QuoteWasUpdatedListenerTest.php — test │ Moved to │
│ │ file in wrong directory, PHPUnit never found it │ Tests/Feature/ │
└─────┴─────────────────────────────────────────────────────────────────────┴──────────────────────┘

What was extracted to branches

┌─────────────────────────────────────────────────────────────┬────────────────────────────────────┐
│ Orphan │ Branch │
├─────────────────────────────────────────────────────────────┼────────────────────────────────────┤
│ app/Mason/Bricks/ + app/Mason/Collections/ (17 files, │ feature/130-report-builder │
│ survived a "remove" commit) │ (already existed) │
├─────────────────────────────────────────────────────────────┼────────────────────────────────────┤
│ Entire DocumentGroup subsystem (model, resource, service, │ │
│ factory, seeder, migration, observer, tests — 14 files, │ feature/document-groups-cleanup │
│ unregistered dead code, Numbering is the canonical │ (new) │
│ replacement) │ │
└─────────────────────────────────────────────────────────────┴────────────────────────────────────┘

Remaining known issues (not blocking tests, need dedicated work)

  • 44 tests tagged #[Group('failing')] — most are markTestSkipped(). Legitimate tracking, but several
    should now actually pass (duplicate-number tests) and need un-tagging. SettingsTest (9 tests) is
    all-skipped due to a Filament 5 blade rendering issue.
  • FileUpload::make('attachments') in InvoiceForm — bound to a field with no backing relation. Won't
    crash at render time, but upload would silently do nothing.
  • QuoteService::createQuote — $quoteTotal is computed but ignored; 'quote_total' =>
    $data['quote_total'] ?? 0 uses raw i Jump to bottom (ctrl+End) ↓ correctly uses the computed value.

@InvoicePlane InvoicePlane deleted a comment from coderabbitai Bot Dec 29, 2025
@InvoicePlane InvoicePlane deleted a comment from coderabbitai Bot Dec 29, 2025
@nielsdrost7

This comment was marked as outdated.

This comment was marked as outdated.

Copilot AI added a commit that referenced this pull request Dec 29, 2025
Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
nielsdrost7 added a commit that referenced this pull request Dec 29, 2025
…ts (#287)

* Initial plan

* Apply code review fixes from PR #282

Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
@InvoicePlane InvoicePlane deleted a comment from coderabbitai Bot Dec 29, 2025
nielsdrost7

This comment was marked as outdated.

nielsdrost7

This comment was marked as outdated.

This comment was marked as outdated.

@InvoicePlane InvoicePlane deleted a comment from coderabbitai Bot Dec 30, 2025
@InvoicePlane InvoicePlane deleted a comment from coderabbitai Bot Dec 30, 2025
@coderabbitai

coderabbitai Bot commented Dec 30, 2025

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9639bc0e-5d58-4a5d-89cf-acebb1548c60

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI commented Dec 30, 2025

Copy link
Copy Markdown
Contributor

@nielsdrost7 I've opened a new pull request, #296, to work on those changes. Once the pull request is ready, I'll request review from you.

nielsdrost7 and others added 30 commits July 17, 2026 17:37
…e-to-invoice

[IP-160]: convert quote to invoice
# Conflicts:
#	.gitignore
#	Modules/Clients/Filament/Company/Resources/Relations/Tables/RelationsTable.php
#	Modules/Quotes/Tests/Feature/QuotesTest.php
#	resources/lang/en/ip.php
PR #580 adds SoftDeletes to the Relation model. ViewRelationTest's
deletion assertion predates that and expected a hard delete -- update
it to assertSoftDeleted, matching the intentional new behavior.
# Conflicts:
#	.gitignore
#	Modules/Clients/Models/Relation.php
#	Modules/Invoices/Filament/Company/Resources/Invoices/Tables/InvoicesTable.php
#	Modules/Quotes/Tests/Feature/QuotesTest.php
#	resources/lang/en/ip.php
…tion

develop already has a fuller EmailInvoiceAction (modal, template-based
recipient/subject/body) that supersedes this PR's inline 'send email'
row action -- keep EmailInvoiceAction, but restore the disabled-when-
no-email safeguard this PR added, which the newer action had dropped.
Applied at the InvoicesTable call site (not inside EmailInvoiceAction
itself) since the action is shared with EditInvoice's header action,
which has no such restriction.

Also fixes EmailInvoiceActionTest's recipient assertion: it compared
against Relation::customer_email (now derived from Contact
communications since PR #577), but InvoiceService::resolveEmailDefaults()
reads the raw email column -- assert against that instead, since it's
what the modal actually prefills from.
…actions

[IP-31]: complete the invoice list row action group (#31, #167, #201, #530, #48)
# Conflicts:
#	.gitignore
#	Modules/Quotes/Tests/Feature/QuotesTest.php
…nit-tests

[IP-135]: calculator unit tests
InvoiceService::createInvoice()/updateInvoice() and QuoteService's
equivalents build their model payload from an explicit field allowlist
that predates this PR's two new columns -- the form fields and
migration existed but were silently dropped on save. Add both fields
to all four methods.

Also fixes both ReferenceFieldsTest files: the Numbering factory picks
a random NumberingType by default, but the resource forms filter the
numbering_id select to a specific type, so the test flakes whenever
the random type doesn't match. Pin the type explicitly.
…gory-filter

[IP-390]: filter products by category and unit
…t-variables

[IP-363]: invoicing-contact variables in email templates
# Conflicts:
#	Modules/Invoices/Filament/Company/Resources/Invoices/Tables/InvoicesTable.php
#	Modules/Invoices/Services/InvoiceService.php
#	Modules/Invoices/Tests/Feature/InvoicesTest.php
…n modal

PR #614 implemented actual invoice email sending (Mail::to()->queue())
and a more robust contacts/communications-based recipient resolution,
while develop's already-merged EmailInvoiceAction (PR #613/#581) only
had a richer modal (recipient/subject/body, prefilled from a company
EmailTemplate) with a 'not_yet_implemented' stub for the actual send.

Combine both: keep the modal, wire its submit to actually queue the
mail using the (possibly user-edited) recipient/subject/body from the
form, and upgrade resolveEmailDefaults()'s recipient resolution to
walk customer->contacts->communications (PR #614's approach) instead
of reading the raw, largely-vestigial Relation::email column.

Also fixes stale test assumptions surfaced by this integration:
- InvoicesTest's email tests referenced the action by its old name
  ('send email'); it's now 'email_invoice' via EmailInvoiceAction.
- it_uses_the_invoice_sent_email_template_when_one_exists_for_the_company
  created a second 'invoice_sent' EmailTemplate row instead of updating
  the one CompanyObserver already auto-bootstraps for every company,
  so the wrong (default) template content won non-deterministically.
- EmailInvoiceActionTest's customer factory only set the raw email
  column, not a Contact+Communication, so the new resolution path
  found no recipient -- added a primary email communication to match.
…mail-numbering

[IP-150]: Implement invoice email sending and numbering-group reassignment (#150, #46)
…rmat-fields

[IP-620]: reconcile Numbering Format/Group Identifier Format fields, add click-to-insert placeholders
# Conflicts:
#	Modules/Core/Filament/Admin/Resources/Numberings/Schemas/NumberingForm.php
#	Modules/Invoices/Filament/Company/Resources/Invoices/Pages/CreateInvoice.php
#	Modules/Invoices/Tests/Feature/InvoicesTest.php
#	Modules/Quotes/Filament/Company/Resources/Quotes/Pages/CreateQuote.php
…ering-system

[IP-618]: [Core][Quotes][Invoices] Fix Numbering seeder type-scoping, wire number generators into create flow, Prefix dropdown
…paign

- CreateRelation page bypassed RelationService, mass-assigning the raw
  form array (including the virtual email_cc field) straight into
  Relation::create(). Add handleRecordCreation() to route through
  RelationService::createRelation(), matching EditRelation's existing
  pattern -- this also fixes CC-email persistence, which only ever
  worked via RelationService::syncCcEmails().

- Invoice/Quote's "notes" MarkdownEditor field had no backing column;
  it belongs in the existing polymorphic notes table (notable_type/
  notable_id) alongside client notes, via the notes() MorphMany already
  defined on both models. Add a shared HasNotesAttribute trait exposing
  notes as a plain get/set attribute backed by a single Note record,
  deferred to the saved event since the relation needs the model's id.

- Delete QuoteWasUpdatedListener/QuoteWasUpdated (empty listener, dead
  event, empty test stub) and their sibling QuoteWasCreatedListener/
  QuoteWasCreated, discovered while removing the first pair --
  QuoteWasCreatedListener's handle() was copy-paste-typed to
  QuoteWasUpdated instead of its own event, and neither event is ever
  dispatched anywhere in the codebase.

CompanyPanelAdminLinkTest (missing Vite manifest) and
InvoicePdfAndCreditNoteTest (missing Dompdf) remain -- both are test
environment gaps (no npm build, incomplete composer install in the
ip2-test-php:8.4 image), not code bugs; left untouched per plan.
…ilures

fix: three pre-existing bugs surfaced repeatedly during the merge campaign
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants