Multiple problems with Payroll

I think the pain point is limited to using gross_year_to_date in the calculations. I agree this is not a bug, but could be a feature request.

I will try to solve this issue with a custom app:

  1. Create a setting for payroll settings “do_not_include_current_earnings_in_ytd”.
  2. Create an override in the Salary Slip calculations to only use submitted salary slips (excluding current) when determining gross_ytd
  3. When I need to include gross_pay in gross_ytd, I’ll add it manually in the formula (gross_year_to_date + gross_pay).

If I get that working, I’ll suggest a feature request to help others with this struggle.

It could certainly be done, and I had a working demo at one point, but the maintainers decided to move in a different direction.

There’s some discussion here: Feature Request: Multiple Income Tax Slab components · Issue #111 · frappe/hrms · GitHub

Hmm…creating a setting like that is going to break a lot of reports out there, and I don’t see how it solves the issue here.

If you want to make gross_year_to_date more useful without re-validation hacks, why not just refactor the validation method to calculate ytds after earnings but before deductions?

We need to gross_year_to_date in earnings components as well (any employer imposed taxes need to go in earnings components, with matching deduction components).

I haven’t investigated yet, but I don’t see how it would break reports. Reports should be based on submitted salary slips and salary detail, no? If the functions are called outside of Salary Slip CRUD operations, then that would be problematic.

Would it be possible to create another variable that references gross_year_to_date which specifically excludes the additions in the current document?

1 Like

If a salary slip has a field called “Gross Year To Date”, I think most people would interpret that value to include the current pay period. Any reports that draw on this field will suddenly not know what’s actually being represented.

Like @trustedcomputer suggests, far better to create a new field than to make the meaning of an existing one ambiguous.

My 2 cents, the gross_year_to_date includes the current slip gross, but only after the Save, since we know this is the intended behaviour, could gross_year_to_date not be calculated on change of the earnings? Like when an employee is added and earnings created, or when the date is changed, whatever triggers the earning, should trigger the gross_year_to_date to be updated and not wait for thew save.

And this is what led me here in the first place, having a change gross_year_to_date when it should be accurate as soon as the earnings are appended.

just a thought.

Hmm…I’m not sure I’m following. The gross_year_to_date field is calculated server-side during the document’s validation event (shortly after earning components are calculated). Under normal circumstances, earnings are created in response to the document being saved, which is also what causes the year to date totals to be calculated as well. Nothing is happening after the save. It’s all part of the save event processing.

Referring to a previous post—here’s what I’m observing when processing a Salary Slip:

Scenario:

Salary Component Setup:

  • Basic = base
  • FEDIT = gross_year_to_date (custom deduction)

Steps:

  1. Create a new Salary Slip.
  2. Add Frequency, Employee, Dates, etc.
  3. Basic earnings of $5000 are added.
  4. At this point, gross_year_to_date is still 0, so FEDIT = 0.

After Saving:

Suggestion:

Why not trigger the gross_year_to_date calculation immediately after earnings are added, or as soon as the employee and period are selected?

If that’s not feasible due to system design, then:

Use the Save function to calculate gross_year_to_date first, then evaluate deductions (like FEDIT) afterwards.
This would ensure consistent and accurate data before Submit—eliminating any discrepancies due to out-of-order evaluations.

I’m not a Frappe expert, but from a logical perspective, this sequencing issue causes deductions that depend on YTD values to miscalculate—until Submit, which is risky as users might not realize changes are happening post-Save/Submit.

Would appreciate feedback or a workaround from the community.

I think your example problem with FEDIT not calculating properly is a problem with either the salary component or the formula that uses it. If your goal is to have accurate salary slips, that’s where the attention needs to be pointed to.

EDIT: @volkswagner provided a good starting point for the formula here:

0 if ((gross_year_to_date - BS) >= 71300 or (gross_year_to_date - BS) <= 3500) else 
0.0595 * (71300 - (gross_year_to_date - BS)) if gross_year_to_date > 71300 else
0.0595 * (gross_year_to_date - 3500) if (gross_year_to_date >= 3500 and (gross_year_to_date - BS) <= 3500) else
0.0595 * BS

I think with some tinkering, it should work real-world when it’s integrated into a Salary Component, Salary Structure, Salary Structure Assignment, and consumed by a Payroll Entry.

Right now I have 2 salary components in my salary structure (an earning and a deduction), and 1 salary structure assignment. Just for testing purposes.

Here’s a screen capture of each, you can see it’s pretty simple, just trying to get my feet wet, and getting the sequencing right.
As you can see from my before save and after save, something is off.
I just want to understand why, is it me, is the system, what is it??

Any assistance is appreciated, I really do thank everyone for chiming in here so far.








@jroyDC We’ve already proved that submitting is necessary to get the calculations based on gross_year_to_date. You will need a customization if you want to see the results in a saved Salary Slip.

Submitting shouldn’t be necessary. Just a second call to validate.

You can create a second validation by manually editing any component that is based on a formula > press save > you’ll see the new calculations. If we are required to perform such a workaround, I would consider it a bug.

Some of the specifics here don’t fit with how Frappe’s document event cycle works, but I get what you’re saying in broad terms. One of your formulas is trying to use a variable before it has been calculated, and that’s leading to unexpected results. It’s not about Save versus Submit, though. In Frappe terms, what you’re suggesting is that the gross_year_to_date should be calculated earlier in the cycle so that it’s available to formulas on the first pass.

That’s possible for deductions but not for earnings. The trade-off is just complexity. Frappe usually tries to avoid hardcoding use-case-specific solutions in favor of generic ones, but the value of making gross_year_to_date available to deductions seems high enough that it may be worth it here. The generic solution – conditional recalculation of components in response to dependencies – is both computationally intensive and very messy.

