[14.0][FIX] mrp_bom_attribute_match #1100
Conversation
|
Hi @ivantodorovich, |
ilyasProgrammer
changed the title
|
@felixhummel Feel free to test ;) |
@lk-eska too :) |
ilyasProgrammer
force-pushed
the
14.0-mrp_bom_attribute_match-fix-boms-struct-report-nested
branch
from
b2c30a2 to
b4130ee
Compare
|
For some reason I still do not see lines in the report (they are skipped) when I set a product component (product template), but that might be from my side. If someone else is willing to review if changes are working for them, I would appreciate. |
|
@giarve did you test on runboat? can you post a video or a screenshot? |
On the still running instance:
I believe it is because when adding variants the line where it appears is not removed (which is supposed to happen), but it still looks for a product_id and it is not found. Also might have to do with dynamically created attributes? Although in this case they were instantly created. But I do not know if they are properly handled when product components are there but their dynamically created variants are not. Something extra: in the structure & cost report, children with product components cannot be unfolded. Summary: I am not sure this PR is the solution or it is just part of it. @ilyasProgrammer I think the |
|
I sent the fix here #997 |
I am not sure whether it works for all use cases. It was the first fix that was proposed in this PR if I recall correctly. |
| product = bom.get_component_template_product( | ||
| line, product, line.product_id | ||
| ) | ||
| product = bom.get_component_template_product(line, product, line.product_id) |
There was a problem hiding this comment.
Please check the function name, shouldn't it start with the underscore?
Maybe there is something I am missing here
|
@giarve Please try again |
no it was something else... anyway this is coming to an end. if it works we would be happy to use it. |
ilyasProgrammer
force-pushed
the
14.0-mrp_bom_attribute_match-fix-boms-struct-report-nested
branch
4 times, most recently
from
d6bf0e0 to
9749e45
Compare
ilyasProgrammer
force-pushed
the
14.0-mrp_bom_attribute_match-fix-boms-struct-report-nested
branch
from
b7f9671 to
e203c7a
Compare
|
@ivantodorovich good for merge? |
| @@ -43,3 +46,67 @@ def _get_bom_lines(self, bom, bom_quantity, product, line_id, level): | |||
| if isinstance(value, models.NewId): | |||
| component[key] = value.origin | |||
| return components, total | |||
|
|
|||
| def _get_price(self, bom, factor, product): | |||
There was a problem hiding this comment.
Would it be possible to add a few inline comments to indicate the patched lines?
As this is another complete override, it's always useful to know exactly what changed, specially for migrations.
ilyasProgrammer
force-pushed
the
14.0-mrp_bom_attribute_match-fix-boms-struct-report-nested
branch
from
e203c7a to
8efface
Compare
|
/ocabot merge patch |
|
What a great day to merge this nice PR. Let's do it! |
|
@ilyasProgrammer your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-1100-by-ilyasProgrammer-bump-patch. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
|
@ilyasProgrammer can you try again? |
|
/ocabot merge patch |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at dac3d8f. Thanks a lot for contributing to OCA. ❤️ |
related to #997
wip
New tests was copied from @felixhummel fork. @felixhummel do you mind? I will add you as a contributor.