[16.0][IMP] barcodes_generator_abstract: Avoid double iteration on base generation #619
Conversation
|
Hi @legalsylvain, |
dalonsofl
force-pushed
the
16.0-fix-barcodes_generator_abstract
branch
2 times, most recently
from
43d3c16
to
04daac1
Compare
| self.user_fake.generate_base() | ||
| self.assertEqual(self.user_fake.barcode_base, 2) | ||
| self.user_fake.generate_barcode() | ||
| self.assertEqual(self.user_fake.barcode, "2000002000006") | ||
|
|
There was a problem hiding this comment.
Why do you remove this test. It looks valid ?
There was a problem hiding this comment.
Hi! Because it will be the same base and barcode. With this change if the product already has a base, it will not generate a new one
There was a problem hiding this comment.
If the code is called two times, It should be fixed. But this change is not desired.
There was a problem hiding this comment.
I don't undestand really well, it make sense that it doesn't change because you can't change the base two times in UI
There was a problem hiding this comment.
Maybe I did'nt understood the point. I'll investigate a little.
Thanks for your time.
There was a problem hiding this comment.
I just investigated a little. the problem exists.
I permit me to fix it with #651.
@dalonsofl : could you review it ?
thanks !
There was a problem hiding this comment.
Hi @legalsylvain !
I checked the PR and it looks great!
I change my test like yours and it seems to work great too.
I get the same value with your PR as mine, so both of them are correct.
What do you think with the new changes?
Thanks!!
There was a problem hiding this comment.
Hi @dalonsofl. Thanks for reviewing the other PR.
Well, I prefer to have explicit text that explain what does the generate_base() function, and to avoid to double check if barcode_base is defined.
I so merged the other PR.
thanks for raising the issue.
There was a problem hiding this comment.
I agree, the result is the same, so if you think it's better to have comments on generate_base() , I'm fine with that.
There was a problem hiding this comment.
I merged the other PR, you can close this one.
thanks for the investigation.
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
dalonsofl
force-pushed
the
16.0-fix-barcodes_generator_abstract
branch
from
04daac1
to
9ac97b1
Compare
dalonsofl
force-pushed
the
16.0-fix-barcodes_generator_abstract
branch
from
9ac97b1
to
cf04f74
Compare
Module
barcodes_generator_abstract
Describe the bug
The base is changed when it is generated by having the ‘Automatic generation’ marked in the barcode nomenclature.
To Reproduce
(image from runboat OCA)
Affected versions:
16.0
Additional context
The problem is that when going through this function inside the barcodes_generator_product it goes back through the generate_base() and a recalculation of the base incorrectly, as it already has one before. With this change it would be solved