Skip to content

fix (Business Trip): reset date picker contraints on refresh#120

Open
Alex9779 wants to merge 1 commit into
alyf-de:version-15-hotfixfrom
Alex9779:Alex9779-patch-1
Open

fix (Business Trip): reset date picker contraints on refresh#120
Alex9779 wants to merge 1 commit into
alyf-de:version-15-hotfixfrom
Alex9779:Alex9779-patch-1

Conversation

@Alex9779

Copy link
Copy Markdown

No description provided.

@greptile-apps

greptile-apps Bot commented May 20, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a bug where datepicker minDate/maxDate constraints were not re-applied when an existing Business Trip document was loaded or the form was refreshed, leaving both pickers unconstrained relative to each other after a page load.

  • Adds two guarded datepicker.update() calls in the refresh hook to restore the maxDate on from_date and the minDate on to_date from the current document values, using optional chaining (?.datepicker) to safely skip the update if the widget is not yet initialised.
  • The existing field-change handlers (from_date, to_date) that perform the same constraint update still call .datepicker.update() directly without the guard, creating a minor inconsistency with the new pattern.

Confidence Score: 4/5

Safe to merge; the change is small and the optional-chaining guard prevents any new crash paths.

The fix correctly resets date constraints on refresh and the optional chaining prevents errors when the datepicker hasn't initialised. The existing field-change handlers lack the same guard, but they fire after full render so the practical risk is low. Only minor style and consistency observations remain.

No files require special attention; the single changed file has a small, well-scoped fix.

Important Files Changed

Filename Overview
erpnext_germany/erpnext_germany/doctype/business_trip/business_trip.js Adds datepicker constraint initialization in the refresh hook with optional chaining guards; trailing whitespace on line 25

Sequence Diagram

sequenceDiagram
    participant User
    participant FrappeForm as Frappe Form
    participant DatePicker as DatePicker Widget

    User->>FrappeForm: Open / Refresh Business Trip
    FrappeForm->>FrappeForm: refresh(frm) fires
    FrappeForm->>DatePicker: "from_date.datepicker.update({ maxDate: to_date || null })"
    FrappeForm->>DatePicker: "to_date.datepicker.update({ minDate: from_date || null })"
    Note over DatePicker: Constraints restored on every refresh

    User->>FrappeForm: Change from_date
    FrappeForm->>FrappeForm: from_date handler fires
    FrappeForm->>DatePicker: "to_date.datepicker.update({ minDate: from_date || null })"

    User->>FrappeForm: Change to_date
    FrappeForm->>FrappeForm: to_date handler fires
    FrappeForm->>DatePicker: "from_date.datepicker.update({ maxDate: to_date || null })"
Loading

Comments Outside Diff (1)

  1. erpnext_germany/erpnext_germany/doctype/business_trip/business_trip.js, line 38-46 (link)

    P2 Inconsistent datepicker guard in field-change handlers

    The existing from_date (line 38) and to_date (line 44) handlers call .datepicker.update(...) directly without the optional-chaining guard (?.datepicker) introduced in this PR. If the datepicker were ever not yet initialised when those handlers fire (e.g., programmatic field changes triggered before full render), they would throw a TypeError. Adding the same ?.datepicker guard here would make the defensive pattern consistent across all three call sites.

    Fix in Cursor

Fix All in Cursor

Reviews (1): Last reviewed commit: "fix (Business Trip): reset date picker c..." | Re-trigger Greptile

Comment on lines +24 to 26
}

if (frm.doc.docstatus === 1) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Trailing whitespace on this blank line — worth removing to keep the diff clean.

Suggested change
}
if (frm.doc.docstatus === 1) {
}
if (frm.doc.docstatus === 1) {

Fix in Cursor

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