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][OU-ADD] account: Migration #4070

Merged
merged 3 commits into from
Oct 24, 2023
Merged

[16.0][OU-ADD] account: Migration #4070

merged 3 commits into from
Oct 24, 2023

Conversation

ngochung207
Copy link
Contributor

@ngochung207 ngochung207 commented Jul 8, 2023

Video test localhost:
https://youtu.be/f1vUk11DpiU

@@ -6,7 +6,7 @@ Module coverage 15.0 -> 16.0
+-------------------------------------------------+----------------------+-------------------------------------------------+
| Module | Status + Extra Information |
+=================================================+======================+=================================================+
| account | | |
Copy link
Contributor

Choose a reason for hiding this comment

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

Partial everybody :v

Copy link
Contributor

Choose a reason for hiding this comment

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

Todo: a dynamic way to fill anaylytic_distribution on account.move.line, sale.order.line for short is any model that inherit from analytic.mixin

@legalsylvain
Copy link
Contributor

legalsylvain commented Jul 8, 2023

/ocabot migration account

Depends on :

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Jul 8, 2023
@legalsylvain
Copy link
Contributor

@remytms could you take a look here ?

@duong77476-viindoo
Copy link
Contributor

@remytms could you take a look here ?

This PR haven't ready yet , but still me and my friend welcome all reviews from anybody especially on the analysis file

@duong77476-viindoo

This comment was marked as duplicate.

@legalsylvain
Copy link
Contributor

@duong77476. No need to write dependencies message. It will be done automatically by this script. (and the list will be updated regularly). #3979
thanks !
Note : You can put your PR in a draft state when the work is not ready to review.

@duong77476-viindoo
Copy link
Contributor

duong77476-viindoo commented Jul 8, 2023

@duong77476. No need to write dependencies message. It will be done automatically by this script. (and the list will be updated regularly). #3979 thanks ! Note : You can put your PR in a draft state when the work is not ready to review.

Okay, thanks, just a normal habit of me when a PR need to depend on something, cool tool btw keep it up

@legalsylvain
Copy link
Contributor

cool tool btw keep it up

thanks !
Well, when there are 100 PR per week (!), It's starting to become mandatory to be able to find your way around.

@duong77476-viindoo
Copy link
Contributor

