[16.0][ADD] account_analytic_tag #572

Yadier-Tecnativa · 2023-07-11T01:25:42Z

Restore account.analytic.tag
Solve the issue #551

@Tecnativa TT44062

Yadier-Tecnativa · 2023-07-11T03:44:49Z

ping @sergio-teruel , @pedrobaeza

pedrobaeza

This is missing a lot of things:

Analytic tags on invoices.
Transfer such tags on the created analytic entries when publishing the invoices.

pedrobaeza · 2023-07-13T06:37:17Z

account_analytic_tag/models/account_analytic_tag.py

+from odoo import fields, models
+
+
+class AccountAnalyticDistribution(models.Model):


This one is not needed anymore. You only need the tag

pedrobaeza · 2023-07-13T06:37:40Z

account_analytic_tag/models/account_analytic_tag.py

+    active_analytic_distribution = fields.Boolean("Analytic Distribution")
+    analytic_distribution_ids = fields.One2many(
+        "account.analytic.distribution", "tag_id", string="Analytic Accounts"
+    )


Remove these 2 fields. We are only wanting the tag part.

pedrobaeza · 2023-07-13T06:37:52Z

account_analytic_tag/models/account_analytic_tag.py

+
+
+class AccountAnalyticLine(models.Model):
+    _inherit = "account.analytic.line"


This should go into a separate file.

pedrobaeza · 2023-07-13T06:39:12Z

account_analytic_tag/readme/DESCRIPTION.rst

@@ -0,0 +1 @@
+This module adds *account analytic tags* on accounts.


Suggested change

This module adds *account analytic tags* on accounts.

This module restores the *account analytic tags* as a method for categorizing analytic entries, selectable from the invoices, and transferred to them on publishing.

pedrobaeza · 2023-07-13T06:42:26Z

account_analytic_tag/readme/CREDITS.rst

Don't add empty files.

account_analytic_tag/readme/USAGE.rst

pedrobaeza · 2023-07-13T06:43:19Z

account_analytic_tag/readme/CONTRIBUTORS.rst

@@ -0,0 +1 @@
+* Yadier Quesada - Tecnativa <[email protected]>


This is not the form we put it:

Suggested change

* Yadier Quesada - Tecnativa <yadier.dequesada@tecnativa.com>

* `Tecnativa <https://www.tecnativa.com>`__:

* Yadier Quesada

Yadier-Tecnativa · 2023-07-17T23:21:14Z

All suggestions were made, please review @pedrobaeza

Yadier-Tecnativa · 2023-07-19T17:31:25Z

ping @pilarvargas-tecnativa , @carlosdauden Please review

rafaelbn · 2023-07-26T20:25:18Z

Thank you! Great contribution! I will review this Friday!

pepetreshere · 2023-08-02T16:03:13Z

@Yadier-Tecnativa @pedrobaeza nice PR!, would you please drop some light on why do you remove the analytic_distribution_ids? The analytic_distribution_ids allowed to have a "predefined" percentage for each analytic account so that it is easier for and end-user to quickly select a set of "predefined" percentages. This is not covered by standard Odoo in v16.0 not even by using the new analytic plans. Thanks in advance for any light

pedrobaeza · 2023-08-02T20:51:39Z

Because this feature is already covered by standard v16.

rafaelbn · 2023-08-09T22:45:40Z

Hello! Really thank you very much for this contribution.

The implementation with this PR of analytic tags is different from Odoo 15 and previous. In this moment functionality of this module duplicated the function of Analytic Plans.

Analytic tags cannot have analytic account mandatory as they are independent, in fact don't need any relation.

You create analytic tags for many analytic accounts that will be created in the future (like projects)

Important: When the mind behind this decision in Odoo deleted the tags, he/she thought that all Odoo customers worldwide where using tags for distributions! But not, just tag. See

I have recorded a video and created an excel 😄 see:

How the OCA will deal with the disappearance of analytic tags #551 (comment)

This PR should be able to reproduce that use case like in Odoo 15, 14, 13, 12, 11, etc.

There is not menu to add, see or edit analytic tags

The setting option to activate is missing

We cannot set analytic tags in invoice lines, the field is not available

Let me know please if I can help you further

Regards,
Rafael

rafaelbn · 2023-08-09T22:46:57Z

account_analytic_tag/README.rst

