Skip to content

fix: update system date format for password in salary slip - #5092

Open
nareshkannasln wants to merge 2 commits into
frappe:developfrom
aerele:password-salary-slip
Open

fix: update system date format for password in salary slip#5092
nareshkannasln wants to merge 2 commits into
frappe:developfrom
aerele:password-salary-slip

Conversation

@nareshkannasln

Copy link
Copy Markdown
Contributor

Closes: #1941

Description: The password policy in Payroll Settings substitutes Employee fields into a template, e.g. SAL-{first_name}-{date_of_birth}. Date fields were interpolated as raw datetime.date objects, which always stringify as ISO, so the password contained 1990-05-08 regardless of the date format configured in System Settings.
An employee whose system is set to dd-mm-yyyy is told the password is their date of birth, tries 08-05-1990, and cannot open the salary slip PDF.

@nareshkannasln
nareshkannasln marked this pull request as ready for review August 13, 2026 10:08
@mergify

mergify Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 4/5

The change appears safe to merge after the non-blocking documentation omission is addressed.

The date-format regression is covered, and the only accepted issue is the missing contract documentation for the new public helper.

Files Needing Attention: hrms/payroll/doctype/salary_slip/salary_slip.py

Fix All in Greploop

Fix All in Claude Code Fix All in Codex

Reviews (1): Last reviewed commit: "test(salary-slip): add password uses sys..." | Re-trigger Greptile

)


def format_dates_in_system_format(value):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Document the date-format helper

format_dates_in_system_format is a new public helper without a docstring, leaving its system-format conversion contract implicit for future callers and maintainers.

Suggested change
def format_dates_in_system_format(value):
def format_dates_in_system_format(value):
"""Wrap date values so string interpolation uses the system format."""

Context Used: Guidelines for reviewing Frappe Framework applicat... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex

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.

password at salary slip

1 participant