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

[14.0][ADD] account_analytic_line_split: add new module #681

Open
wants to merge 2 commits into
base: 14.0
Choose a base branch
from

Conversation

WesleyOliveira98
Copy link

@WesleyOliveira98 WesleyOliveira98 commented Aug 16, 2024

Copy link
Member

@douglascstd douglascstd left a comment

Choose a reason for hiding this comment

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

[FUNCIONTAL REVIEW] [NEED TO CHECK]

  1. Test to split some analytic item (OK)
    image

  2. It could be better with the amount field
    image

  3. The child_ids table is duplicated, maybe it's an effect if install other module: (account_analytic_wip). If the modules use same field it could be a problem in the future?
    image
    This field is created in analytic_activity_based_cost module on:


    I guess this module must to be included on dependency in manifest.py, is it make sense?

@WesleyOliveira98 WesleyOliveira98 force-pushed the 14.0-add-account_analytic_line_split branch 5 times, most recently from 4fe0480 to 83cc187 Compare August 22, 2024 14:22
@WesleyOliveira98 WesleyOliveira98 force-pushed the 14.0-add-account_analytic_line_split branch from 83cc187 to c440a86 Compare August 22, 2024 15:25
@WesleyOliveira98
Copy link
Author

2. It could be better with the amount field

This tree view is added with analytic activity based cost

@WesleyOliveira98
Copy link
Author

3. I guess this module must to be included on dependency in manifest.py, is it make sense?

This module is in Alpha, when I tried to depend on it this error was triggered

account_analytic_line_split (Beta) depends on analytic_activity_based_cost (Alpha)

And I think it doesn't make much sense to depend on this module because the purposes of the modules are different, I change the name of the fields to parent_line_id and child_line_ids, and in the view, I control the visibility of the fields only displaying them when they are filled in

@WesleyOliveira98
Copy link
Author

I also added unit tests, this pull request is ready for review now

@WesleyOliveira98 WesleyOliveira98 marked this pull request as ready for review August 22, 2024 15:36
@WesleyOliveira98
Copy link
Author

To review this pull request, you must uninstall account_analytic_no_lines because this module disables the generation of an analytic line from a move line

@WesleyOliveira98
Copy link
Author

ping @OCA/accounting-maintainers

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.

2 participants