+=====
+
+Analytical tags.
+This function allows you to create formulas to establish the distribution of prices


@Yadier-Tecnativa this is not the function of analytic tags

rafaelbn · 2023-08-09T22:47:40Z

account_analytic_tag/README.rst

+Analytical tags.
+This function allows you to create formulas to establish the distribution of prices
+between analytical accounts. This means that when we create an analytics tag,
+we must establish an Analytical Account to which it is directed and the monetary percentage.


As commented previously not this is not. Please check the video I recorded 🙏🏼 😄

sbidoul · 2023-08-11T07:13:33Z

Thanks for this work.

I think there are many different ways people were using analytic tags with other versions.

Would this make sense to you that we split the feature with first a base module that just provides the account.analytic.tag model, then additional modules that make use of them. Such as using tags on journal items, or settings tags on analytic accounts (as was possible until v11) ?

sbidoul · 2023-08-11T07:15:25Z

account_analytic_tag/models/account_analytic_tag.py

+    company_id = fields.Many2one("res.company", string="Company")
+    account_analytic_id = fields.Many2one(
+        "account.analytic.account", string="Analytic Account", required=True
+    )


This m2o field seems wrong ?

Can you add a help text that explains the purpose of this account_analytic_id field? Perhaps a more specific label than Analytic Account would help too.

pedrobaeza · 2023-08-11T09:09:54Z

Would this make sense to you that we split the feature with first a base module that just provides the account.analytic.tag model, then additional modules that make use of them. Such as using tags on journal items, or settings tags on analytic accounts (as was possible until v11) ?

I don't see many sense on splitting model definition from account.move.line links at least. No need to extra dependencies (all are in account). It's complicating the module tree, even increasing maintenance burden, while the features are code are very simple to be maintained even if packed.

pedrobaeza · 2023-08-11T09:10:08Z

FYI, I will overtake this PR and finish it.

sbidoul · 2023-08-11T10:37:39Z

I don't see many sense on splitting model definition from account.move.line links at least

In our use case we need the tags but not the link with account.move.line because the tags are set on analytic accounts (it was possible until v11).

So I don't mind if the link with move lines is in the same module, but then we also need the group to show the analytic tags on move lines only to selected users.

In general I prefer small modules with restricted scope because they are easier to migrate. But I'm not blocking on this, especially if you do the work :).

FYI, I will overtake this PR and finish it.

@pedrobaeza thanks! Do you know approximately when you'll work on that?

pedrobaeza · 2023-08-11T10:38:38Z

Probably next week.

rafaelbn · 2023-08-12T15:41:46Z

I don't see many sense on splitting model definition from account.move.line links at least. No need to extra dependencies (all are in account). It's complicating the module tree, even increasing maintenance burden, while the features are code are very simple to be maintained even if packed.

I agree 👍🏼 , thank you!

In our use case we need the tags but not the link with account.move.line because the tags are set on analytic accounts (it was possible until v11).

@sbidoul then you set account.analytic.tag in account.analytic.account and you don't want that this tags are propagated to account.move.line ? I don't catch it. Maybe we can talk about this in other moment (for me it is interesting)

but then we also need the group to show the analytic tags on move lines only to selected users.

Interesting too. I guess the users with permissions in analytic with an extra security group can.

FYI, I will overtake this PR and finish it.

Awesome!

Thank you ver much.

victoralmau · 2023-08-16T09:10:23Z

The following changes have been done:

Added checkbox at the billing configuration level for analytical tags.
Removed the active / inactive check in the labels.
Color field added in tree and form view.
Define the corresponding groups in the company_id field.
Analytic account field is not mandatory now to create labels.
The process of creating analytical items is simplified; now only analytical tags are set in the corresponding analytical items (those with the same or no analytical account defined in the label).
Create base tests to be able to use them in other modules that extend this functionality.
Add more use cases in tests.

victoralmau · 2023-08-22T06:16:44Z

Changes done.

sbidoul

Thank you so much @victoralmau !

rafaelbn · 2023-08-22T09:11:36Z

I will finish my review today! This evening 😄

pedrobaeza · 2023-08-22T09:24:12Z

I'm thinking on reintroducing the distributions on tags as others have commented, as a way of "templating" this without defining a distribution model. What do you think about it?

victoralmau · 2023-08-22T10:26:50Z

