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

[16.0][MIG] sale_order_price_recalculation #2778

Merged

Conversation

CRogos
Copy link
Contributor

@CRogos CRogos commented Nov 16, 2023

supersede: #2605

Has anyone an idea what is wrong with the tests? Is this related to the hard disk swap outage?

@CRogos CRogos marked this pull request as ready for review November 16, 2023 21:05
@CRogos CRogos closed this Nov 16, 2023
@CRogos CRogos reopened this Nov 16, 2023
@CRogos CRogos closed this Nov 17, 2023
@CRogos CRogos reopened this Nov 17, 2023
@CRogos CRogos closed this Nov 17, 2023
@CRogos CRogos reopened this Nov 17, 2023
@CRogos CRogos force-pushed the 16.0-mig-sale_order_price_recalculation branch 6 times, most recently from 7ee7ff7 to 7bea749 Compare November 17, 2023 13:25
@CRogos CRogos closed this Nov 18, 2023
@CRogos CRogos reopened this Nov 18, 2023
@CRogos
Copy link
Contributor Author

CRogos commented Nov 18, 2023

Has anyone an idea why this test is failing? When I run it locally the tests passes and also the module is working fine as far as I can say.

@CRogos CRogos mentioned this pull request Nov 20, 2023
100 tasks
Copy link

@MohamedOsman7 MohamedOsman7 left a comment

Choose a reason for hiding this comment

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

code & functional changes LGTM

@CRogos CRogos closed this Nov 29, 2023
@CRogos CRogos reopened this Nov 29, 2023
pedrobaeza and others added 8 commits November 29, 2023 10:39
…n demand sale order line prices

==================================================
Recalculation of sale order lines prices on demand
==================================================

This module adds a button on sale orders below pricelist that recalculates the
prices of the order lines that contain a product in them.

It is launched manually as a button to get the user decide if he/she wants to
recalculate prices when pricelist is changed.

Usage
=====

Inside a sale order, you can click in any moment a button called
"(Recalculate prices)", that is next to the pricelist selection, to launch
a recalculation of all the prices of the lines, losing previous custom prices.

Known issues / Roadmap
======================

* In a sale order with lot of lines, the recalculation may slow down, because
  sale general data (amount untaxed, amount taxed...) are recalculated for
  each line.
* add choice to recompute price only
  This commit add the capability to reset sale order line
  without renaming sale order lines
* upgrade translation
* Split functionnalities reset prices/reset desciptions
* Move button under sale order lines
On certain set of installed modules, standard demo pricelist can be altered for giving
something different than the list price, making these tests to fail. We avoid it
creating our own pricelist with the fixed data that we want.
pedrobaeza and others added 4 commits November 29, 2023 10:39
With current inheritance, the buttons are only visible if you have the
security group "Lock Confirmed Sales" in your user.

As we need to be seen in all cases, we modify the inheritance.

TT37124
@CRogos CRogos force-pushed the 16.0-mig-sale_order_price_recalculation branch from 7bea749 to f77c300 Compare November 29, 2023 10:39
@CRogos
Copy link
Contributor Author

CRogos commented Nov 29, 2023

Is the recalculate price obsolete? When switching the price list, there is an update button:
image

@CRogos CRogos force-pushed the 16.0-mig-sale_order_price_recalculation branch from f77c300 to 2f68126 Compare December 6, 2023 09:53
@CRogos CRogos force-pushed the 16.0-mig-sale_order_price_recalculation branch from 2f68126 to 142f0bd Compare December 6, 2023 10:02
@CRogos
Copy link
Contributor Author

CRogos commented Dec 11, 2023

@arleite , @rousseldenis could you review this migration?

@CRogos
Copy link
Contributor Author

CRogos commented Dec 12, 2023

@arleite can you also add yourself to the reviews?
image

Copy link

@andrel-exo andrel-exo left a comment

Choose a reason for hiding this comment

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

LGTM

@CRogos
Copy link
Contributor Author

CRogos commented Dec 12, 2023

@arleite. I should have been more precise. You have to select "approve", not "comment".

Thank you.

@CRogos
Copy link
Contributor Author

CRogos commented Dec 14, 2023

@pedrobaeza could you merge?

@pedrobaeza
Copy link
Member

This module is outdated after the integration of the feature in core, and if you want it for the description thing, it should be renamed IMO.

@CRogos
Copy link
Contributor Author

CRogos commented Dec 14, 2023

The update price button in the standard is not always visible and there is not possibility to reset the description. But I agree this feature mostly adds some UI improvements.

The functionality is not changed, so I do not understand why we should change the name... but make a suggestion?

@pedrobaeza
Copy link
Member

I would maybe start from scratch a little module sale_order_price_recalculation_visibility with such function. And do you really use any time the reset description feature?

@CRogos
Copy link
Contributor Author

CRogos commented Dec 14, 2023

While refactoring this module I've exactly asked this question to my sales guys and they said that they are missing something like that. There themes to be a use case for that... but I agree that there could be a more beautiful solutions like these buttons.

@pedrobaeza
Copy link
Member

OK, you tell me then: should I press the button without thinking more?

@CRogos
Copy link
Contributor Author

CRogos commented Dec 14, 2023

Yes please... anyone who has this module installed would be able to migrate to v16 without any functionality change. And Odoo has improved but does not fulfil the whole functionality.

I agree that the module is now very small and maybe we can combine it with other functions in the future.

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-2778-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 68ba244 into OCA:16.0 Dec 14, 2023
8 of 9 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 8b63bf2. Thanks a lot for contributing to OCA. ❤️

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.