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

[16.0] [MIG] account_fiscal_year_closing #248

Merged
merged 27 commits into from
Jan 23, 2025

Conversation

Borruso
Copy link

@Borruso Borruso commented Jun 14, 2023

Migration account_fiscal_year_closing from 14.0 to 16.0

--
Confermo di aver firmato il CLA https://odoo-community.org/page/cla e di aver letto le linee guida su https://odoo-community.org/page/contributing

@Borruso Borruso mentioned this pull request Jun 14, 2023
7 tasks
@miguel-binaural
Copy link

miguel-binaural commented Jun 27, 2023

Will this be merged?

@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from b05e291 to 4917d69 Compare July 21, 2023 12:42
Copy link

@SirAionTech SirAionTech left a comment

Choose a reason for hiding this comment

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

account_type_id = fields.Many2one(
comodel_name="account.account.type",
string="Account type",
account_type = fields.Selection(

Choose a reason for hiding this comment

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

This change requires a migration, otherwise when a database is migrated the existing records will have an empty account_type instead of having the value corresponding to their old account_type_id.
You can have a look at https://oca.github.io/OpenUpgrade for better understanding what a migration is.

Since the migration is usually not trivial and it does not really affect the module's behavior, sometimes this can be done later; this will be probably decided by whoever wants to merge this module.

If this is the case, please at least write a note in the roadmap of the module.

Copy link
Author

@Borruso Borruso Oct 6, 2023

Choose a reason for hiding this comment

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

create migration
await you feedback

Choose a reason for hiding this comment

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

👍

not in [
"asset_receivable", # Receivable
"asset_current", # Current Assets
"income", # Current Assets

Choose a reason for hiding this comment

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

Suggested change
"income", # Current Assets
"income", # Income

Copy link
Author

@Borruso Borruso Oct 6, 2023

Choose a reason for hiding this comment

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

done

Choose a reason for hiding this comment

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

👍

@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from 4917d69 to a36c164 Compare October 6, 2023 10:35
@Borruso Borruso requested a review from SirAionTech October 6, 2023 10:37
@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch 3 times, most recently from 8a0946f to 92f8abb Compare October 6, 2023 11:07
Copy link

@SirAionTech SirAionTech left a comment

Choose a reason for hiding this comment

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

Commit 21851e9 is missing from history

account_type_id = fields.Many2one(
comodel_name="account.account.type",
string="Account type",
account_type = fields.Selection(

Choose a reason for hiding this comment

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

👍

not in [
"asset_receivable", # Receivable
"asset_current", # Current Assets
"income", # Current Assets

Choose a reason for hiding this comment

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

👍

@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from 92f8abb to 18480fd Compare October 6, 2023 11:15
@Borruso
Copy link
Author

Borruso commented Oct 6, 2023

Commit 21851e9 is missing from history

done

@Borruso Borruso requested a review from SirAionTech October 6, 2023 11:16
@gjlong68
Copy link

V16
image

@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch 2 times, most recently from 596da34 to 3142b69 Compare October 19, 2023 14:00
@Borruso
Copy link
Author

Borruso commented Oct 20, 2023

V16 image

@gjlong68, can you check again, please?

@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from 3142b69 to 6d874db Compare October 20, 2023 08:33
Copy link
Contributor

@GSLabIt GSLabIt left a comment

Choose a reason for hiding this comment

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

multi company fix

<field name="global" eval="True" />
<field
name="domain_force"
>['|', ('company_id', '=', False), ('company_id', 'child_of', [user.company_id.id])]</field>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
>['|', ('company_id', '=', False), ('company_id', 'child_of', [user.company_id.id])]</field>
>['|', ('company_id', '=', False), ('company_id', 'in', company_ids)]</field>

<field name="global" eval="True" />
<field
name="domain_force"
>['|', ('company_id', '=', False), ('company_id', 'child_of', [user.company_id.id])]</field>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
>['|', ('company_id', '=', False), ('company_id', 'child_of', [user.company_id.id])]</field>
>['|', ('company_id', '=', False), ('company_id', 'in', company_ids)]</field>

@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from 6d874db to 646f046 Compare October 26, 2023 08:04
@Borruso
Copy link
Author

Borruso commented Oct 26, 2023

multi company fix

@GSLabIt thanks for suggest
added migration

@gjlong68
Copy link

Borruso:OK

@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from 646f046 to 92367bb Compare November 13, 2023 08:19
@mde-scopea
Copy link

Hi guys, thanks for your job.

Has anyone tested the migration script?

On my side, it's KO (I'm on Odoo.sh), the pre-mig script doesn't work.
IDs, use with env.ref("account.data_account_type*), don't exist on the migrated system (when the pre script runs)

@SirAionTech
Copy link

Has anyone tested the migration script?

I didn't

On my side, it's KO (I'm on Odoo.sh), the pre-mig script doesn't work.
IDs, use with env.ref("account.data_account_type*), don't exist on the migrated system (when the pre script runs)

Then please add that as a review to the PR, have a look at https://odoo-community.org/resources/review if you don't know how.

@pedrobaeza
Copy link
Member

Please include 55998e8 in this PR.

@pedrobaeza
Copy link
Member

/ocabot migration account_fiscal_year_closing

@lk-eska
Copy link

lk-eska commented May 10, 2024

These 2 PR's are will be necessary as well:

#288
#294

When creating from scratch, Mapping is used as an Array instead of simple id (due to many in another many).
When coming from a template, id is used
To take into account this particularity, check the type before taking the id of the array.
@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from b54b04a to 0e5cd59 Compare August 1, 2024 10:19
UPDATE {table} aa
SET account_type = atm.account_type
FROM account_type_map atm
WHERE atm.user_type_id = aa.user_type_id
Copy link

Choose a reason for hiding this comment

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

ERROR: column aa.user_type_id does not exist
LINE 35:             WHERE atm.user_type_id = aa.user_type_id
                                              ^
HINT:  Perhaps you meant to reference the column "atm.user_type_id".

user_type_id does not exist in any of the two tables? (account_fiscalyear_closing_type_template, account_fiscalyear_closing_type)

Copy link
Author

Choose a reason for hiding this comment

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

yes field user_type_id become account_type

odoo/odoo@26b2472 user_type_id -> account_type account.account.type

Copy link

@SirAionTech SirAionTech left a comment

Choose a reason for hiding this comment

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

I proposed Borruso#1 to support the flow where accounting data is created without loading a template chart of accounts

@peterromao
Copy link

Hello guys,

Where can I translate this module for version 16? It's not on Weblate. Also as I see it it is not yet merged into the 16.0 branch either right?

@andreampiovesana
Copy link

andreampiovesana commented Nov 7, 2024

can you enable runboat?
is good for functional review and/or merge?

@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from a3c19a5 to cc851f5 Compare November 8, 2024 09:59
Although we originally developped the module, we figured out soon
that this approach is not correct, so we prefer to not appearing
in the credits.
@Borruso Borruso force-pushed the 16.0-mig-account_fiscal_year_closing branch from cc851f5 to ff71bb5 Compare November 8, 2024 10:00
@Borruso
Copy link
Author

Borruso commented Nov 8, 2024

Please include 55998e8 in this PR.

commit included

Copy link

@MaurizioPellegrinet MaurizioPellegrinet left a comment

Choose a reason for hiding this comment

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

Functional test: OK

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

Copy link

@AinohaBH AinohaBH left a comment

Choose a reason for hiding this comment

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

Functional: LGTM

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, MERGE?

Copy link

@salvorapi salvorapi left a comment

Choose a reason for hiding this comment

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

LGTM

@Borruso
Copy link
Author

Borruso commented Jan 23, 2025

@pedrobaeza merge?

PS
After merge I will create PR for v17 and v18

@pedrobaeza
Copy link
Member

Well, I discourage the use of this module, and I don't think it's really needed in Italy, but merging due to reviews.

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-248-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit decb974 into OCA:16.0 Jan 23, 2025
8 of 9 checks passed
@OCA-git-bot
Copy link
Contributor

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

@lk-eska
Copy link

lk-eska commented Jan 23, 2025

Well, I discourage the use of this module, and I don't think it's really needed in Italy, but merging due to reviews.

/ocabot merge nobump

then how do we achieve the closing process? would you point us to a direction? thanks.
@pedrobaeza

@pedrobaeza
Copy link
Member

Legislations usually don't talk about any "closing process". They just ask for financial books. The closing process is an old artificial method for being able to obtain that books with the limited resources that exist some time ago. A lot of people say the same that "systems have to make the closing", but we are working since a lot in Odoo without it, and other systems like SAP or Navision do the same.

@lk-eska
Copy link

lk-eska commented Jan 23, 2025

I believe people insist using this module because this is a legal requirement in their country (we are also in this boat).

@pedrobaeza
Copy link
Member

That's what I think that it's not really required in your country...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.