Skip to content

feat: implement XML attachment file naming functionality (backport #253)#258

Open
mergify[bot] wants to merge 2 commits into
version-15-hotfixfrom
mergify/bp/version-15-hotfix/pr-253
Open

feat: implement XML attachment file naming functionality (backport #253)#258
mergify[bot] wants to merge 2 commits into
version-15-hotfixfrom
mergify/bp/version-15-hotfix/pr-253

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented May 12, 2026

  • Added E Invoice Settings field for global autonaming pattern for XML attachements.
  • Added get_xml_attachment_file_base_name function to resolve file names.
  • Introduced unit tests for various naming scenarios in test_xml_attachment_naming.py.
  • Updated Sales Invoice logic to utilize the new naming function for XML file attachments.
  • Integrated autonaming pattern into "Download XML" button on Sales Invoice.

Ref: IBT-164
closes #245


This is an automatic backport of pull request #253 done by Mergify.

@mergify mergify Bot added the conflicts label May 12, 2026
@mergify
Copy link
Copy Markdown
Contributor Author

mergify Bot commented May 12, 2026

Cherry-pick of d45a1f0 has failed:

On branch mergify/bp/version-15-hotfix/pr-253
Your branch is up to date with 'origin/version-15-hotfix'.

You are currently cherry-picking commit d45a1f0.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   eu_einvoice/european_e_invoice/custom/sales_invoice.py
	new file:   eu_einvoice/european_e_invoice/custom/test_sales_invoice.py
	modified:   eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.json
	both modified:   eu_einvoice/locale/de.po
	both modified:   eu_einvoice/locale/main.pot

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 12, 2026

Greptile Summary

This backport (from #253) introduces configurable XML attachment file naming for Sales Invoices: a new auto_name_format_for_xml_file field in E Invoice Settings lets users define a Frappe naming-series pattern, resolved at runtime by the new get_xml_attachment_file_base_name helper with graceful fallback to doc.name. The backport merge was not cleanly resolved, leaving raw Git conflict markers committed in three files.

  • sales_invoice.py / e_invoice_settings.py / test file: Python changes are correct — the naming helper, _no_series_counter, and the updated download_xrechnung / _attach_xml_file call-sites are all well-implemented and covered by 8 unit tests.
  • e_invoice_settings.json: Contains unresolved conflict markers; the file is invalid JSON and Frappe cannot load the DocType definition from it.
  • de.po / main.pot: Both locale files also contain conflict markers at multiple locations, invalidating them as gettext files and losing all German translations.

Confidence Score: 1/5

Not safe to merge — three committed files are broken due to unresolved backport merge conflicts.

The Python implementation and tests are solid, but e_invoice_settings.json is invalid JSON with literal conflict markers, meaning Frappe will fail to load the E Invoice Settings DocType entirely. Both locale files (de.po, main.pot) are also corrupted by the same unresolved conflict, breaking German translations. These files must be corrected before merging.

e_invoice_settings.json, de.po, and main.pot all require manual conflict resolution before this backport can be merged.

Important Files Changed

Filename Overview
eu_einvoice/european_e_invoice/custom/sales_invoice.py Adds get_xml_attachment_file_base_name and _no_series_counter; updates download_xrechnung and _attach_xml_file to use the new naming helper. Python code is clean, logic is correct, and fallback behavior is well-handled.
eu_einvoice/european_e_invoice/custom/test_sales_invoice.py New test file covering 8 naming scenarios including empty patterns, field interpolation, hash/series segments, slash sanitization, and fallback to doc.name.
eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.json Contains unresolved Git merge conflict markers making the file invalid JSON; Frappe cannot parse this DocType definition, breaking the entire E Invoice Settings loading path.
eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.py Adds auto_name_format_for_xml_file: DF.Data
eu_einvoice/locale/de.po Contains unresolved Git merge conflict markers at multiple locations, making the file invalid gettext format and breaking German translations for the module.
eu_einvoice/locale/main.pot Contains unresolved Git merge conflict markers (same backport conflict as de.po), making the template file unparseable by Babel/gettext tooling.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[download_xrechnung / _attach_xml_file] --> B[get_xml_attachment_file_base_name]
    B --> C{pattern provided?}
    C -- No --> D[frappe.get_single_value E Invoice Settings]
    D --> E{pattern empty or whitespace?}
    C -- Yes --> E
    E -- Yes --> F[get_safe_file_name doc.name]
    E -- No --> G[parse_naming_series with _no_series_counter]
    G --> H{resolve succeeded?}
    H -- Exception --> I[log_error] --> F
    H -- Empty string --> F
    H -- Non-empty base --> J[get_safe_file_name base]
    J --> K[base_name + .xml]
    F --> K
Loading

Fix All in Cursor

Reviews (1): Last reviewed commit: "feat: implement XML attachment file nami..." | Re-trigger Greptile

Comment thread eu_einvoice/european_e_invoice/doctype/e_invoice_settings/e_invoice_settings.json Outdated
Comment thread eu_einvoice/locale/de.po Outdated
@dafrose dafrose force-pushed the mergify/bp/version-15-hotfix/pr-253 branch from 4ac1f3d to f67eb48 Compare May 15, 2026 12:03
dafrose and others added 2 commits May 15, 2026 14:12
* feat(eu_einvoice): configurable base name for auto-attached XRechnung XML

Add optional pattern on E Invoice Settings (auto_name_format_for_xml_file) for the
file base name when auto-attaching XML on Sales Invoice submit. Empty pattern keeps
using the document name; otherwise use naming-series-style segments (dot-separated,
parse_naming_series, {field} placeholders, date tokens), strip leading # per
segment so counter-only parts do not consume Series, sanitize with get_safe_file_name,
and fall back to the document name on error.

Colocate get_xml_attachment_file_base_name with Sales Invoice custom logic and cover
naming edge cases in test_sales_invoice.py.

Refresh E Invoice Settings controller and de / template catalogs for the new strings.

* refactor(eu_einvoice): enhance download_xrechnung to use configurable base name

Update the download_xrechnung function to retrieve the Sales Invoice document
and check permissions before generating the XML file.
The filename now utilizes a configurable base name from E Invoice Settings,
improving flexibility for file naming.
This change ensures that the generated XML file adheres to the specified naming format.

* chore(eu_einvoice): update E Invoice Settings and localization files

Rearrange fields in e_invoice_settings.json for better organization, adding a label for
the XML Settings section. Update localization files (de.po and main.pot) to reflect
changes in field names and add new translations for XML Settings.
Adjust POT creation dates for consistency.

* refactor(eu_einvoice): centralize sales invoice xml stem naming

Resolve the downloadable XML filename stem in get_xml_attachment_file_base_name:
read *Auto name format for XML file* from **E Invoice Settings** when the pattern
argument is omitted, accept an optional keyword-only pattern override, and build
the stem with parse_naming_series plus a no-op number generator so naming has no
series counter DB side effects. Sanitize with get_safe_file_name and fall back to
doc.name when the pattern is empty or fails.

Call sites use the helper without duplicating settings reads. Tests pass pattern=
for empty or whitespace patterns, and assert the settings-backed path by patching
frappe.get_single_value only for **E Invoice Settings** / auto_name_format_for_xml_file.

---------

Co-authored-by: Daniel Rose <26166128+dafrose@users.noreply.github.com>
(cherry picked from commit d45a1f0)
Co-authored-by: Cursor <cursoragent@cursor.com>
Frappe v15 does not expose get_safe_file_name on file utils; mirror the v16+
implementation beside get_xml_attachment_file_base_name as _get_safe_file_name.
@dafrose dafrose force-pushed the mergify/bp/version-15-hotfix/pr-253 branch from f67eb48 to 3ed4c2e Compare May 15, 2026 12:21
@dafrose dafrose requested a review from barredterra May 15, 2026 12:25
@dafrose
Copy link
Copy Markdown
Member

dafrose commented May 15, 2026

Conflicts are resolved and I added a _get_safe_filename local helper, because it did not exist in frappe v15.

@dafrose dafrose removed the conflicts label Jun 5, 2026
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.

1 participant