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

[master] Tighten type check in domain #18

Merged
merged 1 commit into from
Aug 13, 2014

Conversation

bwrsandman
Copy link

Fixes #15
Check that domain[2] is str because in some cases it's an int and list
operations don't work on ints.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e5b37b1 on bwrsandman:8.0-search_type into ba279f0 on OCA:master.

@OSguard
Copy link

OSguard commented Jul 18, 2014

i tested the patch and it solve the problem. Thanks

@bwrsandman
Copy link
Author

a :+1: would be apreciated :)

@pedrobaeza
Copy link
Member

Here you have ;)

👍 (code review)

@hbrunn
Copy link
Member

hbrunn commented Aug 11, 2014

👍

@sbidoul
Copy link
Member

sbidoul commented Aug 11, 2014

👍 (code review)

@bwrsandman
Copy link
Author

Rebased

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling dcff018 on bwrsandman:8.0-search_type into 4765f54 on OCA:master.

@@ -32,7 +32,7 @@ def search(
model_domain = []
for domain in args:
if domain[0] == 'model_id' and domain[2]\
Copy link
Member

Choose a reason for hiding this comment

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

@bwrsandman I think you should also remove the and domain[2] which is useless (and confusing) after the change below, as an empty string will not cause a problem. You may want to change it to and len(domain) == 3 which will avoid an IndexError when confronted with a bad formed domain, and which maybe was the original intent, but I would not do this as this will most certainly crash a couple of calls further down in the stack.

-> needs fixing

@bwrsandman
Copy link
Author

@gurneyalex done

Fixes OCA#15
Check that `domain[2]` is `str` because in some cases it's an `int` and `list`
operations don't work on `int`s.
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7fc2ef4 on bwrsandman:8.0-search_type into 4765f54 on OCA:master.

@gurneyalex
Copy link
Member

@bwrsandman OK, I'll merge this and port the merge to 7.0 too (instead of #16)

@gurneyalex gurneyalex merged commit 7fc2ef4 into OCA:master Aug 13, 2014
@bwrsandman bwrsandman deleted the 8.0-search_type branch August 13, 2014 14:36
StefanRijnhart pushed a commit to StefanRijnhart/server-tools that referenced this pull request Feb 26, 2017
[MIG] web_easy_switch_company
dreispt pushed a commit that referenced this pull request Jun 5, 2017
Garamotte pushed a commit to subteno-it/server-tools that referenced this pull request Jul 12, 2017
modoolar-bot pushed a commit to modoolar/oca-server-tools that referenced this pull request Jan 14, 2022
[12.0][MIG] server environment from 11.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.

7 participants