There’s definitely a larger structural limitation on using gross_year_to_date in formulas. This isn’t something that tinkering will fix.

Literally any save will update the values. Editing a component based on a formula isn’t required.

As for calling this a bug, I can definitely see why you’d think of it that way. The behavior is clearly not what most people are expecting, and it’s possible to end up with incoherent results. With user-defined scripts and formulas, though, it’s always going to be possible for people to do things that break pre-existing doctype logic. The question is how much effort should Frappe take to install guard-rails against that.

In general, I personally think it’s better for users to understand that circular dependencies will always need special handling. The need for gross_year_to_date in deductions might be common enough, however, that it’s worth hardcoding a fix. But, if you need gross_year_to_date in earnings too, that won’t help you.

I agree with you. Using gross_year_to_date in the formula worked for me and I never noticed the glitch @jroyDC did because I never looked too closely at the document flow. If you use the Payroll Entry workflow, you never even see the Salary Slips until they are submitted. And since the computations were always correct in the submitted document, all was well that ended well.

But after all this I agree that gross_year_to_date is not the “right” way to do this. There is no problem with the document life cycle. The problem is in the “need” to use gross_year_to_date, since there is no variable we can use in the formula that represents the YTD earnings before the current document. Even a quick look at how I’m using it makes that obvious, since I immediately need to subtract the current earnings from it.

Reviewing salaryslip.py gave me an idea on how to do this correctly. Starting on line 1610 is the get_taxable_earnings_for_prev_period function. I stumbled upon it because that function is used to compute gross_year_to_date, taking previous earnings and adding on the current ones (at least that’s my read on it, though I could be wrong).

Assuming that’s how it works, if there were a hidden field in the Salary Slip document referencing the return from that function, then it could be used in payroll formulae and we wouldn’t be talking about how documents work on a nuts and bolts level at all. It would be a simple matter of including that variable in formulae and there would be no difference in values at different document stages.

1 Like

I have nothing against people creating a previous_gross_year_to_date field or whatever if it’d be helpful.

I still think folks are making this waaay more complicated than it needs to be, though. Formulas are convenient for simple things, but forcing them to work through circular references is just always going to be fragile and complicated.

The correct answer here, I am 100% convinced, is to pass calculations for complex cases to an external script run during the before_save event. At that point, the validation method has already been run once, and it’s possible to do literally anything that python can handle: complex YTD calculations, multiple slab-based components, annual caps, values that draw on external data sources, etc.

Here’s a very simple example. In one of my jurisdictions, I need to withhold a tax equal to 1% of gross income, but there’s an annual limit to the tax of 5000. Doing this with formulas would be very messy, running into many of the same issues described here.

It’s trivial to do with a short Server Script:

# before_save
for deduction in doc.deductions:
    if deduction.salary_component == "Social Security Tax":
        base_tax = doc.gross_pay * 0.01
        cap_left = 5000 - deduction.year_to_date
        
        deduction.amount = min(base_tax, cap_left)
        doc.validate()

Here, Social Security Tax is just an empty component with 0 amount. After all other Salary Slip values have been posted, the script calculates the right amount for the tax, sets it, and triggers validation again to recalculate computed values. There’s no need for formulas. It’s easy to troubleshoot and the sky is the limit.

1 Like

Once the Salary slip is saved, the save button is hidden. Pressing cmd+S will submit. The user must edit the document to allow saving a second time. I suggested
editing a field generated by a formula, so it would be recalculated (essentially undoing the edit).

I believe there was a design choice that could have considered how important gross_year_to_date is.

Since gross_year_to_date is a Salary Slip field, the issue arises. There should be a provision to fetch “previous_gross_year_to_date”.

I think I’ll try to add a field to Salary Slip for prev_gross_ytd. This would solve the circular dependency.

1 Like

With minimal testing, here’s what I’ve come up with (help from ChatGPT).
@jroyDC please give this a try to see if it helps your workflow.

Customize Salary slip, add field (currency, name “Prev Gross YTD”) it should create a field custom_prev_gross_ytd. After saving, you can change the display name to Previous Gross YTD, for better readability.

Create a server script that’s activated before_insert.

# Set only if not already set (so it doesn't change on updates or submit)
if not doc.custom_prev_gross_ytd:
    last_slip = frappe.db.get_all(
        "Salary Slip",
        filters={
            "employee": doc.employee,
            "docstatus": 1,
            "start_date": ["<", doc.start_date]
        },
        fields=["gross_year_to_date"],
        order_by="start_date desc",
        limit=1
    )
    doc.custom_prev_gross_ytd = last_slip[0].gross_year_to_date if last_slip else 0

I used the following formula for FUTA:

0 if ((custom_previous_gross_ytd) >= 7000) else 
0.006 * (7000 - (custom_previous_gross_ytd)) if ((custom_previous_gross_ytd + B) > 7000) else 
0.006 * B

I’m unable to get the custom_prev_gross_ytd to populate as early as the salary components are first calculated.

I don’t know why frappe has the components calculated before save or why I can’t populate custom_prev_gross_ytd as at the same time or before calculations are performed before pressing save (after selecting the employee).

I still have to test this with Payroll Entry workflow.

Thank you, @volkswagner. I’ve been experimenting with a custom field as well and seem to be on a similar track to yours. I’ve also tried using a custom server script and hooks.py to trigger updates on insert or validate events—aiming to calculate the gross and apply the deduction formula before the document is submitted.

I’m confident I’ll get it working eventually, and when I do, I’ll be sure to share my results with the forum.

Thanks again to everyone who contributed to this discussion. I understand and respect the various perspectives shared, but I still firmly believe that the values posted to the General Ledger should be accurate after a Save and before a Submit. From an accountant’s point of view, I think we can all agree on the importance of that.