Skip to content

[FIX] sale_order_product_recommendation: wizard didn't properly get on memory created lines#713

Merged
pedrobaeza merged 1 commit intoOCA:11.0from
Tecnativa:11.0-fix-sale_order_product_recommendation-wiz
Oct 25, 2018
Merged

[FIX] sale_order_product_recommendation: wizard didn't properly get on memory created lines#713
pedrobaeza merged 1 commit intoOCA:11.0from
Tecnativa:11.0-fix-sale_order_product_recommendation-wiz

Conversation

@chienandalu
Copy link
Member

I couldn't get to the origin point of the issue (lines info gets lost in the wizard action) but this patch fixes it.

cc @Tecnativa

@pedrobaeza
Copy link
Member

@yajo you were the one that creates this. Can you please check why this is needed?

@rafaelbn rafaelbn added this to the 11.0 milestone Oct 10, 2018
for line in self.order_id.order_line:
found_line = found_dict[line.product_id.id]
new_line = RecomendationLine.new({
new_line = RecomendationLine.create({
Copy link
Member

Choose a reason for hiding this comment

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

In general terms, creating records in an onchange is not a very good idea... This is a transient model, so records get removed every now and then, so maybe we can make an exception?

🤔 Have you tried converting each .new() to (0, 0, {...})? I think that behind the scenes both get casted to the same operation, but @rco-odoo told me recently that this wasn't 100% the case. At least it's worth trying?

Copy link
Member

Choose a reason for hiding this comment

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

Of course, you shouldn't create on an onchange! @chienandalu this is not correct, so you have to find another way to solve the problem.

vals['line_ids'] = [
line for line in vals['line_ids'] if line[2]["is_modified"]]
return super(SaleOrderRecommendation, self).create(vals)
self.line_ids = wiz_line_ids
Copy link
Member

Choose a reason for hiding this comment

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

Or is it possible that all those += mess things up? Have you tried just storing all new records in a separate variable and assigning it just once at the end like here?

BTW you can remove line 56 with this approach.

"sale.order.recommendation",
"Wizard",
ondelete="cascade",
required=True,
Copy link
Member

Choose a reason for hiding this comment

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

This should be required...

@pedrobaeza pedrobaeza force-pushed the 11.0-fix-sale_order_product_recommendation-wiz branch from dd69a81 to 6d95a59 Compare October 25, 2018 12:26
@pedrobaeza
Copy link
Member

@yajo, this is the correct solution

@chienandalu please review it.

Copy link
Member Author

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

👍 force_save was the key

@pedrobaeza pedrobaeza merged commit 1fe4a9a into OCA:11.0 Oct 25, 2018
@pedrobaeza pedrobaeza deleted the 11.0-fix-sale_order_product_recommendation-wiz branch October 25, 2018 12:59
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.

4 participants