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

[FIX] quality_control: Fix TypeError on new test category #193

Merged
merged 4 commits into from
Nov 18, 2017

Conversation

zamberjo
Copy link
Member

@zamberjo zamberjo commented May 4, 2017

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`.
@oihane
Copy link
Contributor

oihane commented May 10, 2017

If name is required how can it be False?

@zamberjo
Copy link
Member Author

Is failling on line:20 (self.complete_name = " / ".join(reversed(names))) when computing values for new records (not yet persisted) as join is expeacting a list of strings and receives a list of booleans ([False])

@oihane
Copy link
Contributor

oihane commented May 10, 2017

Won't be more correct not to compute the complete_name if there is not name?

@zamberjo
Copy link
Member Author

zamberjo commented May 10, 2017

LGTM

Copy link
Contributor

@oihane oihane left a 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:
Copy link
Member

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
...

Copy link
Contributor

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))     

Copy link

@damendieta damendieta Nov 14, 2017

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.

@LoisRForgeFlow
Copy link
Contributor

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.

@MiquelRForgeFlow
Copy link
Contributor

Please @zamberjo , will you update the PR?

@pedrobaeza
Copy link
Member

I think one of the suggestions should be applied at least before merging

@damendieta
Copy link

@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.

@LoisRForgeFlow
Copy link
Contributor

LoisRForgeFlow commented Nov 15, 2017

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.

@zamberjo
Copy link
Member Author

Sorry, I plan to review all outstanding PRs -including this one- over the week.

@LoisRForgeFlow
Copy link
Contributor

@zamberjo ok, let's keep this one then.

@LoisRForgeFlow LoisRForgeFlow added this to the 8.0 milestone Nov 15, 2017
@damendieta
Copy link

damendieta commented Nov 16, 2017

@zamberjo thanks a lot.
If possible, it's better to fix it just with this:

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.

@zamberjo
Copy link
Member Author

I should change the method name _get_complete_name to _compute_complete_name?

@pedrobaeza pedrobaeza merged commit 5e32d91 into OCA:8.0 Nov 18, 2017
LoisRForgeFlow pushed a commit to ForgeFlow/manufacture that referenced this pull request Nov 20, 2017
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
LoisRForgeFlow pushed a commit to ForgeFlow/manufacture that referenced this pull request Nov 20, 2017
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
SimoRubi pushed a commit to SimoRubi/manufacture that referenced this pull request Nov 21, 2017
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
SimoRubi pushed a commit to SimoRubi/manufacture that referenced this pull request Nov 21, 2017
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
ProcessControl-JHF pushed a commit to ProcessControl-JHF/manufacture that referenced this pull request Aug 23, 2018
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
pedrobaeza pushed a commit to ProcessControl-JHF/manufacture that referenced this pull request Aug 30, 2018
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
@crimoniv crimoniv deleted the 8.0-fix-create-test-category branch November 2, 2018 12:27
NachoAlesLopez pushed a commit to NachoAlesLopez/manufacture that referenced this pull request May 10, 2019
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
LoisRForgeFlow pushed a commit that referenced this pull request Jun 6, 2019
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
rvalyi pushed a commit to akretion/manufacture that referenced this pull request Mar 3, 2020
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
ps-tubtim pushed a commit to ecosoft-odoo/manufacture that referenced this pull request Mar 17, 2020
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
ps-tubtim pushed a commit to ecosoft-odoo/manufacture that referenced this pull request Nov 24, 2020
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
CarlosRoca13 pushed a commit to Tecnativa/manufacture that referenced this pull request Feb 8, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
pedrobaeza pushed a commit to Tecnativa/manufacture that referenced this pull request Feb 13, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
CarlosRoca13 pushed a commit to Tecnativa/manufacture that referenced this pull request Feb 15, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
CarlosRoca13 pushed a commit to Tecnativa/manufacture that referenced this pull request Feb 22, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
CarlosRoca13 pushed a commit to Tecnativa/manufacture that referenced this pull request Mar 2, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
enriquemartin pushed a commit to Digital5-Odoo/manufacture that referenced this pull request Mar 5, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
enriquemartin pushed a commit to Digital5-Odoo/manufacture that referenced this pull request Apr 21, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
enriquemartin pushed a commit to Digital5-Odoo/manufacture that referenced this pull request Oct 1, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
hpatelserpentcs pushed a commit to hpatelserpentcs/manufacture that referenced this pull request Dec 29, 2021
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
mymage pushed a commit to mymage/manufacture that referenced this pull request Dec 10, 2022
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
mymage pushed a commit to mymage/manufacture that referenced this pull request Dec 26, 2022
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
mymage pushed a commit to mymage/manufacture that referenced this pull request Jan 17, 2023
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
peluko00 pushed a commit to APSL/manufacture that referenced this pull request May 14, 2024
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
Trivedi-Vacha-SerpentCS pushed a commit to Trivedi-Vacha-SerpentCS/manufacture that referenced this pull request Oct 23, 2024
`TestQualityControl` is failing on `QcTestTemplateCatergory._get_complete_name`
when `name` is set as `False`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants