-
-
Notifications
You must be signed in to change notification settings - Fork 425
Add 10.0 project task analytic tag #88
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
Conversation
|
Hey @acm1pt-colorado, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
project_task_analytic_tag/README.rst
Outdated
| Project Task Analytic Tag | ||
| ========================= | ||
|
|
||
| This module extends the functionality of project and mrp_repair to support to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mrp_repair?
It is not in dependency list and anyway would simply need another module if you'd wish to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isnot this module overlapping with https://github.com/OCA/project/tree/10.0/project_categ and https://github.com/OCA/project/tree/10.0/project_categ_issue?
I am not so much in favor of adding a new category system without clear value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @elicoidal , you are right about README, it's a mistake about copy & paste from other module.
But this module is not overlapping with project category. The target to add analytic tags to project task is to create analytic lines (timesheet lines) with right tag.
This way you can set tags like "Design", "Develop", "Testing"... on your timesheet lines, and you will be able to get analytic about type of work you was doing. Also interesting in combination with analytic dimensions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a note in the README
|
I copy this comment because it's hidden by outdated change, but I think is usefull to see for other reviewers
|
|
@angelmoya then I would suggest to make it permanent in the README ;) |
| :target: http://www.gnu.org/licenses/agpl-3.0-standalone.html | ||
| :alt: License: AGPL-3 | ||
|
|
||
| Project Task Analytic Tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line before for the document title
project_task_analytic_tag/README.rst
Outdated
| Project Task Analytic Tag | ||
| ========================= | ||
|
|
||
| This module extends the functionality of project to support to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"This module extends the functionality of project to support Tags in timesheet and the possibility to add them from the projects."
project_task_analytic_tag/README.rst
Outdated
|
|
||
| To install this module, you need to: | ||
|
|
||
| - First Add in the addons path the module and install in your odoo. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odoo
project_task_analytic_tag/README.rst
Outdated
|
|
||
| To use this module, you need to: | ||
|
|
||
| - Go to project and enter in a task section, here you only need to go to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make better use of enumerated list "#. " for procedures. For example:
#. Go to project and enter in a task section,
#. Go to the page timesheet
#. You can add a new element.
#. When you add the tag in the extra Information, the tag is automatically set for newly added element.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When i read the code, I dont get all above mentioned feature though.
Anything I missed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really no more code is needed, now on tasks, timesheets page are analytic entries (previously are timesheet_sheet_lines that are related).
Then this module just show tag_ids from analytic_account_line on project_task. And set analytic_tag_ids on project task to define default analytic_tag_ids for all timesheets (account_analytic_line) for this task.
project_task_analytic_tag/README.rst
Outdated
| extra-Information, when you add a new element the tag is set auto. | ||
|
|
||
|
|
||
| Known issues / Roadmap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the section
|
@elicoidal thanks for the comments all the changes updated |
|
I wonder if OCA/project is not better suited for this. From the functional PoV, it's not clear to me the use case for this. |
|
Hi @dreispt , we also were thinking about puting it on OCA/project , but finally we thought that final scope is to get correct analytic accounting. You are wrong about this "several analytic accounts on the same task", it's not for analytic accounts, is for analytic tags. This way you can set type of work you are doing in one task. Without this module you can not set analytic tags for this account.analytic.lines |
|
@angelmoya I see, but what is the point. Can you give a real life example of haw this is used? |
|
Sorry @dreispt , I said you are wrong because you said account instead tag, and I thought that you can undertand the scope using analytic tags. Ok, for example.
My real use case is more easy, I improve the use of mrp_repair, to be able to create projects and task. Then all task created from RMA has analytic tag "RMA" and then you can get analytic account for RMA. All this could be better if you install also #87 this way you can group analytic tags on dimensions like:
|
|
For the example, IMO it would be better to have an Analytic Account on Tasks. This has been done on OCA/project (although looking the module name - |
|
Since v9, there's a field analytic_account_id in tasks and issues... |
|
@pedrobaeza Thanks! I guess it shows I'm still on v8 😛 |
|
Yeah, at task or issue level you can predefine an analytic account to be filled for timesheet entries, so theoretically you can switch this analytic account for changing where to put the entry. We have been working on restoring analytic hierarchy. Have you seen it? |
|
I haven't read the RFC yet, but I'm aware it is being discussed. |
|
It's no more an RFC, but an implementation: https://github.com/OCA/account-analytic/tree/9.0/account_analytic_parent and an improvement here: #91 |
|
@dreispt with analytic dimension you can support things like WBS, and I think that it could be better than use analytic parent. You can think on this structure with analytic parent:
With previous structure, if you use parent account you need to create one account for each Materials of each Project. And you need to set task on Materials or on Hours. Also on BI you can not view amounts group rightly. If you use dimensions you can do things like:
And you can use on BI, and group on diferents dimensions on diferent order. Is hard to me to explain in english, but I really think that tags with dimensions are better way to do analytic. And for BI is better than use parent account. On v8 I did things like add parent_id on BI, and parent_parent_id, and product_category, to get similar functionality, and know this could be done automatically. |
|
That was a great explanation, and I think I understand your point.
Now each analytic line should have be assigned to a "analytic combination", with values for each of the Dimensions. have a look at this. Maybe what I describe if far from being possible in practice and your approach is a good short term solution. We're just discussing here. |
|
@dreispt @pedrobaeza one more review to aprove? |
|
|
||
| analytic_tag_ids = fields.Many2many( | ||
| comodel_name='account.analytic.tag', | ||
| relation='default_tag_account_tag', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convention is for this table to end with _rel
| "license": "AGPL-3", | ||
| "depends": [ | ||
| 'project', | ||
| "hr_timesheet" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is related to analytic_tag_dimension right?
Does it make sense to add it as a dependency?
If not, at least that module should be mentioned in the README, as a way to use the information added here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dreispt , it's not related, and is not a dependency.
You can use it alone, to set analytic tag on analytic lines created on timesheet.
We only talk about the other module to explain why we decided to improve analytic tags instead of migrate analytic account parent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dreispt The dependency is not necessary. As @angelmoya said you can use it alone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dreispt Do you agree that there is no need for dependency? I'm going to add in the readme file for more info as you said.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
Rebuilding for testing @angelmoya |
|
@angelmoya @acm1pt-colorado could you please rebase? I cannot test it. Travis fails and runbot doesn't build 😢 |
|
Done, I will check new travis error
El vie., 14 sept. 2018 17:51, Rafael Blasco <notifications@github.com>
escribió:
… @angelmoya <https://github.com/angelmoya> @acm1pt-colorado
<https://github.com/acm1pt-colorado> could you please rebase? I cannot
test it. Travis fails and runbot doesn't build 😢
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#88 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGcAhZvJjUq4SNJsxNBRYXpt5VGfqqUmks5ua9BygaJpZM4NDJtJ>
.
|
|
Hi @angelmoya , I've tested functionally and it's 👍 But I don¡t understand why in this PR are you changing files of other modules: https://github.com/OCA/account-analytic/pull/88/files#diff-de1af174f37e99cfcd5af533f2eeb68f ❓ 😄 |
rafaelbn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally 👍
But in this PR "Files changed 934" 😲
8c5d3c8 to
85a5ae5
Compare
|
OK @rafaelbn , maybe was a problem with rebase, I did it again and do force push to do in one commit and only update my module files. |
85a5ae5 to
6e4d8c7
Compare
6e4d8c7 to
42ac8cc
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
Project Task Analytic Tag
This module extends the functionality of project to support to
add Tags in the timesheet and allow you to use the tags of the projects in your timesheet