I'm thinking on reintroducing the distributions on tags as others have commented, as a way of "templating" this without defining a distribution model. What do you think about it?

This is a very good point.

Previously (up to v15) we all modeled the analytical distribution with labels, but now in v16 the concept of distributions has been introduced.

Advantages / disadvantages of each option.

Option A: Keep the distributions in the labels.

Disadvantage: Would duplicate functionality with distributions.
Advantage: It would work the same as in v15 and before.
Advantage: It will not be necessary to define a distribution on the invoice lines.
Disadvantage: It will be necessary to create the analytical entries when there is no distribution defined. What happens if there are labels and analytical distribution?
Advantage: No need to define distributions in OCA modules that add analytics (e.g. project_stock).

Option B: Leave the analytical tags without distributions.

Disadvantage: Having to define the concept of distributions in OCA modules that add analytics (e.g. project_stock).
Advantage: Not having to add code to create analytical entries without distributions.

Maybe option A is the most reasonable, what do others think?

sbidoul · 2023-08-22T13:21:36Z

I'm thinking on reintroducing the distributions on tags as others have commented, as a way of "templating" this without defining a distribution model. What do you think about it?

@pedrobaeza Can this be done in another module, or in a second step after merging this PR?

sbidoul · 2023-08-22T13:27:19Z

Disadvantage: Would duplicate functionality with distributions

To me this justifies doing another analytic distributions mechanisms in a different module.

victoralmau · 2023-08-22T13:48:10Z

Disadvantage: Would duplicate functionality with distributions

To me this justifies doing another analytic distributions mechanisms in a different module.

Ok, thinking about this again, I think the best thing to do would be:

account_analytic_tag_distribution

A checkbox is added in Invoicing > Configuration to enable distribution at analytic tag level.
It hides the Analytic Distribution + Analytic Distribution columns menu if the checkbox is checked.
The possibility to define the analytical distribution on tags is "restored".
When defining an analytical label on an invoice line, what is defined on the labels will be defined as distribution.

In this way, I think it avoids "duplicating" the operation of the analytical distributions and could be used in one way or another.

pedrobaeza

OK about a separated module for distributions.

rafaelbn

Hello,

Thank you for this great work @victoralmau

After review, I see this (let me know your opinions):

In Invoice lines, set Analytic Tags after Analytic

Go to Accounting -> Accounting -> Journal Items , Then we cannot group by "Analytic Account"

Go to Accounting -> Accounting -> Management -> Analytic Items, change label "Tags" by "Analytic Tags" for better fields order (as alphabetical) and understanding

In a separate comment I can give my personal opinion about distributions.

Thank you very much! ❤️

rafaelbn · 2023-08-18T17:12:24Z

account_analytic_tag/README.rst

+=============
+
+To manage analytic account tags using this module, you need to:
+


Go to Accounting -> Configuraton -> Settings -> Activate "Analytic Tags"

OCA-git-bot · 2023-08-27T13:00:02Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

rafaelbn · 2023-08-27T14:41:32Z

Hello, this is my opinion about a new module for distributions.

The most important writing this down here is that I don't want to be right, I am aware that there are many different types of Odoo implementations and different types of companies and clients. I don't want to convince anyone. I just want to provide a vision that may help someone else make their own decisions. In fact, I really like reading your opinions and decisions and they make me wonder why and what I can learn from it.

So...

If we in OCA reintroduce analytic distributions in tags with are confusing users further. As for every one in Odoo distributions are done in Analytic Plans (account.analytic.plan). This re-introducing feature for me, maybe could be decision prompted by the effort to understand the change and the technical dependencies of other modules.

I'm not opposed to anything 😄, this is an open community, but I do think that in addition to having a separate module we should make a separate repository like https://github.com/OCA/account-analytic-tags-distribution . Because we must keep an open scenario where contributors extend Odoo using Odoo analytics distributions with their analytics Plans.

In other case, this will be really confusing when developing modules around distributions in Analytic Plans like Odoo Core and Analytic Tags as older versions. In Odoo 18 nobody (new contributors) will remember about why are Distributions in Tags talking around product vision.

You have done a really nice analysis 👍🏼 take this point of view just if it gives you more perspective:

Perfect migrations from Odoo 15 to Odoo 16 for me is like:

If account.analytic.tag has fields distribution "False" Then maintain it in account.analytic.tag
If account.analytic.tag has fields distribution "True" Then migrate it to account.analytic.plan

