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.

Sign up for GitHub

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

Jump to bottom

[16.0][MIG] mis_builder_cash_flow: Migration to version 16.0 #1049

Merged
merged 27 commits into from
Sep 19, 2023
Merged

[16.0][MIG] mis_builder_cash_flow: Migration to version 16.0 #1049

merged 27 commits into from
Sep 19, 2023

Conversation

stefan-tecnativa
Copy link

cc @Tecnativa TT43621

@pedrobaeza @andreampiovesana please review!

This PR is a superseed of #1046

@pedrobaeza pedrobaeza mentioned this pull request Jun 27, 2023
Closed
@pedrobaeza
Copy link
Member

/ocabot migration mis_builder_cash_flow

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Jun 27, 2023
@OCA-git-bot
Copy link
Contributor

The migration issue (#924) has not been updated to reference the current pull request because a previous pull request (#1046) is not closed.
Perhaps you should check that there is no duplicate work.
CC @heliaktiv

@pedrobaeza
Copy link
Member

/ocabot migration mis_builder_cash_flow

@OCA-git-bot OCA-git-bot mentioned this pull request Jun 8, 2023
Open
7 tasks
pedrobaeza
pedrobaeza reviewed Jun 27, 2023
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Co-Author line is not correctly put. You only need to add your name, but as fully qualified with email:

Co-Authored-By: Stefan Ungureanu <[email protected]>

mis_builder_cash_flow/i18n/es.po Outdated Show resolved Hide resolved
mis_builder_cash_flow/models/mis_report_instance.py Outdated Show resolved Hide resolved
mis_builder_cash_flow/report/mis_cash_flow_views.xml Outdated Show resolved Hide resolved
mis_builder_cash_flow/views/mis_cash_flow_forecast_line_views.xml Outdated Show resolved Hide resolved
@stefan-tecnativa
Copy link
Author

Reviewed all the changes. Thanks for the suggestions. Now I also know how to add correctly the Co-Authored

andreampiovesana
andreampiovesana suggested changes Jun 29, 2023
View reviewed changes
Copy link

@andreampiovesana andreampiovesana left a comment •
edited
Loading

Choose a reason for hiding this comment

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

you should rename account_internal_type (old name) in account_type
and in mis_cash_flow_forecast_line_view_form there are 2 company_id fields

andreampiovesana
andreampiovesana approved these changes Jun 29, 2023
View reviewed changes
Copy link

@andreampiovesana andreampiovesana left a comment

Choose a reason for hiding this comment

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

LGTM

andreampiovesana
andreampiovesana suggested changes Jun 29, 2023
View reviewed changes
Copy link

@andreampiovesana andreampiovesana left a comment •
edited
Loading

Choose a reason for hiding this comment

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

                    <field
                        name="account_id"
                        domain="[('company_id', '=', company_id)]"
                        groups="base.group_multi_company"
                    />

you should remove groups="base.group_multi_company"
immagine
immagine (2)

@stefan-tecnativa stefan-tecnativa force-pushed the 16.0-mig-mis_builder_cash_flow branch 2 times, most recently from 94ab9ba to 466d1e8 Compare June 30, 2023 05:52
@stefan-tecnativa
Copy link
Author 

                    <field
                        name="account_id"
                        domain="[('company_id', '=', company_id)]"
                        groups="base.group_multi_company"
                    />

you should remove groups="base.group_multi_company" immagine immagine (2)

Thanks for all the suggestions, now it is corrected. It is necessary to add two fields, if not the account_id field returns an error because company_id is not allways in the view.

@pedrobaeza
Copy link
Member

@stefan-tecnativa what you have to add 2 times is the field company_id, not the other, one invisible=1 and without group, and the other visible with the group.

@stefan-tecnativa stefan-tecnativa force-pushed the 16.0-mig-mis_builder_cash_flow branch 2 times, most recently from 6d4d7b4 to f455c42 Compare July 3, 2023 06:34
ioans73
ioans73 approved these changes Sep 19, 2023
View reviewed changes
Copy link
Member

@ioans73 ioans73 left a comment

Choose a reason for hiding this comment

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

Functional review

pedrobaeza and others added 9 commits September 19, 2023 13:03
@pedrobaeza @stefan-tecnativa
[IMP] mis_builder_cash_flow: Consider states according selection
5ac0d4a 
"Target Moves" option in MIS report instance was previously ignored.

Now it's taken into account, but still excluding cancelled entries.
@ibuioli @stefan-tecnativa
Translated using Weblate (Spanish (Argentina))
32ae98c 
Currently translated at 100.0% (55 of 55 strings)

Translation: account-financial-reporting-13.0/account-financial-reporting-13.0-mis_builder_cash_flow
Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-13-0/account-financial-reporting-13-0-mis_builder_cash_flow/es_AR/
@rvalyi @stefan-tecnativa
[IMP] mis_builder_cash_flow: black, isort, prettier
c692223
@rvalyi @stefan-tecnativa
[MIG] mis_builder_cash_flow: Migration to 14.0
ed9fa18
@francesco-ooops @stefan-tecnativa
Added translation using Weblate (Italian)
2ac482d
@francesco-ooops @stefan-tecnativa
Translated using Weblate (Italian)
d4c3427 
Currently translated at 100.0% (55 of 55 strings)

Translation: account-financial-reporting-14.0/account-financial-reporting-14.0-mis_builder_cash_flow
Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-14-0/account-financial-reporting-14-0-mis_builder_cash_flow/it/
@JasminSForgeFlow @stefan-tecnativa
[MIG] mis_builder_cash_flow: Migration to 15.0
29f0d0e
@Ivorra78 @stefan-tecnativa
Translated using Weblate (Spanish)
4967849 
Currently translated at 100.0% (55 of 55 strings)

Translation: account-financial-reporting-15.0/account-financial-reporting-15.0-mis_builder_cash_flow
Translate-URL: https://translation.odoo-community.org/projects/account-financial-reporting-15-0/account-financial-reporting-15-0-mis_builder_cash_flow/es/
@heliaktiv @stefan-tecnativa
[IMP] mis_builder_cash_flow: black, isort, prettier
33cc465
andreampiovesana
andreampiovesana approved these changes Sep 19, 2023
View reviewed changes
Copy link

@andreampiovesana andreampiovesana left a comment

Choose a reason for hiding this comment

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

LGTM

@OCA-git-bot
Copy link
Contributor

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). 🤖

1 similar comment
@OCA-git-bot
Copy link
Contributor

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). 🤖

pedrobaeza
pedrobaeza requested changes Sep 19, 2023
View reviewed changes
mis_builder_cash_flow/i18n/es.po Outdated Show resolved Hide resolved
mis_builder_cash_flow/i18n/es.po Outdated Show resolved Hide resolved
pedrobaeza
pedrobaeza approved these changes Sep 19, 2023
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-1049-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

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). 🤖

@OCA-git-bot OCA-git-bot merged commit 43b5308 into OCA:16.0 Sep 19, 2023
6 checks passed
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ioans73 ioans73 ioans73 approved these changes

@andreampiovesana andreampiovesana andreampiovesana approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.