fix(page-data-script): make translation actually callable as _(...)

[suparikoli]

Fixes #626.

The issue's diagnosis is right and I ran it down the same way to be sure before touching anything: frappe._("Supplier") in a Page Data Script never gets anywhere near frappe._. frappe._ is an attribute read, and RestrictedPython compiles that to _getattr_(frappe, "_"). That guard function is _getattr_for_safe_exec in frappe/utils/safe_exec.py, and it calls _validate_attribute_read, which rejects any attribute name starting with "_" unconditionally, before it even looks at what the attribute would resolve to:

def _validate_attribute_read(object, name):
    ...
    if name.startswith("_"):
        raise AttributeError(f'"{name}" is an invalid attribute name because it starts with "_"')

So nesting _=frappe._ differently inside the frappe namespace wouldn't have changed anything - frappe._(...) can't work regardless of where _ is defined, because it's the attribute access itself that's rejected, not a lookup failure.

A bare name reference (_("Supplier"), no dot) compiles to a plain global lookup, which doesn't go through _getattr_ at all - RestrictedPython only wraps attribute access, not name resolution. That's exactly what frappe's own render_safe_globals() (the base sandbox builder's get_safer_globals() extends) already relies on: it puts _=frappe._ at the top level of out, not under out.frappe. Builder's version has it nested one level down, which is the actual gap. Moved it to match.

Added a test in test_utils.py alongside the existing execute_script ones: runs data.text = _("Supplier") through execute_script() and checks it resolves to "Supplier" (no translation is loaded for that string in a test run, so it comes back unchanged, which is expected), and separately asserts that frappe._("Supplier") still raises - want that locked in so nobody "fixes" this again later by moving _ back under frappe, which would look reasonable but doesn't actually work.

Ran ruff check and ruff format --check against both changed files - clean. Didn't have a live site to run the actual test against; the reasoning above is what I'm relying on instead, traced against current frappe/utils/safe_exec.py and render_safe_globals() rather than assumed.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

_{Reviews (2): Last reviewed commit: "chore: trim oversized comments" | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.20%. Comparing base (38d68fd) to head (9772661).
⚠️ Report is 424 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #745      +/-   ##
===========================================
+ Coverage    66.05%   71.20%   +5.14%     
===========================================
  Files           37       63      +26     
  Lines         4982     7320    +2338     
===========================================
+ Hits          3291     5212    +1921     
- Misses        1691     2108     +417     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[surajshetty3416]

Automated review (Claude Code)

Change looks correct. _ is the one underscore-prefixed name RestrictedPython's check_name explicitly permits, and frappe's _getattr_ guard does reject the frappe._ attribute path at runtime, so both the fix and the comment are accurate. Exposing frappe._ adds no new capability beyond translation lookup.

Two minor test nits (non-blocking):

• builder/builder/tests/test_utils.py:143 — _("Supplier") only equals "Supplier" when the test site's language is en. If a site under test has a non-English default lang with a Supplier translation, this breaks. Wrapping in frappe.set_user_lang/with frappe.change_site_config-style lang pinning, or asserting on a string with no translation entry, would make it deterministic.
• builder/builder/tests/test_utils.py:147 — assertRaises(Exception) is broad enough to pass for the wrong reason (e.g. if frappe ever stopped being in globals, you'd get a NameError and the test would still be green). assertRaises(SyntaxError) matches what the guard actually raises and pins the intended behavior.

---

This is an automated review and may be wrong — generated by a Claude Code devbox, not a human.