@legalsylvain Do you have tools that allow to rebase automatically, as many PRs are coming , i always have to do it manually :((

@legalsylvain
Copy link
Contributor

@legalsylvain Do you have tools that allow to rebase automatically, as many PRs are coming , i always have to do it manually :((

You can call ocabot rebase (with \ before.)

If you have write access on the branch of the PR, it should work. Can you try ?

@ngochung207 ngochung207 changed the title [WIP][OU-ADD] account: Migration [WIP][16.0][OU-ADD] account: Migration Jul 8, 2023
@duong77476-viindoo
Copy link
Contributor

duong77476-viindoo commented Jul 8, 2023

\ocabot

ocabot rebase

Not working.
IMO, the person who has write access to his/her PR should be able to call that

@OCA-git-bot
Copy link
Contributor

Sorry @duong77476 you are not allowed to rebase.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@ngochung207 ngochung207 changed the title [WIP][16.0][OU-ADD] account: Migration [16.0][OU-ADD] account: Migration Jul 10, 2023
@ngochung207 ngochung207 marked this pull request as ready for review July 10, 2023 06:23
@ngochung207
Copy link
Contributor Author

@OCA-git-bot rebase

@ngochung207
Copy link
Contributor Author

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Sorry @ngochung207 you are not allowed to rebase.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@duong77476-viindoo
Copy link
Contributor

duong77476-viindoo commented Aug 21, 2023

Thanks @chrisandrewmann . But as you can see someone else has attempted and worked. If you could provide a video , show me your odoo conf as well, i might be able to check it out. Otherwise , i really hard for me though

@chrisandrewmann
Copy link

Apologies @duong77476 my mistake.
Was not loading openupgrade framework correctly in conf! Working well now.

@duong77476-viindoo
Copy link
Contributor

Apologies @duong77476 my mistake. Was not loading openupgrade framework correctly in conf! Working well now.

Thank you very much, let's me know if something wrong or need improvement.

SET journal_id = aml.journal_id
FROM account_move_line aml
WHERE aml.id = aal.move_line_id
""",
Copy link
Member

@sbidoul sbidoul Sep 12, 2023

Choose a reason for hiding this comment

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

I got insert or update on table "account_analytic_line" violates foreign key constraint "account_analytic_line_journal_id_fkey" on this update.

We should probably drop the foreign key constraint before applying this:
ALTER TABLE account_analytic_line DROP CONSTRAINT IF EXISTS account_analytic_line_journal_id_fkey;

Because that constraints from v15 and earlier relates to analytic journals.

Copy link
Member

Choose a reason for hiding this comment

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

Another issue I've encountered around here is with analytic lines linked to the Timesheets Journal, which prevent the journal_id foreign key to apply. I've set journal_id to NULL on these records as a workaround. Not sure where the best place to do that is.

@@ -0,0 +1,563 @@
from openupgradelib import openupgrade

_xmlids_renames = [
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @ngochung207 Thanks for your work !
I just tested it and found out that this rename is not done (since not called in migrate function below AFAICT).

You may want to add a openupgrade.rename_xmlids(env.cr, _xmlids_renames) to enforce it !

Copy link
Contributor

Choose a reason for hiding this comment

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

I know, i have postpone the work from this PR, you may want to try at https://github.com/Viindoo/OpenUpgrade , my work has finished there

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @duong77476 I manage to fully migrate my database using both your repo and PRs from OCA/OpenUpgrade repo. It is however very difficult to track differences since this PR and your repo contain a lot of things which are related to your instances / modules.
Could you please let me know your plans to complete/cleanup this PR ? I can help if needed !
Best Regards

@ahshakir
Copy link

Hi,
I have been using OpenUpgrade to migrate from version 11 to version 16. So far till version 15 everything works fine and I know the migration script for 16 is not completely ready yet but I have to complete this migration so I collected the migration scripts from different repos to use and for account module I used script from here. Now the problem is after migration I tried to view the invoices and the invoice lines are not being displayed for some reason. Kindly advise please to rectify this. Thank you in advance.

@duong77476-viindoo
Copy link
Contributor

Hi, I have been using OpenUpgrade to migrate from version 11 to version 16. So far till version 15 everything works fine and I know the migration script for 16 is not completely ready yet but I have to complete this migration so I collected the migration scripts from different repos to use and for account module I used script from here. Now the problem is after migration I tried to view the invoices and the invoice lines are not being displayed for some reason. Kindly advise please to rectify this. Thank you in advance.

Thanks, you may want to try at https://github.com/Viindoo/OpenUpgrade instead

@ahshakir
Copy link

ahshakir commented Oct 13, 2023

Hi, I have been using OpenUpgrade to migrate from version 11 to version 16. So far till version 15 everything works fine and I know the migration script for 16 is not completely ready yet but I have to complete this migration so I collected the migration scripts from different repos to use and for account module I used script from here. Now the problem is after migration I tried to view the invoices and the invoice lines are not being displayed for some reason. Kindly advise please to rectify this. Thank you in advance.

Thanks, you may want to try at https://github.com/Viindoo/OpenUpgrade instead

I tried them too but still it didn't work as expected. Also did you come across the same error as mine after migration? And do you think is there any other way possibly I could try to fix this issue please. I am in real need of this fix right now.

@duong77476-viindoo
Copy link
Contributor

@ahshakir no unfortunately, do you have any custom module beside odoo module ?

@ahshakir
Copy link

@ahshakir no unfortunately, do you have any custom module beside odoo module ?

Yes I do and I upgraded them too. Though some modules created new fields in account but nothing sort of doing major change in them And all other modules working fine except for account.

@duong77476-viindoo
Copy link
Contributor

@ahshakir OK now go to debug mode, check the Edit form view then see the view architecture of account_move to see anything strange

@ahshakir
Copy link

I did not see anything strange. but what I would like to add is
The table names in account in Odoo 11,
account invoice - Invoices
account invoice line - Invoice Lines
account move - Journal Entry
account move line - Journal Items
and from Odoo 13, Invoices had name changed from account invoice to account move and from there Invoices, Journal Entries everything feeds into same table if I am not wrong.
Now coming to version 16 Invoices and Journal Entries are in account move and in the db also everything is in the place but still unable to view them. Also there is field in move line I saw display_type. does it have anything to do with the invoices?

@ikus060
Copy link

ikus060 commented Oct 13, 2023

@duong77476 I wish to test your migration script, but I'm block by the migration of "payment". Do you happen to have a branch somewhere with both of these changes ?

As recommended, I already tried to upgrade by using https://github.com/Viindoo/OpenUpgrade But it it's failing the same way.

@duong77476-viindoo
Copy link
Contributor

duong77476-viindoo commented Oct 13, 2023

@duong77476 I wish to test your migration script, but I'm block by the migration of "payment". Do you happen to have a branch somewhere with both of these changes ?

As recommended, I already tried to upgrade by using https://github.com/Viindoo/OpenUpgrade But it it's failing the same way.

What is blocking exactly , show me error on your console log if you have any ? i have tried on a production database and it works fine

@ikus060
Copy link

ikus060 commented Oct 13, 2023

Here the relevant part where the migration is failing. But it's not related to "account" migration. It's the "payment" migration that is failing.

odoo.log

Thanks for your help.

@duong77476-viindoo
Copy link
Contributor

I did not see anything strange. but what I would like to add is The table names in account in Odoo 11, account invoice - Invoices account invoice line - Invoice Lines account move - Journal Entry account move line - Journal Items and from Odoo 13, Invoices had name changed from account invoice to account move and from there Invoices, Journal Entries everything feeds into same table if I am not wrong. Now coming to version 16 Invoices and Journal Entries are in account move and in the db also everything is in the place but still unable to view them. Also there is field in move line I saw display_type. does it have anything to do with the invoices?

Sorry, it is very complex when running migration all the way up from 11 to 16. I have ran it from 10 to 14 already, i must tell you that is not an easy thing to do. You will have to make sure each migration jump (11 -> 12, 12->13, ...) work smootly

@duong77476-viindoo
Copy link
Contributor

duong77476-viindoo commented Oct 13, 2023

Here the relevant part where the migration is failing. But it's not related to "account" migration. It's the "payment" migration that is failing.

odoo.log

Thanks for your help.

Please , check your odoo 16 CE addons, why the log said base.module_payment_abs not found, strange

@ikus060
Copy link

ikus060 commented Oct 13, 2023

Please , check your odoo 16 CE addons, why the log said base.module_payment_aps not found, strange

The module payment_aps seams to be a new module in Odoo v16.0. My database doesn't have any entry in table ir_module_module for this new module. Not sure if a script should be responsible to create an entry for it !?

@ahshakir
Copy link

I did not see anything strange. but what I would like to add is The table names in account in Odoo 11, account invoice - Invoices account invoice line - Invoice Lines account move - Journal Entry account move line - Journal Items and from Odoo 13, Invoices had name changed from account invoice to account move and from there Invoices, Journal Entries everything feeds into same table if I am not wrong. Now coming to version 16 Invoices and Journal Entries are in account move and in the db also everything is in the place but still unable to view them. Also there is field in move line I saw display_type. does it have anything to do with the invoices?

Sorry, it is very complex when running migration all the way up from 11 to 16. I have ran it from 10 to 14 already, i must tell you that is not an easy thing to do. You will have to make sure each migration jump (11 -> 12, 12->13, ...) work smootly

I didn't check thoroughly with everything working smoothly while migrating to every version but I do checked if the invoices were visible properly in every migration and they were there. Only in version 16 they weren't visible at all. ALSO compared the columns in version 15 and 16 of table account.move and didn't find anything different between them.

Is there anyone here migrated to 16 and came across something similar to what I found??

@duong77476-viindoo
Copy link
Contributor

Please , check your odoo 16 CE addons, why the log said base.module_payment_aps not found, strange

The module payment_aps seams to be a new module in Odoo v16.0. My database doesn't have any entry in table ir_module_module for this new module. Not sure if a script should be responsible to create an entry for it !?

No, i dont think because of the script, here is the config that i have used to run migration script. I use eclipse as an IDE to run this, of course you will need to make sure both odoo 15 and odoo 16 CE has the latest code
--addons-path=/git/odoo16/addons,/git/Openupgrade16
--load=base,web,openupgrade_framework
--limit-time-real=10000000
--http-port=8016
-d oca1
-u all
--upgrade-path=~/git/Openupgrade16/openupgrade_scripts/scripts

@duong77476-viindoo
Copy link
Contributor

I did not see anything strange. but what I would like to add is The table names in account in Odoo 11, account invoice - Invoices account invoice line - Invoice Lines account move - Journal Entry account move line - Journal Items and from Odoo 13, Invoices had name changed from account invoice to account move and from there Invoices, Journal Entries everything feeds into same table if I am not wrong. Now coming to version 16 Invoices and Journal Entries are in account move and in the db also everything is in the place but still unable to view them. Also there is field in move line I saw display_type. does it have anything to do with the invoices?

Sorry, it is very complex when running migration all the way up from 11 to 16. I have ran it from 10 to 14 already, i must tell you that is not an easy thing to do. You will have to make sure each migration jump (11 -> 12, 12->13, ...) work smootly

I didn't check thoroughly with everything working smoothly while migrating to every version but I do checked if the invoices were visible properly in every migration and they were there. Only in version 16 they weren't visible at all. ALSO compared the columns in version 15 and 16 of table account.move and didn't find anything different between them.

Is there anyone here migrated to 16 and came across something similar to what I found??

Maybe try inspect (f12) to see if it still there. You mean not visible here is the tab Invoice Line is missing right ?

ngochung207 and others added 2 commits October 24, 2023 08:47
account: more analysis on xml
account.accouunt: Account type in account template and account
 - account.move: auto_post
 - account.move.line: Delete sql_contraints, display_type
 - account.move.line: Dynamic analytic_distribution fill yeahhh
 - account.journal: Delete sql_contraints
 - account.payment: Add field amount_company_currency_signed
 - account.bank.statement: internal_index, first_line_index, is_complete
method

-display_type improvement
-remove bank statement fastfill
-add account_reconcile_model_line analytic_distribution generate
-improve analytic_distribution_model generation from analytic default
@pedrobaeza
Copy link
Member

Thanks to the Viindoo pioneers on this difficult task.

I have performed the following changes and push in this PR (as there are permissions to maintainers granted):

  • Don't load non useful noupdate_changes.
  • Precompute account.payment~amount_company_currency_signed for the easy case of same currency.
  • Preserve old account.move~auto_post column.
  • Adapt account.account~account_type filling SQL and add a fallback.
  • Fill properly account.account~include_initial_balance.
  • No need of fast filling account.analytic.line~journal_id.
  • Adapt account_analytic_distribution_model generation SQL.
  • Adapt analytic_distribution SQL filling and make it generic.

I'm merging as soon as this is green, so finally! 🎉

@duong77476-viindoo
Copy link
Contributor

Thanks to the Viindoo pioneers on this difficult task.

I have performed the following changes and push in this PR (as there are permissions to maintainers granted):

  • Don't load non useful noupdate_changes.
  • Precompute account.payment~amount_company_currency_signed for the easy case of same currency.
  • Preserve old account.move~auto_post column.
  • Adapt account.account~account_type filling SQL and add a fallback.
  • Fill properly account.account~include_initial_balance.
  • No need of fast filling account.analytic.line~journal_id.
  • Adapt account_analytic_distribution_model generation SQL.
  • Adapt analytic_distribution SQL filling and make it generic.

I'm merging as soon as this is green, so finally! 🎉

At your service sir 🙉
Thank you for your work too

- Don't load non useful noupdate_changes
- Precompute account.payment~amount_company_currency_signed for the
  easy case of same currency.
- Preserve old account.move~auto_post column.
- Adapt account.account~account_type filling SQL and add a fallback.
- Fill properly account.account~include_initial_balance.
- No need of fast filling account.analytic.line~journal_id.
- Adapt account_analytic_distribution_model generation SQL.
- Adapt analytic_distribution SQL filling and make it generic.

 TT42204
@pedrobaeza pedrobaeza merged commit dcb7cc3 into OCA:16.0 Oct 24, 2023
2 checks passed
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.