Opining and introducing more points about this great 🥇 pros and cons analysis:

Advantages / disadvantages of each option.

Option A: Keep the distributions in the labels.

Disadvantage: Would duplicate functionality with distributions.

Advantage: It would work the same as in v15 and before.

For me this is a disadvantage as you are separating important concepts in Odoo Core. Imagine you decide in Odoo 13 to maintain account.invoice model as a similar example.

Advantage: It will not be necessary to define a distribution on the invoice lines.

Disadvantage: It will be necessary to create the analytical entries when there is no distribution defined. What happens if there are labels and analytical distribution?

Advantage: No need to define distributions in OCA modules that add analytics (e.g. project_stock).

For me this is not a Advantage as you don't need to define ~~distributions~~ analytic plans. The is always one by default that works in the same way as I you have in Odoo 15 only analytic accounts without distributions. Really I don't see the problem with project_stock
Are you talking around UX introducing invoices? I would really love to go deep in this problem that I don't see.

Disadvantage: You are missing the amazing new great functionality around Applicability in Analytic Plans with all the automation rules. https://www.odoo.com/documentation/16.0/applications/finance/accounting/reporting/analytic_accounting.html#analytic-accounting-analytic-plans

Option B: Leave the analytical tags without distributions.

Disadvantage: Having to define the concept of distributions in OCA modules that add analytics (e.g. project_stock).

As said before, you don't have to define concept of distributions. It's not a must, it is an option. Analytic Plans are just plans of analytic account, grouped, that maybe you you may want to distribute among them or not.

Advantage: Not having to add code to create analytical entries without distributions.

Maybe option A is the most reasonable, what do others think?

If you think is needed is OK. I think all the time about have we have (a lot) to not repeat my self (DRY)

"Analytic Distribution Models" account.analytic.distribution.model are not mandatory as they are rules for automations similar to previous "Analytic Defaults Rules" account.analytic.default
"Analytic Plans" account.analytic.plan are like "Dimensions", they group Analytic accounts and give you the possibility to make distributions or not.
Analytic Accounts, are values of the Plans (dimensions) with a direct relations
Analytic Tags account.analytic.tag in this PR are another level of analysis, for other values and dimensions that are not related so you can assign freely.

My best!, sorry for the length! (my fault)

💯 🥇 👍🏼

Thank you very much for this work! 👍🏼

victoralmau · 2023-08-28T06:59:40Z

Changes done:

In Invoice lines, set Analytic Tags after Analytic
Change tag_ids position in analytical items (same as in v15).

Other comments:

There is no analytical account field in the Journal Items, they are built according to the analytical distribution.
Tags is the correct name of the field (same as in v15) in the analytical items.
The configuration section of the readme already indicates the activation of the "Analytic Tag" checkbox.

pedrobaeza · 2023-08-28T07:01:16Z

@victoralmau put Co-Authored-By: ... in the commit for your co-authorship.

Restore account.analytic.tag TT44062 Co-authored-by: victoralmau

rafaelbn · 2023-08-29T13:42:06Z

/ocabot merge nobump

Fantastic! You are awesome! 💚

OCA-git-bot · 2023-08-29T13:42:39Z

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-572-by-rafaelbn-bump-nobump, awaiting test results.

OCA-git-bot · 2023-08-29T13:48:06Z

Congratulations, your PR was merged at 70bde0c. Thanks a lot for contributing to OCA. ❤️

Yadier-Tecnativa force-pushed the 16.0-add-account_analytic_tag branch 4 times, most recently from 5a73f68 to ea90829 Compare July 11, 2023 02:26

pedrobaeza added this to the 16.0 milestone Jul 13, 2023

pedrobaeza requested changes Jul 13, 2023

View reviewed changes

Yadier-Tecnativa force-pushed the 16.0-add-account_analytic_tag branch 2 times, most recently from 32f2c0d to 650b157 Compare July 17, 2023 23:21

Yadier-Tecnativa force-pushed the 16.0-add-account_analytic_tag branch 2 times, most recently from 87b62ce to d36983e Compare July 19, 2023 17:27

Yadier-Tecnativa requested a review from pedrobaeza July 19, 2023 17:27

rafaelbn added the needs review label Jul 26, 2023

rafaelbn self-requested a review July 26, 2023 20:24

