feat(attachments): multi-annex embed for Sales Invoice (part 1)

[dafrose]

Summary

Part 1 of selectable PDF annexes for Sales Invoice (#261, coordination #76). This PR is supposed to only add support for multiple attachments via the same embedding path as before. File attachments are still embedded into XML (which may be embedded into PDF). It does not provide support for embedding additional files directly into PDF. This is planned for a follow-up PR.

When Multiple Attachment Embedding is enabled on E Invoice Settings, annexes are selected from the new einvoice_attachments child table and embedded in the e-invoice XML as 916 supporting documents (same _embed_attachments loop as today). When the setting is off, behaviour stays on the legacy einvoice_embedded_document attach field.

• einvoice_attachments child table on Sales Invoice with Desk attach button and first-row-wins dedup on validate
• multi_attachment_embed_enabled checkbox on E Invoice Settings
• get_embed_attachments branches between table and legacy field; shared module sales_invoice_attachments.py
• Migration on enable: per-save move of legacy field → table (orange msgprint); bulk RQ job via Desk button; legacy field hidden + read_only while multi-embed is on
• Embed refactor: get_legacy_embed_attachment + _embed_attachments URL loop (no change to hybrid PDF mechanics yet — annex bytes still travel inside embedded Factur-X XML)
• Tests: New tests cover embedding behaviour of the legacy path as well as the table path, migration from legacy field to table and table-specific validations.

Not in this PR (possible follow-ups): PDF/A-3 factur-x pipeline with N PDF file attachments (#262), profile-aware 916 encoding, MIME allowlist, v15/develop backports.

Settings:

Migration Button UI:

Sales Invoice EInvoice tab:

Test plan

• bench --site <site> run-tests --app eu_einvoice --lightmode — 22 tests green
• E Invoice Settings: enable Multiple Attachment Embedding → legacy attach field hidden/read-only
• Sales Invoice with legacy einvoice_embedded_document only → save with setting on → row appears in Embedded Documents, legacy field cleared, orange migration message
• Sales Invoice with broken legacy URL → save with setting on → field unchanged, Error Log entry (no blocking save)
• E Invoice Settings → Migrate attachments to table → RQ job clears legacy fields site-wide
• Sales Invoice with two rows in einvoice_attachments → submit/download PDF → annex payloads present in embedded Factur-X XML (Desk spot-check or XML download)

Notes for reviewers

• Hybrid PDF path unchanged: attach_xml_to_pdf still embeds invoice XML only; new integration test verifies table annex content survives the PDF round-trip inside that XML.
• Target branch is version-16-hotfix per rollout plan; develop backport differs (patch migration drops legacy field).

Ticket Ref

LMK-16

[dafrose]

@greptileai

[greptile-apps]

Confidence Score: 5/5

• Safe to merge; all changes are well-contained and the previously reported issues have been resolved.
• The new embedding path is thoroughly tested, permission checks are in place, the bulk migration uses per-invoice commits with rollback on failure, and the validate_doc gating correctly limits migration and row validation to the multi-embed mode. The two flagged items are edge-case nits that do not affect the main code paths.
• eu_einvoice/european_e_invoice/custom/sales_invoice_attachments.py — the _file_already_in_attachments parameter naming and the duplicate-filename migration edge case are both minor but worth a quick look before the next iteration.

Sequence Diagram

sequenceDiagram
    participant Desk as Sales Invoice (Desk)
    participant VD as validate_doc
    participant MIG as migrate_legacy_embed_to_table
    participant VAL as validate_einvoice_attachment_rows
    participant GEN as EInvoiceGenerator._embed_attachments
    participant GEA as get_embed_attachments
    participant DB as Database

    Desk->>VD: save / submit

    VD->>DB: get_single("E Invoice Settings")
    DB-->>VD: settings

    alt "multi_attachment_embed_enabled = True"
        VD->>MIG: migrate_legacy_embed_to_table(doc)
        MIG->>DB: _resolve_embed_file_for_invoice
        DB-->>MIG: "File | None"

        alt File resolved
            MIG->>MIG: _persist_legacy_embed_migration_on_save
            MIG-->>VD: True (migrated)
        else Broken URL, table has rows
            MIG->>DB: clear einvoice_embedded_document (in-memory)
            MIG-->>VD: False
        else Broken URL, no table rows
            MIG->>DB: log_error
            MIG-->>VD: False
        end

        VD->>VAL: validate_einvoice_attachment_rows(doc, settings)
        VAL->>VAL: _validate_duplicate_embed_filenames
        VAL->>DB: "get_value(File, file_id, attached_to_*)"
        VAL->>DB: get_value(File, file_id, content_hash) per row
        VAL-->>VD: ok / throw ValidationError
    else "multi_attachment_embed_enabled = False"
        VD->>VD: skip migration and table validation
    end

    VD->>VD: validate_einvoice(doc) if configured

    Note over GEN,GEA: On e-invoice generation (download / submit)

    GEN->>GEA: get_embed_attachments(invoice)
    GEA->>DB: get_single_value(multi_attachment_embed_enabled)

    alt "enabled = True"
        GEA->>GEA: _get_table_embed_attachments (validates again)
        GEA-->>GEN: [EmbedAttachment, ...]
    else "enabled = False"
        GEA->>GEA: _get_legacy_embed_attachment
        GEA-->>GEN: "[EmbedAttachment] | []"
    end

    loop for each EmbedAttachment
        GEN->>DB: frappe.get_doc("File", attachment.file)
        DB-->>GEN: File doc
        GEN->>GEN: build AdditionalReferencedDocument (ARD 916)
    end

Loading

_{Reviews (22): Last reviewed commit: "refactor(install): remove redundant afte..." | Re-trigger Greptile}

[dafrose]

@greptileai

[greptile-apps]

Want your agent to iterate on Greptile's feedback? Try greploops.

[dafrose]

I tested a simple print format addition for the embedded files.

The code I used:

		{# Embedded e-invoice annexes (916 supporting documents) #}
		{% set multi_embed = frappe.db.get_single_value("E Invoice Settings", "multi_attachment_embed_enabled") %}
		{% if multi_embed and doc.einvoice_attachments %}
		<div class="info-card">
			<div class="title">{{ _("Embedded Documents") }}</div>
			<ul class="small-text" style="margin: 8px 0 0 0; padding-left: 18px;">
				{% for row in doc.einvoice_attachments %}
				{% set file_url = frappe.db.get_value("File", row.file, "file_url") %}
				<li style="margin-bottom: 4px;">
					{% if file_url %}
					<a href="{{ frappe.utils.get_url(file_url) }}">
						{{ row.display_name or row.file_name or file_url }}
					</a>
					{% else %}
					{{ row.display_name or row.file_name or row.file }}
					{% endif %}
				</li>
				{% endfor %}
			</ul>
		</div>
		{% elif not multi_embed and doc.einvoice_embedded_document %}
		<div class="info-card">
			<div class="title">{{ _("Embedded Document") }}</div>
			<p class="small-text" style="margin-top: 8px;">
				<a href="{{ frappe.utils.get_url(doc.einvoice_embedded_document) }}">
					{{ doc.einvoice_embedded_document }}
				</a>
			</p>
		</div>
		{% endif %}

The result:

[dafrose]

@greptile-apps

[dafrose]

Update since 19 Jun 2026

Focus: annex validation hardening, Desk field exclusivity, and two production fixes.

What changed

• Unique embed filenames (BR-DE-22) — DB unique index on (parent, file_name) on E Invoice Attachment Row; save-time duplicate check when an error action is configured; embed/XML path always enforces uniqueness.
• Duplicate annex content — identical File content_hash across table rows → orange Desk hint on save (warn-only; filenames can still differ).
• Desk exclusivity — with multi-embed off: only legacy Embedded Document visible; with multi-embed on: only Embedded Documents table; legacy hidden + read-only. Synced from E Invoice Settings, after_install, and after_migrate.
• Annex row validation — validate_einvoice_attachment_rows runs only when multi_attachment_embed_enabled is on (orphan table rows in legacy mode no longer block save).
• Bulk legacy migrate — failed per-invoice DB persist rolls back the transaction so a later success cannot leave orphan child rows.
• Binary annex embed — CII embed reads file bytes with get_content(encodings=[]) so small PNG/PDF annexes are not corrupted by Frappe’s default text decoding.
• German locale — de.po / main.pot updated for new strings; run compile-po-to-mo if you test Desk in German.

Tests: bench --site <site> run-tests --app eu_einvoice --lightmode (app tests self-seed; no ERPNext test_records preload).

Reviewer setup — unique constraint

There is no migrate patch for unique_parent_file_name. Greenfield installs get the constraint when the child DocType is first synced.

If your review site already had E Invoice Attachment Row before pulling this branch:

1. Remove any duplicate (parent, file_name) rows on test invoices.
2. Apply the constraint once:

bench --site <site> execute eu_einvoice.european_e_invoice.doctype.e_invoice_attachment_row.e_invoice_attachment_row.on_doctype_update

bench migrate alone does not re-run on_doctype_update when the child DocType JSON hash is unchanged.

Alternatively: fresh site + install-app eu_einvoice, or save E Invoice Attachment Row in Desk once to trigger on_doctype_update.

Test plan

Area	Steps
Constraint	Enable multi-embed; on one draft Sales Invoice, add two Embedded Documents rows with the same File Name → save blocked with duplicate-filename message when error action is set.
Content hash	Two rows, different filenames, same file content → orange msgprint; save still allowed.
Exclusivity	Never-enabled site: reload invoice form → only legacy attach. Enable setting → reload → only table; legacy not editable.
Validate gate	(Optional) With setting off and table rows present via API/import → save does not run annex row validation (normal Desk path hides the table).
Bulk migrate	E Invoice Settings → migrate with Include submitted; submitted invoice with legacy embed migrates to table; broken URL skip/remove still works.
Embed smoke	Draft invoice with one annex (legacy or table path) → create/download e-invoice XML; annex present as ARD 916.

Automated: bench --site <site> run-tests --app eu_einvoice --lightmode.

[barredterra]

Suggested merge bar

Solid after the earlier review rounds (permissions, POST-only, orphaned-row rollback, binary encodings=[], field exclusivity). CI is green. Please address these three correctness items before merge:

1. Don’t use validating save() for bulk draft migration (or skip e-invoice validation there).
2. Always enforce duplicate annex filenames when multi-embed is on.
3. Set idx on direct child inserts.

Inline comments below.