Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(really) Fix #20527 Accountancy Unbalanced entry proposed when an employee are declared on social contribution #30428

Closed
wants to merge 2 commits into from

Conversation

t3cneo
Copy link

@t3cneo t3cneo commented Jul 20, 2024

FIX #20527

Merged PR #21193 doesn't fix the issue at least in latest v19.0.2

@eldy
Copy link
Member

eldy commented Jul 20, 2024

It seems another fix was applied with this patch ca39809 in v18 (and merged into upstream).
Can you describe how to reproduce the bug if you still have it in develop despit this patch ?

@eldy eldy added the Bug or PR need more information This bug or PR needs more information (answer to a question or more accurate description) label Jul 20, 2024
@t3cneo
Copy link
Author

t3cneo commented Jul 20, 2024

I'm experiencing the issue described in #20527 in v 19.0.2 : A social charge attached to a user will result in unbalanced entry.

To my (very little and possibly wrong) understanding, merged PR #21193 was "flawed" : the patch adds a test to detect a social charge and intend to skip user related journal entry but it doesn't because !$is_sc is used instead of $is_sc

I guess that is why someone was surprised the issue was closed back then.

This has been kept since and even more tests have been added from line 339 to line 356 making the condition in line 361 impossible to meet.

@eldy
Copy link
Member

eldy commented Jul 20, 2024

When line is a user line, when for social contribution, expense report or salary payment, the line is supported by the rest of code, thus must din't need to ignore line type user. Management is not implemented for other type this is why there is a continue for any other cases.
Also after fixes merged, it was no more possible to reproduce the bug (meaning bug was fixed).
If there is still a use case known to reproduce the bug, this use case must be described. Currently, with all merged last fix, i did not succeed in reproducing the bug.

@t3cneo
Copy link
Author

t3cneo commented Jul 21, 2024

When line is a user line, when for social contribution, expense report or salary payment

This is not what I understand from line 361

if ($links[$key]['type'] == 'user' && !$is_sc && !$is_salary && !$is_expensereport) {

I understand when line is a user line AND when NOT for social contribution AND NOT expense report AND NOT salary payment which is basically never...

Please correct me if I'm wrong, I'm just a user not a dev.

Anyway, when I create a social charge and attach it to a user :

Copie d'écran_20240721_113717

Here is what I get in accountancy :

Copie d'écran_20240721_113935

This PR fixes this.

It removes expense report and salary payment as they are correctly handled.

EDIT :

Oh sorry I just tried with latest patch from develop and it is indeed fixed by ca39809
I therefore think line 361 to line 365 can be removed as it is useless.

@eldy
Copy link
Member

eldy commented Jul 25, 2024

i will keep line 361 to 365 for few version waiting to be sure everything is correct and stable before trying to remove it to avoid to increase risk of regression...
Thanks for you feedback on the very latest version so i can close the ticket.

@eldy eldy closed this Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug or PR need more information This bug or PR needs more information (answer to a question or more accurate description)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Social charge with user related create unbalanced entry in accounting breakdown
2 participants