rafaelbn requested changes Aug 9, 2023

View reviewed changes

sbidoul reviewed Aug 11, 2023

View reviewed changes

victoralmau force-pushed the 16.0-add-account_analytic_tag branch from d36983e to e4951af Compare August 16, 2023 09:04

victoralmau force-pushed the 16.0-add-account_analytic_tag branch from a0742ba to 6f487a2 Compare August 22, 2023 06:13

sbidoul approved these changes Aug 22, 2023

View reviewed changes

sbidoul requested a review from rafaelbn August 22, 2023 08:32

pedrobaeza approved these changes Aug 22, 2023

View reviewed changes

victoralmau mentioned this pull request Aug 24, 2023

[16.0][ADD] account_analytic_tag_distribution: New module #587

Merged

1 task

rafaelbn approved these changes Aug 27, 2023

View reviewed changes

OCA-git-bot added the approved label Aug 27, 2023

rafaelbn mentioned this pull request Aug 27, 2023

How the OCA will deal with the disappearance of analytic tags #551

Closed

2 tasks

OCA-git-bot added the ready to merge label Aug 27, 2023

victoralmau force-pushed the 16.0-add-account_analytic_tag branch from 6f487a2 to 7f99d4c Compare August 28, 2023 06:53

[ADD] account_analytic_tag

28e3d95

Restore account.analytic.tag TT44062 Co-authored-by: victoralmau

victoralmau force-pushed the 16.0-add-account_analytic_tag branch from 7f99d4c to 28e3d95 Compare August 28, 2023 07:02

OCA-git-bot added the bot is merging ⏳ label Aug 29, 2023

OCA-git-bot merged commit 318f04b into OCA:16.0 Aug 29, 2023
4 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Aug 29, 2023

		from odoo import fields, models


		class AccountAnalyticDistribution(models.Model):



		class AccountAnalyticLine(models.Model):
		_inherit = "account.analytic.line"

		@@ -0,0 +1 @@
		This module adds account analytic tags on accounts.

	This module adds account analytic tags on accounts.
	This module restores the account analytic tags as a method for categorizing analytic entries, selectable from the invoices, and transferred to them on publishing.

		@@ -0,0 +1 @@
		* Yadier Quesada - Tecnativa <[email protected]>

-* Yadier Quesada - Tecnativa <yadier.dequesada@tecnativa.com>
+* `Tecnativa <https://www.tecnativa.com>`__:
+  * Yadier Quesada

		=============

		To manage analytic account tags using this module, you need to:

[16.0][ADD] account_analytic_tag #572

[16.0][ADD] account_analytic_tag #572

Conversation

Yadier-Tecnativa commented Jul 11, 2023

Yadier-Tecnativa commented Jul 11, 2023

pedrobaeza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yadier-Tecnativa commented Jul 17, 2023 • edited Loading

Yadier-Tecnativa commented Jul 19, 2023

rafaelbn commented Jul 26, 2023

pepetreshere commented Aug 2, 2023

pedrobaeza commented Aug 2, 2023

rafaelbn commented Aug 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbidoul commented Aug 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedrobaeza commented Aug 11, 2023

pedrobaeza commented Aug 11, 2023

sbidoul commented Aug 11, 2023

pedrobaeza commented Aug 11, 2023

rafaelbn commented Aug 12, 2023

victoralmau commented Aug 16, 2023

victoralmau commented Aug 22, 2023

sbidoul left a comment

Choose a reason for hiding this comment

rafaelbn commented Aug 22, 2023

pedrobaeza commented Aug 22, 2023

victoralmau commented Aug 22, 2023

sbidoul commented Aug 22, 2023

sbidoul commented Aug 22, 2023

victoralmau commented Aug 22, 2023

pedrobaeza left a comment

Choose a reason for hiding this comment

rafaelbn left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OCA-git-bot commented Aug 27, 2023

rafaelbn commented Aug 27, 2023

victoralmau commented Aug 28, 2023 • edited Loading

pedrobaeza commented Aug 28, 2023

rafaelbn commented Aug 29, 2023

OCA-git-bot commented Aug 29, 2023

OCA-git-bot commented Aug 29, 2023

Yadier-Tecnativa commented Jul 17, 2023 •

edited

Loading

rafaelbn left a comment •

edited

Loading

victoralmau commented Aug 28, 2023 •

edited

Loading