-
-
Notifications
You must be signed in to change notification settings - Fork 490
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
[FIX] quality_control: Fix TypeError on new test category #193
[FIX] quality_control: Fix TypeError on new test category #193
Conversation
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
If |
Is failling on line:20 ( |
Won't be more correct not to compute the |
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
names.append(parent.name) | ||
parent = parent.parent_id | ||
self.complete_name = " / ".join(reversed(names)) | ||
if self.name: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't make the diff bigger and increase cyclomatic complexity. Make instead:
if not self.name:
return
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't be more correct to convert it to @api.multi
for record in self.filtered('name'):
names = [record.name]
parent = record.parent_id
while parent:
names.append(parent.name)
parent = parent.parent_id
record.complete_name = " / ".join(reversed(names))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to just add a or '' to the code, like this:
names = [record.name or '']
But it works, so, approved.
Just a favor, can you squash to only one commit when merging? 😁, that way is very fast for me to move this changes to v9 PR. |
Please @zamberjo , will you update the PR? |
I think one of the suggestions should be applied at least before merging |
@zamberjo are you going to apply the suggestions. As @zamberjo hasn't commented or made any commit from may 10, maybe it's better to close this PR and open a new one with a fix. It's a one liner 3 characters fix and it's hanging for almost 6 months. Please @zamberjo, could you be so kind to help us with this modifications? Thanks. |
I agree with @damendieta , this is a very small fix and it has to move forward... EDIT: Please, @damendieta open a PR with your proposal and we can supersede this PR. |
Sorry, I plan to review all outstanding PRs -including this one- over the week. |
@zamberjo ok, let's keep this one then. |
@zamberjo thanks a lot. names = [record.name or ''] Adding the empty string alternative will solve the problem with the least code and no need to add a new indent to the formula, also, only one line will change. Could you please consider this fix instead. Thanks a lot. |
I should change the method name |
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name` when `name` is set as `False`.
TestQualityControl
is failing onQcTestTemplateCatergory._get_complete_name
when
name
is set asFalse
.