feat: add optional PDF Button for Sales and Purchase Doctypes

[HenningWendtland]

Usecase:

PDF on Submit tries to minimize the clicks needed to create and attach a PDF. In most cases users want to see the PDF as a form of Preview before submission though. Since the PDF On Submit Settings allow now multiple prints with filters and differ from the ERPNext default print it can become cumbersome for the User to go to "Print -> Select the correct format and letterhead -> Press PDF Button"

Solution:

Optionally add a simple "PDF" Button for all enabled Doctypes

Features of the Button:

• Fetches first Print Format in PDF on Submit settings that matches, or falls back to ERPNext Default Format in case filters don't match any format
• Opens a popup with the print, similar to PDF Button in print preview
• Add a Checkbox in PDF On Submit Settings to toggle the feature
• Check Print Permissions

PDF on Submit Settings:

Button in Form:

Tested

• Button appears on correct Doctypes
• Correct PDF is displayed
• Toggle works as expected

[Copilot]

Pull request overview

This PR adds an optional PDF download button for sales and purchase document types in ERPNext. The feature respects PDF on Submit Settings configurations and falls back to default ERPNext print formats when appropriate.

Changes:

• Added a new whitelisted Python utility function to retrieve print format and letterhead settings for documents
• Implemented a JavaScript utility that adds a PDF button to common sales/purchase doctypes (Quotation, Sales Order, Invoice, etc.) when enabled via settings
• Added a new checkbox field "Show PDF Button in Sales and Purchase DocTypes" to PDF on Submit Settings
• Extracted get_matching_enabled_doctype function from existing code for reuse
• Updated translation files (main.pot and de.po) with new UI strings

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
pdf_on_submit/utils.py	New Python utility module providing whitelisted API to fetch print details (format and letterhead) for documents
pdf_on_submit/public/js/pdf_button_utils.js	JavaScript utility that registers PDF button handlers for sales/purchase doctypes and handles PDF generation via API calls
pdf_on_submit/pdf_on_submit/doctype/pdf_on_submit_settings/pdf_on_submit_settings.json	Added new checkbox field and column break to enable/disable the PDF button feature
pdf_on_submit/locale/main.pot	Updated translation template with new strings for the PDF button and related messages
pdf_on_submit/locale/de.po	Updated German translations for new UI elements and messages
pdf_on_submit/hooks.py	Added JavaScript file inclusion via both app_include_js and doctype_js hooks
pdf_on_submit/attach_pdf.py	Extracted get_matching_enabled_doctype function for reuse in the new utils module

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[HenningWendtland]

Review Summary

(AI-generated review) This is a well-implemented feature with clean code! I've analyzed the implementation and identified a few security improvements that would strengthen it before merging.

CI Status

✅ All checks passing (CodeQL, JavaScript analysis, Python analysis)

PR Description

The description is accurate but quite minimal. Consider adding:

• What the feature does (adds optional PDF button to sales/purchase forms)
• How it works (calls whitelisted method to get print details, opens PDF in new tab)
• How to enable it (via PDF on Submit Settings checkbox)

What Works Well

• Clean implementation following existing Frappe patterns
• Permission check with frappe.has_permission() before document access
• Safe URL construction using URLSearchParams
• German translations properly added to .po file
• Good error handling with user-friendly messages
• Proper check for new documents (frm.is_new())
• Follows conventional commit format

Main Concerns

1. Missing Server-Side DocType Allowlist (High Priority)

The whitelisted method in utils.py accepts any arbitrary doctype parameter. While there's a client-side ALLOWED_DOCTYPES list in JavaScript, this provides no server-side protection. An attacker could call the API directly with any doctype they have print permission for.

Current code (utils.py:6-7):

@frappe.whitelist()
def get_print_details(doctype: str, docname: str) -> tuple:

Issue: Someone could call:

frappe.call({
    method: "pdf_on_submit.utils.get_print_details",
    args: {doctype: "User", docname: "Administrator"}
})

Suggested fix: Add server-side doctype validation:

ALLOWED_DOCTYPES = [
    "Quotation", "Sales Order", "Sales Invoice", "Delivery Note", "Dunning",
    "Request for Quotation", "Supplier Quotation",
    "Purchase Order", "Purchase Invoice", "Purchase Receipt",
]

@frappe.whitelist()
def get_print_details(doctype: str, docname: str) -> tuple:
    if doctype not in ALLOWED_DOCTYPES:
        frappe.throw(
            _("PDF button not available for doctype {0}").format(doctype),
            frappe.PermissionError
        )
    # ... rest of function

2. Missing Server-Side Setting Validation (Medium Priority)

The show_pdf_button setting is only checked client-side (pdf_button_utils.js:36-40). Users can bypass this by calling the whitelisted method directly via browser console or API.

Suggested fix: Add server-side check in utils.py:

@frappe.whitelist()
def get_print_details(doctype: str, docname: str) -> tuple:
    # ... doctype validation ...
    
    # Check if feature is enabled
    show_button = frappe.db.get_single_value("PDF on Submit Settings", "show_pdf_button")
    if not show_button:
        frappe.throw(_("PDF button is currently disabled"), frappe.ValidationError)
    
    # ... rest of function

3. No HTTP Method Restriction (Medium Priority)

The whitelisted method doesn't specify allowed HTTP methods, making it accessible via GET requests. While this is a read-only operation, best practice is to use POST for methods that require authentication and return user-specific data.

Suggested fix:

@frappe.whitelist(methods=["POST"])
def get_print_details(doctype: str, docname: str) -> tuple:
    # ... function body

Additional Suggestions (Optional)

1. Input validation: Consider validating input lengths (doctype/docname max 140 chars per Frappe standards)
2. Rate limiting: Consider adding @rate_limit() decorator to prevent abuse
3. Security logging: Consider logging permission failures for security monitoring

Testing Recommendations

Please test:

• Direct API calls with unauthorized doctypes
• API calls with show_pdf_button disabled
• User Permission scenarios (users restricted to specific companies/territories)

Let me know if you have questions about any of these suggestions! I think addressing the doctype allowlist (#1) would be the most important improvement.

[HenningWendtland]

@greptileai

[greptile-apps]

Confidence Score: 5/5

Safe to merge — the changes are well-scoped, the permission checks are correct, and the prior review concerns have been addressed.

The popup-before-await pattern is correctly implemented to avoid browser popup blockers. The null-popup guard gives users an actionable message. The server-side get_print_details endpoint is properly gated by db.exists and has_permission. Boot info only exposes enabled_doctypes when the feature is active. The attach_pdf refactoring preserves the original behavior exactly. Test coverage is solid across the new code paths.

No files require special attention.

_{Reviews (6): Last reviewed commit: "refactor: clearer separation" | Re-trigger Greptile}

[HenningWendtland]

@greptileai rereview the PR please

[HenningWendtland]

@greptileai rereview please

[barredterra]

@greptileai

[barredterra]

@greptileai

[barredterra]

@Mergifyio backport version-15

[mergify]

backport version-15

✅ Backports have been created

Details

• #80 feat: add optional PDF Button for Sales and Purchase Doctypes (backport #75) has been created for branch version-15 but encountered conflicts

Cherry-pick of 114c2d1 has failed:

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

You are currently cherry-picking commit 114c2d1.
  (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:   pdf_on_submit/attach_pdf.py
	modified:   pdf_on_submit/hooks.py
	new file:   pdf_on_submit/public/js/pdf_button_utils.js
	modified:   pdf_on_submit/tests/test_pdf_on_submit.py
	new file:   pdf_on_submit/utils.py

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

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