sale-workflow icon indicating copy to clipboard operation
sale-workflow copied to clipboard

[17.0][IMP] sale_quotation_number: allow customizable quotation sequence

Open MohamedOsman7 opened this issue 1 year ago • 1 comments

Enable users to customize the sale.quotation sequence prefix, instead of it being restricted to the default 'SQ'.

Solution from version 14: https://github.com/OCA/sale-workflow/pull/2126

(PR 16: #3309 ) (PR 15: #3311 )

(Follow-up PR: https://github.com/OCA/sale-workflow/pull/3213)

MohamedOsman7 avatar Sep 12 '24 15:09 MohamedOsman7

@MohamedOsman7 Ok, next time, the better is to cheery-pick the original fix and then do the modification (the search before the loop) to keep original author. Thanks

rousseldenis avatar Sep 16 '24 07:09 rousseldenis

@rousseldenis could you add a review and merge?

CRogos avatar Oct 09 '24 15:10 CRogos

@pedrobaeza could you also merge this?

v17 of: https://github.com/OCA/sale-workflow/pull/3309

CRogos avatar Nov 16 '24 14:11 CRogos

/ocabot merge patch

pedrobaeza avatar Nov 18 '24 08:11 pedrobaeza

This PR looks fantastic, let's merge it! Prepared branch 17.0-ocabot-merge-pr-3310-by-pedrobaeza-bump-patch, awaiting test results.

OCA-git-bot avatar Nov 18 '24 08:11 OCA-git-bot

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

OCA-git-bot avatar Nov 18 '24 09:11 OCA-git-bot

@MohamedOsman7 @CRogos @pedrobaeza @rousseldenis @ValentinVinagre @HaraldPanten

This last improvement applied to versions from 14 up to 17 is not working properly.

First of all, there was an improvement already merged in v15 that solved the issues pointed out here. The PR was https://github.com/OCA/sale-workflow/pull/2719/files and there was a fix aftewards: https://github.com/OCA/sale-workflow/pull/2752/files. Is there any reason why these commits were not cherry-picked and another approach was developed instead?

Second, we think the approach previously available in v15 is more convenient, for two reasons basically:

  1. The approach in this PR does not work if the quotation's sequence uses a formula in its prefix. For example, if the sequences's prefix is "Q/%(year)s/" the following line will not work as expected: if sequence and self.name[: len(sequence.prefix)] != sequence.prefix: As the quotation number will contain the year itself (and not the string "%(year)"), the sequence will be not changed to the sale order sequence when the quotation gets confirmed.

  2. The approach in this PR only considers the sequences with codes "sale.quotation" and "sale.order", whereas the approach previsouly available in v15 uses methods get_quotation_seq and get_sale_order_seq to get the sequence to be applied. Those methods can easily be inherited so other sequences can be used depending on different requirements (for example, in module sale_order_type_quotation_numer https://github.com/OCA/sale-workflow/tree/15.0/sale_order_type_quotation_number).

Some of our clients currently use this module in v15 and are having issues as they use a formula in the quotation sequence prefix. We should find a way to proceed to get this sorted, as the old v15 of the module with an improvement, which worked fine, has stopped working and the new version of the module in the rest of versions will cause problems.

Thank you.

manuelregidor avatar Nov 19 '24 16:11 manuelregidor

IMO:

  • I think we should revert this last commit (At least in V15). It has been done in a stable version and it's not needed there.
  • Then we should discuss about the best approach for V16 and V17.

WDYT?

HaraldPanten avatar Nov 19 '24 16:11 HaraldPanten

Solving the issue in V15 before discussing the best solution.

https://github.com/OCA/sale-workflow/pull/3416

HaraldPanten avatar Nov 20 '24 09:11 HaraldPanten