Skip to content

Master analytic improvement tbe - #17499

Closed
qdp-odoo wants to merge 10 commits into
odoo:masterfrom
odoo-dev:master-analytic-improvement-tbe
Closed

Master analytic improvement tbe#17499
qdp-odoo wants to merge 10 commits into
odoo:masterfrom
odoo-dev:master-analytic-improvement-tbe

Conversation

@qdp-odoo

@qdp-odoo qdp-odoo commented Jun 8, 2017

Copy link
Copy Markdown
Contributor

@qdp-odoo qdp-odoo left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

first pass done. some changes are required though you're on the right track :)

Comment thread addons/account/models/account_move.py Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

these 2 functions should be removed since we want that on analytic lines (not account.move.line)

Comment thread addons/account/models/account_move.py Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

to remove

Comment thread addons/account/models/account_move.py Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

to remove

Comment thread addons/account/models/account_move.py Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

must refactor the cancellation of account.move to unlink all the analytic lines created (even with a distribution).

Otherwise, currently you're duplicating the analytic entries each time you cancel-revalidate a journal entry

Comment thread addons/account/models/account_move.py Outdated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

remove the currency computation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

+set explicitly the company_id

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

to remove entirely

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

line['amount_user_currency'] += r['amount_user_currency']

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

required=True

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

group_analytic_accounting

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

dead code to remove

Comment thread addons/account/models/account_move.py Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not move.mapped('line_ids.analytic_line_ids').unlink()? That will remove all analytic entries in a single unlink call.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"Or" between a leaf and nothing? That doesn't seem correct.

@tbe-odoo tbe-odoo Jun 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you enlight me please, would something like this be better?

    @api.model
    def name_search(self, name, args=None, operator='ilike', limit=100):
        if name:
            recs = self.search([('account_id', operator, name)] + (args or []), limit=limit)
            return recs.name_get()
        return super(AccountAnalyticDistribution, self).search(domain, limit=limit).name_get()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't know what you're trying to do, the original just doesn't make any sense. The new version is at least syntactically correct.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, qdp asked me to remove the 'name' field from the model and to use account_id's name for the search.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, the version you posted in your comment makes more sense then.

@xmo-odoo xmo-odoo Jun 9, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why copy the context twice?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just call .with_context(date=line.date), there's no need to do any context copy.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That strikes me as a pointless optimisation, the only thing it potentially saves is a rounding (which you can disable with round=True anyway).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not call it domain? And why the list conversion?

@tbe-odoo tbe-odoo Jun 9, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I can't call it domain since there's already a parameter named domain, the list is pointless ?

Should I do ?

__domain = '__domain' in line and line['__domain'] or []

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use sum()?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why all the spaces?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bad habits, sorry.

@C3POdoo C3POdoo added the RD research & development, internal work label Jun 9, 2017
@tbe-odoo
tbe-odoo force-pushed the master-analytic-improvement-tbe branch from 1f49cd4 to 2af953c Compare June 9, 2017 10:36
@xmo-odoo
xmo-odoo dismissed their stale review June 12, 2017 07:49

fixed

@tbe-odoo
tbe-odoo force-pushed the master-analytic-improvement-tbe branch from b4664c6 to eeaec6c Compare August 10, 2017 13:12
- Categories can now be set to analytic account.
- Analytic tags can now be used on journal entries without the need to activate analytic acounting.

task: https://www.odoo.com/web#id=32386&view_type=form&model=project.task&action=333&active_id=967&menu_id=4720
- Added the ability to create a hierarchy to categories.
- Analytic tags are now fully independent to Analytic Accounting and can be activited through the settings.
…cy/multi-company.

- Added Analytic Distribution to analytic tags, Analytic Accounting and Analytic Tags must be activated.
- Analytic moves are now linked to the user's company.
- Analytic Analysis & analytic move are now expressed in user's currency.
- Analytic Accounts do not require anymore linking them to a company.
- Added the ability to use analytic tags on analytic default.
- Removed field 'analytic_amount_currency' from account analytic line.
- The childrens are now displayed anymore for categories.
- The categories are now displayed with this format 'Parent n°1 / Parent n°2 / Parent n°X / Name'
@tbe-odoo
tbe-odoo force-pushed the master-analytic-improvement-tbe branch from eeaec6c to 4a6530c Compare November 27, 2017 12:36
…ies can now be set to analytic account. - Analytic tags can now be used on journal entries without the need to activate analytic acounting.
@tbe-odoo tbe-odoo closed this Mar 22, 2018
@tbe-odoo
tbe-odoo deleted the master-analytic-improvement-tbe branch March 22, 2018 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RD research & development, internal work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants