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

[8.0][FIX] account_asset: Do not include inactive asset lines on asse… #600

Closed
wants to merge 1 commit into from

Conversation

Daniel-CA
Copy link

@Daniel-CA Daniel-CA commented Mar 22, 2017

Description of the issue/feature this PR addresses:
Not reported
Current behavior before PR:
Inactive asset lines are computed on asset analysis report
Desired behavior after PR is merged:
Do not include inactive asset lines on asset analysis report

odoo#16092

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@pedrobaeza
Copy link
Member

IMO the report should include all assets, but let's see what Odoo says

@Daniel-CA
Copy link
Author

Thanks @pedrobaeza

Copy link
Member

@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

Agree with @pedrobaeza: if there are depreciation lines, they should be shown even for inactive assets.

@Daniel-CA
Copy link
Author

Thanks for your reply @StefanRijnhart
But let me give my view about this issue. In my opinion the main problem is when the asset's source invoice is changed, it inactivates the previous asset and creates a new one with his own depreciation lines, so we have both depreciation lines computed in the analysis report and I think this is incorrect because the information is duplicated in this case. Moreover the user is not known that a "new" active has been generated even if the previous one has already posted lines.

@pedrobaeza
Copy link
Member

But then the bug is because canceling the invoice doesn't remove the asset. In version 9 it's done this way.

@Daniel-CA
Copy link
Author

But you can not remove the asset, it could have posted depreciation lines linked to the account moves. In this case the invoice should not be able to be cancelled.

I have been reviewing v9 code as @pedrobaeza says, but I see a similar code only moved to another object:

    def action_move_create(self):
        result = super(AccountInvoice, self).action_move_create()
        for inv in self:
            if inv.number:
                asset_ids = self.env['account.asset.asset'].sudo().search([('invoice_id', '=', inv.id), ('company_id', '=', inv.company_id.id)])
                if asset_ids:
                    asset_ids.write({'active': False})
            inv.invoice_line_ids.asset_create()
        return result

Moreover, I just want to add an easy solution for the asset report, more than start changing the asset code and how it works.

@pedrobaeza
Copy link
Member

Sorry, you're right, assets become inactive, but then again your statement is not correct. You need to show depreciation lines already posted, even for inactive assets. With this, you're hiding them.

@pedrobaeza
Copy link
Member

As stated upstream, the bug is not correct, so I close.

@pedrobaeza pedrobaeza closed this Jul 4, 2017
@Daniel-CA
Copy link
Author

Hi @pedrobaeza,
But the code was changed to show depreciation lines already posted. Not posted lines are computed in the analysis report even if they are in a cancelled asset, and this is a bug imho.

@pedrobaeza
Copy link
Member

OK, I didn't see the change, but why don't you say this to Odoo also? They can accept this change. I reopen anyway.

@pedrobaeza pedrobaeza reopened this Jul 5, 2017
@pedrobaeza
Copy link
Member

@StefanRijnhart, what do you think about the new code?

@Daniel-CA
Copy link
Author

Hi @pedrobaeza,
Ok, I will post it too in the Odoo branch, thanks for reply.

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.

None yet

4 participants