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

fix: double future qty updates #30913

Merged
merged 3 commits into from May 10, 2022
Merged

Conversation

ankush
Copy link
Member

@ankush ankush commented May 6, 2022

image

This is an edge case, where if you have transactions like these then canceling the middle transaction results in double processing of future quantity.

The incorrect data resolves itself in some time once the backdated entry is processed.

This is because:

  1. Future qty for those SLE with same posting time are updated in update_entries_after

Here all SLE that match posting date time (after removing milliseconds) are processed.

def get_sle_against_current_voucher(self):
self.args["time_format"] = "%H:%i:%s"
return frappe.db.sql(
"""
select
*, timestamp(posting_date, posting_time) as "timestamp"
from
`tabStock Ledger Entry`
where
item_code = %(item_code)s
and warehouse = %(warehouse)s
and is_cancelled = 0
and timestamp(posting_date, time_format(posting_time, %(time_format)s)) = timestamp(%(posting_date)s, time_format(%(posting_time)s, %(time_format)s))
order by
creation ASC
for update
""",
self.args,
as_dict=1,
)

  1. Same are again updated when doing update future qty.

frappe.db.sql(
"""
update `tabStock Ledger Entry`
set qty_after_transaction = qty_after_transaction + {qty_shift}
where
item_code = %(item_code)s
and warehouse = %(warehouse)s
and voucher_no != %(voucher_no)s
and is_cancelled = 0
and (timestamp(posting_date, posting_time) > timestamp(%(posting_date)s, %(posting_time)s)
or (
timestamp(posting_date, posting_time) = timestamp(%(posting_date)s, %(posting_time)s)
and creation > %(creation)s
)

Since step 1 updates all current posting time SLEs, only strictly newer SLEs should be updated while updating qty.

Both these function calls in this order (i.e. this exact code execution path) ONLY occurs when you are making backdated entry.

TODO:

  • check impact on similar query part for stock reco
    def get_datetime_limit_condition(detail):
    return f"""
    and
    (timestamp(posting_date, posting_time) < timestamp('{detail.posting_date}', '{detail.posting_time}')
    or (
    timestamp(posting_date, posting_time) = timestamp('{detail.posting_date}', '{detail.posting_time}')
    and creation < '{detail.creation}'
    )
    )"""

Unrelated fix:

Sort SLEs while picking the next stock reco entry:
0fb6062#diff-7a6f48ad1157b2728d67c44c2bdb26439c7eec44a4241b915070f63fef695f95R1382

@github-actions github-actions bot added the stock label May 6, 2022
@ankush ankush force-pushed the against_voucher_processing branch from 1f5be88 to 98cb967 Compare May 6, 2022 07:26
@ankush ankush changed the title fix: process only SLEs against actual voucher fix: double future qty updates May 6, 2022
@ankush ankush marked this pull request as ready for review May 6, 2022 08:23
@ankush ankush force-pushed the against_voucher_processing branch from b520f07 to 4e7a92c Compare May 9, 2022 05:57
ankush added 2 commits May 9, 2022 11:28
update_qty_in_future_sle is reprocessing rows which are already
processed by process_sle_against_current_voucher
This hides some information that would otherwise help during debugging
@ankush ankush force-pushed the against_voucher_processing branch from 4e7a92c to 0fb6062 Compare May 9, 2022 05:59
@ankush
Copy link
Member Author

ankush commented May 9, 2022

unrelated test_tax_rule failing (on like 80-90% PRs now, its time has come 🥲 #30934 )

@ankush ankush merged commit d258413 into frappe:develop May 10, 2022
@ankush ankush deleted the against_voucher_processing branch May 10, 2022 09:16
ankush added a commit that referenced this pull request May 10, 2022
…-30913

fix: double future qty updates (backport #30913)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants