OCA / account-financial-tools

Odoo Accountant Financial Tools and Utils
GNU Affero General Public License v3.0
303 stars 749 forks source link

[16.0][IMP] account_asset management: add depreciated value to facilitate import #1793

Closed luc-demeyer closed 4 months ago

luc-demeyer commented 8 months ago

Module: account_asset_manangement

This commit adds the fields import_depreciated_value, import_depreciated_date to facilitate importing assets from another accounting package.

Assume You have an asset of 5000 EUR, depreciated over 5 years whereby 700 EUR has already been depreciated previous fiscal year. If you set import_depreciated_value to 700 and import_date to the last day of your previous fiscal year in the import file the depreciation table will be calculated as from the beginning of the current fiscal year and a single 'init' entry will be created on the last day of the previous fiscal year.

luc-demeyer commented 8 months ago

@pedrobaeza this repo seems to be broken, cf. pre-commit error message: account_invoice_constraint_chronology/i18n/nl.po:31 Translation string couldn't be parsed correctly using str.format KeyError('datum_factuur')

luc-demeyer commented 8 months ago

depreciation table when using these fields: image

Table is nicer when using this PR in combination with https://github.com/OCA/account-financial-tools/pull/1785 asset_board

pedrobaeza commented 8 months ago

I think you should avoid the word import, as the use case is wider (you may depreciate manually in the past, and want to continue using this module). already_depreciated_amount may be a good match. For me, the semantics of the fields should be:

Other than that, the idea seems good for me. Not sure if you can modify the _compute_depreciation_base method to decrease the already depreciated amount instead of intercepting the other methods.

What do you think?

luc-demeyer commented 7 months ago

@pedrobaeza

The similar field in Odoo Enterprise account_asset module is called: already_depreciated_amount_import

# Adapt for import fields
already_depreciated_amount_import = fields.Monetary(
    readonly=True, states={'draft': [('readonly', False)]},
    help="In case of an import from another software, you might need to use this field to have the right "
         "depreciation table report. This is the value that was already depreciated with entries not computed from this model",
)

Hence your suggestion for already_depreciated_amount is close to this and is also ok for me.

I have a bit of an issue with your suggestion to rename the 'import_depreciated_date' to 'depreciation_date_start'. I agree that 'import_depreciated_date' may be confusing but imho that's also the case with 'depreciation_date_start'. This date is the date of the 'INIT' entry with the 'already_depreciated_amount' value. What about calling this field 'already_depreciated_amount_init_date' ?

Concerning modifying the '_compute_depreciation_base' logic in stead of changing the 'compute_depreciation_board': I think we risk to break the whole computation logic whereas my current change doesn't touch the existing logic, hence comes with low risk.

pedrobaeza commented 7 months ago

Please rebase properly

luc-demeyer commented 4 months ago

Closing since I broke this PR with a wrong rebase. A new, clean PR with the improvement, cf. https://github.com/OCA/account-financial-tools/pull/1857