Skip to content

mass_editing: Fix issue with ir.model.fields implementation of search#638

Merged
pedrobaeza merged 1 commit into
OCA:8.0from
merchise-autrement:8.0-fix-mass-editing-with-strings
Jan 17, 2017
Merged

mass_editing: Fix issue with ir.model.fields implementation of search#638
pedrobaeza merged 1 commit into
OCA:8.0from
merchise-autrement:8.0-fix-mass-editing-with-strings

Conversation

@mvaled

@mvaled mvaled commented Dec 8, 2016

Copy link
Copy Markdown

Without 'mass_editing' installed you can search the 'ir.model.fields' like this:

ir_model_fields.search([('model_id', '=', 'res.partner')])

and it works.

However mass_editing changes the 'search' method and broke this type of query. This commit fixes the 'search' method so that it allows the legacy behavior.

Closes #325 for a new PR.

@mvaled

mvaled commented Dec 9, 2016

Copy link
Copy Markdown
Author

I kept digging about the usage of 'model_id' to test a string. In all our codebase I found two usages:

  • One in an old (now unused) module from Julius Networks.
  • A single addon of one of our customer.

Can I safely switch the 'model_id' for 'model'? I found only two cases where model_id.model is not the same as model:

SELECT DISTINCT imf.model, m.model, imf.model = m.model
       FROM ir_model_fields imf JOIN ir_model m ON imf.model_id = m.id WHERE imf.model <> m.model;
         model          |         model         | ?column? 
------------------------+-----------------------+----------
 crm.case.resource.type | crm.tracking.campaign | f
 crm.case.channel       | crm.tracking.medium   | f

But I wonder if that's an error.

@StefanRijnhart

Copy link
Copy Markdown
Member

Thank you for taking up this one! This hack in the model's search method allowed for a domain containing a stringified list of ids. It has been obsolete since 4442424, and can safely be removed (i.e. remove the whole file).

@mvaled

mvaled commented Jan 16, 2017

Copy link
Copy Markdown
Author

Hi @StefanRijnhart, I'll move your suggestion to the PR #638 which is the one standing for this issue.

@mvaled

mvaled commented Jan 16, 2017

Copy link
Copy Markdown
Author

Opps, I didn't notice this is the PR...

As commented by @StefanRijnhart, this is no longer needed after 4442424:

  > This hack in the model's search method allowed for a domain containing a
    stringified list of ids. It has been obsolete since 4442424, and can
    safely be removed (i.e. remove the whole file).

Closes OCA#638.
@mvaled
mvaled force-pushed the 8.0-fix-mass-editing-with-strings branch from 6168a48 to 09732a8 Compare January 16, 2017 23:17
@mvaled

mvaled commented Jan 16, 2017

Copy link
Copy Markdown
Author

Done in 09732a8

@StefanRijnhart StefanRijnhart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! Tested on runbot.

@pedrobaeza
pedrobaeza merged commit fd70c9a into OCA:8.0 Jan 17, 2017
JayVora-SerpentCS pushed a commit to JayVora-SerpentCS/server-tools that referenced this pull request Feb 15, 2017
As commented by @StefanRijnhart, this is no longer needed after 4442424:

  > This hack in the model's search method allowed for a domain containing a
    stringified list of ids. It has been obsolete since 4442424, and can
    safely be removed (i.e. remove the whole file).

Closes OCA#638.
@mvaled
mvaled deleted the 8.0-fix-mass-editing-with-strings branch May 9, 2019 20:22
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (10.0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants