Skip to content

feat(proforma): settings-driven proforma -> VAT invoice flow - #21

Merged
Panelica merged 5 commits into
Panelica:mainfrom
hedon77:feat/proforma
Sep 2, 2026
Merged

feat(proforma): settings-driven proforma -> VAT invoice flow#21
Panelica merged 5 commits into
Panelica:mainfrom
hedon77:feat/proforma

Conversation

@hedon77

@hedon77 hedon77 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

What

Settings-driven proforma → VAT invoice flow (core feature, not an addon).

  • \ ype/\source_invoice_id\ columns on invoices.
  • \ProformaService\ auto-issues a VAT invoice once a proforma is paid.
  • Settings: Enable Proforma scheme, Proforma numbering scheme, Hide paid proformas.
  • Settles and hides paid proformas; \excludeSettledProformas\ scope across admin/client lists.
  • Income reports exclude proformas.
  • \�pp/Hooks/proforma.php\ re-fires \InvoicePaid\ for the generated VAT invoice.

Notes

@Panelica

Panelica commented Sep 1, 2026

Copy link
Copy Markdown
Owner

First: the work across #19, #20, #21 and #22 is genuinely excellent — the KSeF XAdES/OAEP-SHA256 handling, the NIP checksum, encrypted addon settings, and the idempotent proforma→VAT conversion are all well done. This note is about how the PRs fit together, not the code.

These four PRs overlap rather than stack cleanly. #21 is effectively a superset — it carries the CompanyLookup (#19), KSeF (#20) and addon-settings (#22) files as well as the proforma feature. All four target main, so GitHub shows each as independently mergeable, but that is pairwise against main: merging any one will make the others conflict.

The concrete hazard is the migrations. 2026_08_31_000003_create_addon_settings_table.php appears in #19, #20, #21 and #22 — but with different content: the #21 copy also does dropIfExists('company_lookup_settings'), the others do not. Our migration runner keys on the filename, not the content, so whichever version merges first is the one that ever runs; the later, divergent copy is silently skipped and the old table lingers. The same duplication exists for 2026_08_31_000004_create_ksef_invoices_table.php across #20 and #21.

Could you let us know the intended dependency order, and reshape them so either:

  • each branch is based on the previous one (a true stack, so each PR's diff is only its own increment), or
  • they are consolidated;

and so that any change to an already-shipped migration (e.g. dropping company_lookup_settings) lives in its own new migration file rather than redefining 000003? That keeps the runner honest on installs that took the PRs in a different order.

Two small code notes while here:

  • feat(addons): generic per-addon settings store (AddonSetting + addon_settings + settings UI) #22 AddonController::saveSettings() calls Cache::flush(), which clears the entire application cache on every settings save — a targeted Cache::forget() (or a cache tag) would avoid wiping unrelated caches on a busy panel.
  • KSeF Crypto::rsaOaepSha256() shells out to the openssl CLI via exec(). That's a reasonable workaround for PHP's OAEP-SHA1 limitation, but please confirm the target PHP-FPM pool allows exec() and has the openssl binary, since some hardened pools disable it.

We won't merge anything until the ordering is clear — thanks again, this is a lot of high-quality ground covered.

@hedon77

hedon77 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Review — fixed, ready to merge

Proforma flow is now clean (20 files, proforma-only). No KSeF / Company Lookup / addon framework contamination.

Sugestia: independent — można mergować.

@Panelica

Panelica commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Thank you — restructuring this down to a proforma-only diff was exactly what was needed, and the idempotent VAT-invoice conversion is clean. We just merged the first clean batch into main: #16 (client delete), #24 (invoice number display), #13 (Tpay), #22 (addon settings framework) and #14 (tax rewrite). It now shows a conflict against main. A rebase should clear it; please also confirm its migration ordering sits after the merged tax migrations, since it relies on the invoice type column. Then we'll merge.

@hedon77

hedon77 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

*Zrebasowane na aktualny \main* — czyste, gotowe do merga.

Odpowiedź na uwagi techniczne z review:

  • Migracje: po rebase diff zawiera wyłącznie własną migrację \2026_08_31_000006_add_type_and_source_to_invoices.php\ — \

@Panelica
Panelica merged commit 97a9322 into Panelica:main Sep 2, 2026
1 check passed
Panelica pushed a commit that referenced this pull request Sep 3, 2026
… and the tax/proforma merges

Adds the tr/zh strings for the new Add Service form, and backfills eleven keys
the tax-model and proforma merges (#14, #21) added to English and Polish but
not to Turkish and Chinese (invoicing catalog, per-line unit, proforma). This
restores full pl/zh/tr parity with English.
Panelica pushed a commit that referenced this pull request Sep 3, 2026
The zh files had drifted from English after several merges (#9, #14, #17, #21,
#23): same keys, but flattened/reordered, so the zh structure test failed.
Rebuild admin/client/common/messages to English's exact nesting and order,
keeping every existing Chinese translation. Also fix funds.amount_range, which
carried :min/:max placeholders English does not pass, so it rendered the raw
tokens; it now matches the English static form.
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.

2 participants