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 core paths (migration, embedding, permission checks, rollback on DB failure) are correctly implemented and the previously flagged issues are resolved.

Every issue raised in previous review rounds is confirmed fixed. The migration path correctly handles draft, submitted, and cancelled invoices through direct DB writes, rolls back on failure, deduplicates the background job, and gates the whitelisted endpoint behind System Manager. The validate_doc hook correctly guards attachment-table validation behind the multi-embed setting. DocType JSON timestamps carry proper microseconds and the auto-generated Python type annotations match the JSON field definitions.

Files Needing Attention: No files require special attention. The one remaining note (misleading parameter name in _file_already_in_attachments) is cosmetic and does not affect correctness.

Sequence Diagram

sequenceDiagram
    participant User
    participant SalesInvoice as Sales Invoice (validate)
    participant Attachments as sales_invoice_attachments.py
    participant DB as Database
    participant RQ as RQ Background Job

    User->>SalesInvoice: Save / Submit
    SalesInvoice->>Attachments: validate_doc(doc)
    Attachments->>DB: get E Invoice Settings.multi_attachment_embed_enabled
    alt "multi_attachment_embed_enabled = ON"
        Attachments->>Attachments: migrate_legacy_embed_to_table(doc)
        alt legacy URL present
            Attachments->>DB: _resolve_embed_file_for_invoice (invoice-scoped)
            alt File resolved
                Attachments->>Attachments: _persist_legacy_embed_migration_on_save
                note right of Attachments: append row in-memory, clear legacy field
            else File not found + table empty
                Attachments->>DB: frappe.log_error
            end
        end
        Attachments->>DB: _validate_embed_file_attached_to_invoice (per row)
        Attachments->>DB: get content_hash per row (dup-content msgprint)
    end

    User->>SalesInvoice: Download XML / PDF
    SalesInvoice->>Attachments: get_embed_attachments(invoice)
    alt "multi_attachment_embed_enabled = ON"
        Attachments->>Attachments: _get_table_embed_attachments
    else
        Attachments->>Attachments: _get_legacy_embed_attachment (find_file_by_url)
    end
    Attachments-->>SalesInvoice: list[EmbedAttachment]
    SalesInvoice->>SalesInvoice: _embed_attachments → CII 916 nodes

    User->>+DB: POST migrate_attachments_to_table (System Manager only)
    DB->>DB: frappe.only_for("System Manager")
    DB->>RQ: enqueue bulk_migrate_legacy_embed_attachments
    loop each invoice with legacy URL
        RQ->>DB: _resolve_embed_file_for_invoice
        alt File resolved
            RQ->>DB: _insert_attachment_row (INSERT)
            RQ->>DB: "set_value einvoice_embedded_document="""
            RQ->>DB: frappe.db.commit()
        else broken link
            RQ->>DB: log_error / clear field (if remove_broken_links)
        end
    end
    RQ-->>User: realtime msgprint summary

Loading

_{Reviews (21): Last reviewed commit: "fix(attachments): make legacy embed migr..." | 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.

[barredterra]

Custom fields should likely be (re-)created through a patch or bumping execute:from eu_einvoice.install import after_install; after_install() # 17 to #18

[dafrose]

Would it also make sense to include the custom field sync in after_migrate or is the patch bump the preferred solution? @barredterra

[barredterra]

Strictly speaking, I don't think we actually need the after_migrate part, since it does during every update what is really only necessary during the first update (via patch).

[dafrose]

fixed in bc47ed5

[barredterra]

I would have expected removal of after_migrate logic because it should now be redundant, right?

[dafrose]

@greptile-apps

[dafrose]

@greptile-apps

[greptile-apps]

Too many files changed for review (24 files, 15 file limit).

Bypass the limit by tagging @greptile-apps to review.