Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The system does not store Many2many field value by default #14761

Closed
minhyk opened this issue Dec 19, 2016 · 11 comments
Closed

The system does not store Many2many field value by default #14761

minhyk opened this issue Dec 19, 2016 · 11 comments
Labels
Confirmed the bug was confirmed by testers

Comments

@minhyk
Copy link

minhyk commented Dec 19, 2016

Impacted versions:

  • 10.0
    Steps to reproduce:
  1. Added Many2many field in the payslip object then default by onchange employee with the code below

@api.onchange('employee_id', 'date_from')
def onchange_employee(self):
expenses = self.env['hr.expense.sheet'].search([('state', '=', 'approve'),
('is_paid', '=', False),
('employee_id', '=', self.employee_id.id)])
if expenses:
expense_list = [line.id for line in expenses]
self.payslip_expense = [[6, 0, expense_list]]
return super(HrPayslip, self).onchange_employee()

Current behavior:
After press Save button, the Many2many field does not store the value. I tested with selecting by manual, this issue does not happen.

Expected behavior:
The system should store the value to use in next action

Video/Screenshot link (optional):

image

@minhyk
Copy link
Author

minhyk commented Dec 28, 2016

It seems there is a problem with Many2many new API field. I tried to default value for that field but the system cannot store. The 'vals' parameter return in create function like below
'payslip_expense': [[1, 10, {'employee_id': 3, 'total_amount': 5000000, 'currency_id': 24, 'state': 'approve', 'apply_date': '2016-12-28', 'name': u''}]]
The actual value should be like
'payslip_expense': [[6, False, [10]]]

I do not know why this thing happen.

@omar7r
Copy link
Contributor

omar7r commented May 23, 2017

We found this issue using the account_financial_report_qweb module, if you check "receivable_accounts_only" or "payable_accounts_only" in general ledger, it executes an on_change that set account_ids field (a many2many) when you save the wizard, it send in create for accounts fields the format (1, id, {vals}) thus it writes first all accounts, provoking that the reports prints very slow if you set this checkboxes. If you change the account_ids widget in view to many2many_tags it works perfectly an pass to create function the format (6, 0, [ids]) and it prints the report very fast.

Thus, there is an important bug in Many2many widget if you write it from an on_change in 10.0 version.

@zuher83
Copy link

zuher83 commented May 25, 2017

@omar7r yes i confirm this bug

@pedrobaeza
Copy link
Collaborator

@ged-odoo this bug is hitting several modules. Can you please check it?

@ged-odoo
Copy link
Contributor

@omar7r @zuher83 @pedrobaeza Can someone give me a more detailed sequence of steps that I can try on the runbot?

@pedrobaeza
Copy link
Collaborator

Well, it's not reproducible I think in Odoo itself, but you can try on OCA runbot in one module that is happening:

  • Go to http://3290907-10-0-69653a.runbot1.odoo-community.org/web (of the branch OCA/account-financial-report).
  • Invoicing > OCA accounting reports > Aged Partner Balance
  • Check "Receivable accounts only" for filling the AR account in the many2many_list widget.
  • Click on "Export PDF" button.
  • If you analyze the command, you will see a (1, ID, {...}) one, that rewrites the account, and that doesn't fill the many2many.

Thanks.

@ged-odoo
Copy link
Contributor

Ok, thank you @pedrobaeza for your answer. This is really interesting. The onchange (triggered by receivable accounts only) returns [[5], [1, 7, {"user_type_id": [1, "Receivable"], "currency_id": ...}].

So, we have a 5 (delete all) and a 1 (update). (This is already slightly weird, because commands 1 do not link to the record, except in onchange. In my opinion, commands 1 should always update and link to records).

Then, the create command sends only a command 1. This is why the wizard is empty at this point, because we created the record, but it is not linked in the m2m relation.

So, there is clearly a bug. As far as I can tell, we should also send a command 4, to link the updated record.

Now, I do not understand why you think it should be a 6 (replace all)? From the point of view of the web client, it only sees an onchange with an updated value, and it has no way to know that the record was not actually modified, so it has to perform a command 1. Am I missing something?

@pedrobaeza
Copy link
Collaborator

But do you think is logical to update the linked record in this case (no editable)?

@ged-odoo
Copy link
Contributor

Good point. It should not update if m2m is not editable. Any change will be saved already anyway if one open a record from the relation in form view and modifies it.

I confirm that this is a bug.

@kebeclibre
Copy link
Contributor

Although the bug is confirmed, repairing it in versions < saas-16 would be too sensitive to the rest of the code.

Above Saas-16, the client does send commands 6 and 1, hence the issue doesn't exists in versions above.

I'm therefore closing this issue

@omalbastin
Copy link

if we apply many2many_tags widget, it is working fine

alexis-via added a commit to akretion/account-financial-reporting that referenced this issue Sep 5, 2018
It replaces the workaround widget="many2many_tags" on field name="account_ids" which prevented from selecting several accounts at the same time (quite useful when you want to select an interval of accounts for example). We now use the regular M2M widget and inherit create()
JordiBForgeFlow pushed a commit to OCA/account-financial-reporting that referenced this issue Oct 1, 2018
It replaces the workaround widget="many2many_tags" on field name="account_ids" which prevented from selecting several accounts at the same time (quite useful when you want to select an interval of accounts for example). We now use the regular M2M widget and inherit create()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Confirmed the bug was confirmed by testers
Projects
None yet
Development

No branches or pull requests

7 participants