OCA / cooperative

GNU Affero General Public License v3.0
11 stars 22 forks source link

Credit note sequences prefix doesn't start with R for capital requests moves #97

Closed enricostano closed 2 months ago

enricostano commented 8 months ago

Module

cooperator -> https://github.com/OCA/cooperative/commit/edc54c894697a84f41671ccf1c02cbbb0c0b6cbd#diff-fa00e2ab14dae977fac13190dd93c7a1b9ae67f945673b2cfdc7919702757357R23

Describe the bug

A credit note (refund) for a capital request should have a number like RSUBJ/2023/001. Creating a refund from capital request invoice generates a refund with number SUBJ/2023/001 instead.

To Reproduce

Affected versions: 14.0

Steps to reproduce the behavior:

  1. Create a capital request invoice (release_capital_request = True)
  2. From the invoice create a credit note (refund)
  3. Validate the credit note
  4. The credit note number doesn't start with an R

Expected behavior A credit note number should always start with an R added to the journal prefix in order to avoid confusions.

Additional context Standard Odoo behavior: https://github.com/odoo/odoo/blob/14.0/addons/account/models/account_move.py#L1369

enricostano commented 8 months ago

@huguesdk Hi! Why are we overriding _get_starting_sequence() in account move? Thanks!

cc/ @robinkeunen

enricostano commented 8 months ago

For the moment we're exploring a solution but would be nice to understand if we can fix it in cooperator.

Let's see what you think and possible solutions.

huguesdk commented 8 months ago

hi @enricostano!

this is indeed a bug that we discovered and fixed while migrating cooperator to 16 (still in progress). the fix is exactly the same as yours. we’ve also added a test for this.

it would be nice to backport the important fixes to 14.0. we should discuss together how to handle this work.

_get_starting_sequence() is overridden to allow to set the number to the correct format (SUBJ/{YYYY}/{NNN}) (as no more sequences are involved for this).

enricostano commented 8 months ago

Hi @huguesdk ,

it would be nice to backport the important fixes to 14.0. we should discuss together how to handle this work.

Yep, we can help on that. Let's talk about it.

_get_starting_sequence() is overridden to allow to set the number to the correct format (SUBJ/{YYYY}/{NNN}) (as no more sequences are involved for this).

What I don't understand is why we can't use standard Odoo 14 format (SUBJ/{YYYY}/{MM}/{NNN}) also for capital requests invoices. Adopting standard Odoo 14 format would allow us to remove the override altogether, right? Am I missing something else? :thinking: :sweat_smile:

EDIT: Is this to keep compatibility with data coming from 12.0 in migrations?

huguesdk commented 8 months ago

actually, i don’t know where the original format is coming from (it is present as is in the first commit). when we ported to 14, we ensured that it did not change (there is even a test for this). maybe this can be questioned, indeed.

enricostano commented 8 months ago

@huguesdk OK, thanks! Let's talk about it.

In any case, one thing that I don't understand about this change Odoo core did compared to past versions is that not relying on actual sequences (a dedicated model, etc) we lose the ability to customize the numbering. What if I don't like the proposed format? Before, at least in 12.0, you could specify complex formats for each sequence to adapt to your needs. How do you do that on 14.0? Do you know of any OCA module helping dealing with this?

EDIT: I asked this question in OCA discord https://discord.com/channels/737652535149592587/765579573894709298/1162363562874908793 :crossed_fingers:

huguesdk commented 8 months ago

i think that odoo moved away from sequences for this to avoid problems related to sequences (for example: sequence number out of sync after importing data). the current way is quite hacky, though, as the sequence is “guessed” from the last value, except for the first one, where _get_starting_sequence() is used.

i’ve just found the module account_move_name_sequence to allow to customize this.

i think that it’s a good idea to simply use the default naming scheme, so that it would work in the same way as for invoices. however, we need to keep in mind that this naming scheme changed from 14 to 15 (from a month-based one to a year-based one).

huguesdk commented 8 months ago

apparently, the naming scheme currently used by cooperator was more or less copied from the invoice naming scheme in odoo 9 (except using 3 digits instead of 4). it was already year-based. 14 is the only version where it is month-based.

enricostano commented 8 months ago

it would be nice to backport the important fixes to 14.0. we should discuss together how to handle this work.

@huguesdk do you need any help on this?

In any case, I think we can close this issue. At least from our side. What do you think? Thanks!

huguesdk commented 2 months ago

closing this, as this has been fixed in version 16.0 (see commit 98b8d4b6e7538440b878006ed01f211d3c026fff). if someone needs this in 14.0, feel free to backport this change and create a pull request.