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(Quotation): calculate row values for alternative items #43054

Merged
merged 5 commits into from
Oct 3, 2024

Conversation

barredterra
Copy link
Collaborator

@barredterra barredterra commented Sep 4, 2024

Introduced via: #33874

Problem

When marking a Quotation row as Is Alternative, the row is completely ignored in code. E.g. the row amount does not get updated, the tax template is not verified, the tax exclusive rate is no longer determined, etc.

This can easily result in a wrong quotation. For example, if the rate is edited after the row was marked as Is Alternative, the row amount is no longer correct.

Solution

Exclude alternative rows only when calculating total taxes and amounts for the entire Quotation. Individual row values should get updated as usual, for alternative items as well.


Internal ref: DEP-567

@github-actions github-actions bot added accounts needs-tests This PR needs automated unit-tests. labels Sep 4, 2024
@barredterra barredterra requested review from marination and removed request for rohitwaghchaure and deepeshgarg007 September 4, 2024 15:04
@barredterra barredterra added backport version-15-hotfix and removed needs-tests This PR needs automated unit-tests. labels Sep 4, 2024
@barredterra barredterra self-assigned this Sep 24, 2024
@barredterra barredterra added the needs-tests This PR needs automated unit-tests. label Sep 24, 2024
@barredterra barredterra removed the needs-tests This PR needs automated unit-tests. label Oct 2, 2024
@barredterra barredterra enabled auto-merge (squash) October 3, 2024 20:16
@barredterra barredterra merged commit 9872645 into frappe:develop Oct 3, 2024
18 checks passed
mergify bot pushed a commit that referenced this pull request Oct 3, 2024
@barredterra barredterra deleted the fix_alternative_item branch October 4, 2024 00:23
barredterra added a commit that referenced this pull request Oct 4, 2024
…43054) (#43495)

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
fix(Quotation): calculate row values for alternative items (#43054)
RiaanRoetz pushed a commit to epiusegs/erpnext that referenced this pull request Oct 4, 2024
frappe-pr-bot pushed a commit that referenced this pull request Oct 4, 2024
# [15.38.0](v15.37.0...v15.38.0) (2024-10-04)

### Bug Fixes

* 'NoneType' object has no attribute 'has_serial_no' ([21a0157](21a0157))
* add company filter in Warehouse wise Item Balance Age and Value ([4fc6d3e](4fc6d3e))
* adjustmen entry for stock reco ([c551c27](c551c27))
* Cannot read properties of undefined (reading 'price_list_rate') (backport [#43376](#43376)) ([#43377](#43377)) ([47f06dc](47f06dc))
* Data missing in table: None, MandatoryError (backport [#43422](#43422)) ([#43429](#43429)) ([4b3f143](4b3f143))
* **Dunning:** logic for fetching text (backport [#43160](#43160)) ([#43490](#43490)) ([1b28a4e](1b28a4e))
* Fix API endpoint for Frankfurter ([d96cee8](d96cee8))
* Ignore transaction deletion check on ledger entry insertion ([1d6f97a](1d6f97a))
* **Item:** error message on tax rate (backport [#42955](#42955)) ([#42956](#42956)) ([5fc5934](5fc5934))
* last purchase rate for purchase invoice (backport [#43448](#43448)) ([#43452](#43452)) ([ee2c8c8](ee2c8c8))
* negative stock error for batch (backport [#43450](#43450)) ([#43454](#43454)) ([7bf6251](7bf6251))
* patch to update Currency Exchange Settings for `frankfurter.app` (backport [#43481](#43481)) ([#43483](#43483)) ([35a08f8](35a08f8))
* quality inspection creation (backport [#43416](#43416)) ([#43417](#43417)) ([a1b6628](a1b6628))
* **Quotation:** calculate row values for alternative items (backport [#43054](#43054)) ([#43495](#43495)) ([4fa5131](4fa5131))
* removed validation for materials return (backport [#43461](#43461)) ([#43463](#43463)) ([9c0a17e](9c0a17e))
* serial and batch no selector (backport [#43387](#43387)) ([#43390](#43390)) ([74c880c](74c880c))
* set margin fields for purchase documents when updating items ([6516e68](6516e68))
* Stock Ledger Invariant Check report ([2984bad](2984bad))
* Stock UOM not fetched when Stock Entry create from Item Dashboard (backport [#43457](#43457)) ([#43465](#43465)) ([f2a72e5](f2a72e5))
* tests for work order consumption (backport [#41814](#41814)) ([#43430](#43430)) ([86b10ce](86b10ce))
* use serial and batch fields (backport [#43421](#43421)) ([#43423](#43423)) ([d495d93](d495d93))

### Features

* added 'cost of new capitalized asset' column ([27cd51e](27cd51e))
* provide hook point for bulk transaction tasks ([50e47e7](50e47e7))
khalandarsihan pushed a commit to khalandarsihan/erpnext that referenced this pull request Oct 6, 2024
…rappe#43054) (frappe#43495)

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
fix(Quotation): calculate row values for alternative items (frappe#43054)
khalandarsihan pushed a commit to khalandarsihan/erpnext that referenced this pull request Oct 6, 2024
# [15.38.0](frappe/erpnext@v15.37.0...v15.38.0) (2024-10-04)

### Bug Fixes

* 'NoneType' object has no attribute 'has_serial_no' ([21a0157](frappe@21a0157))
* add company filter in Warehouse wise Item Balance Age and Value ([4fc6d3e](frappe@4fc6d3e))
* adjustmen entry for stock reco ([c551c27](frappe@c551c27))
* Cannot read properties of undefined (reading 'price_list_rate') (backport [frappe#43376](frappe#43376)) ([frappe#43377](frappe#43377)) ([47f06dc](frappe@47f06dc))
* Data missing in table: None, MandatoryError (backport [frappe#43422](frappe#43422)) ([frappe#43429](frappe#43429)) ([4b3f143](frappe@4b3f143))
* **Dunning:** logic for fetching text (backport [frappe#43160](frappe#43160)) ([frappe#43490](frappe#43490)) ([1b28a4e](frappe@1b28a4e))
* Fix API endpoint for Frankfurter ([d96cee8](frappe@d96cee8))
* Ignore transaction deletion check on ledger entry insertion ([1d6f97a](frappe@1d6f97a))
* **Item:** error message on tax rate (backport [frappe#42955](frappe#42955)) ([frappe#42956](frappe#42956)) ([5fc5934](frappe@5fc5934))
* last purchase rate for purchase invoice (backport [frappe#43448](frappe#43448)) ([frappe#43452](frappe#43452)) ([ee2c8c8](frappe@ee2c8c8))
* negative stock error for batch (backport [frappe#43450](frappe#43450)) ([frappe#43454](frappe#43454)) ([7bf6251](frappe@7bf6251))
* patch to update Currency Exchange Settings for `frankfurter.app` (backport [frappe#43481](frappe#43481)) ([frappe#43483](frappe#43483)) ([35a08f8](frappe@35a08f8))
* quality inspection creation (backport [frappe#43416](frappe#43416)) ([frappe#43417](frappe#43417)) ([a1b6628](frappe@a1b6628))
* **Quotation:** calculate row values for alternative items (backport [frappe#43054](frappe#43054)) ([frappe#43495](frappe#43495)) ([4fa5131](frappe@4fa5131))
* removed validation for materials return (backport [frappe#43461](frappe#43461)) ([frappe#43463](frappe#43463)) ([9c0a17e](frappe@9c0a17e))
* serial and batch no selector (backport [frappe#43387](frappe#43387)) ([frappe#43390](frappe#43390)) ([74c880c](frappe@74c880c))
* set margin fields for purchase documents when updating items ([6516e68](frappe@6516e68))
* Stock Ledger Invariant Check report ([2984bad](frappe@2984bad))
* Stock UOM not fetched when Stock Entry create from Item Dashboard (backport [frappe#43457](frappe#43457)) ([frappe#43465](frappe#43465)) ([f2a72e5](frappe@f2a72e5))
* tests for work order consumption (backport [frappe#41814](frappe#41814)) ([frappe#43430](frappe#43430)) ([86b10ce](frappe@86b10ce))
* use serial and batch fields (backport [frappe#43421](frappe#43421)) ([frappe#43423](frappe#43423)) ([d495d93](frappe@d495d93))

### Features

* added 'cost of new capitalized asset' column ([27cd51e](frappe@27cd51e))
* provide hook point for bulk transaction tasks ([50e47e7](frappe@50e47e7))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant