Skip to content

mass_editing: Avoid errors if 'model_id' is not a list of ids.#325

Merged
lasley merged 0 commit into
OCA:8.0from
merchise-autrement:8.0
Dec 9, 2016
Merged

mass_editing: Avoid errors if 'model_id' is not a list of ids.#325
lasley merged 0 commit into
OCA:8.0from
merchise-autrement:8.0

Conversation

@hpestano

@hpestano hpestano commented Jan 6, 2016

Copy link
Copy Markdown

When I try to make this query

@api.model
def get_default_fields(self):
    field_pool = self.env['ir.model.fields']
    fields = ['name', 'street', 'zip', 'city', 'email']
    ids = [field_pool.search([('model_id', '=', "'res.partner'"), ('name', '=', f)]).id for f in fields]
    return ids

I get this error

ValueError: invalid literal for int() with base 10: 'es.partne'

That is why i suggest this change

@oca-clabot

Copy link
Copy Markdown

Hey @hpestano, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

@pedrobaeza

Copy link
Copy Markdown
Member

Can you please explain your use case (steps to reproduce the error on the UI)?

You need also to send the CLA

@hpestano

hpestano commented Jan 7, 2016 via email

Copy link
Copy Markdown
Author

@hpestano

hpestano commented Jan 7, 2016

Copy link
Copy Markdown
Author

The error is to make a search in the model 'ir.model.fields'
Using the domain [('model_id', '=', 'res.partner'), ('name', 'in', ['name', 'street', 'zip'])]

My goal is to get ids fields 'res.partner' but the addon mass_editing overwrites the search function in IrModelFields. When I run the query ValueError "invalid literal for int () With base 10: 'es.partne'"

@mvaled

mvaled commented Dec 8, 2016

Copy link
Copy Markdown

Hi @pedrobaeza,

I'm a coworker with @hpestano. The point of this change is that without mass_editing installed a query like:

>>> imf.search([('model_id', '=', 'res.partner')])
    ir.model.fields(909, 929, 950, 954, 933, 914,..., 916)

works out of the box. However, after installing mass_editing it fails cause it changes the search method of ir.model.fields and expects that strings are just comma-separate ids.

I'm working with @hpestano so that he signs the CLA and send it and also we're gonna rebase this change on top of 8.0.

Best regards,
Manuel.

mvaled pushed a commit to merchise-autrement/server-tools that referenced this pull request Dec 8, 2016
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 OCA#325 for a new PR.
mvaled pushed a commit to merchise-autrement/server-tools that referenced this pull request Dec 8, 2016
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 OCA#325 for a new PR.
@lasley
lasley merged commit 1861b63 into OCA:8.0 Dec 9, 2016
@pedrobaeza

Copy link
Copy Markdown
Member

@lasley, why did you merge it so fast? We haven't even checked the CLA.

@lasley

lasley commented Dec 9, 2016

Copy link
Copy Markdown
Contributor

Wait what? This is the first I've seen of this PR, I don't even see how I could have accidentally merged it. The merge itself only contains translations too, none of which was included in the commits in this PR, so I'm thoroughly confused 1861b63

@lasley

lasley commented Dec 9, 2016

Copy link
Copy Markdown
Contributor

So uhhhh how do we fix this?

@pedrobaeza

Copy link
Copy Markdown
Member

No, it seems that you haven't do anything. It has been auto-merged as they have make an strange GIT operation, resulting this PR in 0 commits. No worry then.

@lasley

lasley commented Dec 9, 2016

Copy link
Copy Markdown
Contributor

I wonder why it marked me as the merger though, that's weird. I guess this is why I sign my commits - maybe should start doing it on merges too in hindsight

@pedrobaeza

Copy link
Copy Markdown
Member

I think it could happen because you have pushed a branch to GitHub that contains same commits, and detect this one as the same with commit history, and interpreted it as it was merged. But nothing to do with the signing of the commits.

@mvaled

mvaled commented Dec 9, 2016

Copy link
Copy Markdown

Hi @pedrobaeza, Indeed we reset our 8.0 branch to match that of OCA and created another branch with the patch (now PR #638). This seems to have confused github somehow. Sorry for the inconvenience.

@pedrobaeza

Copy link
Copy Markdown
Member

The problem is that you used here the 8.0 branch instead of another different branch name. Try to avoid this next time.

@lasley

lasley commented Dec 9, 2016

Copy link
Copy Markdown
Contributor

@mvaled - FYI the commits you made on this fix are now gone, but still recoverable. I recommend you make a new branch, cherry pick 86dc30f & 6168a48, and submit a new PR (assuming you want to continue forward with the changes). You will also need to sign the CLA & send to cla@odoo-community.org

@mvaled

mvaled commented Dec 9, 2016

Copy link
Copy Markdown

@lasley I did that already. Now is PR #638.

I have personally signed the CLA, but since this change comes from my team, I've just sent the CLA for our organization https://github.com/merchise-autrement.

Best regards,
Manuel.

StefanRijnhart pushed a commit to StefanRijnhart/server-tools that referenced this pull request Feb 26, 2017
…mpty-field

8.0 web widget one2many tags empty field
@mvaled
mvaled deleted the 8.0 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 (8.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.

5 participants