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][IMP] update analyst file #4042

Closed
wants to merge 1 commit into from

Conversation

ndd-odoo
Copy link
Contributor

@ndd-odoo ndd-odoo commented Jul 5, 2023

Regenerate analyst because some of them are oudated (eg: analytic)

@ndd-odoo ndd-odoo changed the title [IMP] update analyst file [16.0][IMP] update analyst file Jul 5, 2023
@ndd-odoo
Copy link
Contributor Author

ndd-odoo commented Jul 5, 2023

@legalsylvain I regenerate analyst files here, if you have time please take a look.
One problem i have encountered when running analyst is if you install some of deprecated module like payment_alipay (this module is marked as deprecated in odoo 16) and then you hit button generate record, it will get errors because in that module have this code https://github.com/odoo/odoo/blob/914870f2d4b9dbdf39cdde98fed3bb21ca0074cf/addons/payment_alipay/__init__.py#L12

@@ -9,6 +9,7 @@ hr / hr.contract.type / sequence (integer) : NEW
hr / hr.department / master_department_id (many2one): NEW relation: hr.department, isfunction: function, stored
hr / hr.department / parent_path (char) : NEW
hr / hr.department / plan_ids (one2many) : NEW relation: hr.plan
hr / hr.employee / currency_id (many2one) : previously in module hr_timesheet
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one correct, manual checked

@@ -101,7 +101,7 @@ account / account.chart.template / complete_tax_set (boolean) : DEL
account / account.chart.template / use_storno_accounting (boolean): NEW hasdefault: default
account / account.journal / default_account_type (many2one): relation is now 'False' ('account.account.type') [nothing to do]
account / account.journal / default_account_type (many2one): type is now 'char' ('many2one')
account / account.journal / payment_sequence (boolean) : NEW hasdefault: default
account / account.journal / payment_sequence (boolean) : NEW hasdefault: compute
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one correct too, manual checked

@ndd-odoo
Copy link
Contributor Author

ndd-odoo commented Jul 5, 2023

The red CI i have a PR fix here OCA/openupgradelib#335

---Models in module 'gift_card'---
obsolete model gift.card
obsolete model gift.card (renamed to loyalty.card in module loyalty)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one too XD

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Hi @duong77476 Thanks a lot for taking the time to update the files.

  1. what a huge diff ! What a stable release ;-) !

  2. a blocking point inline. Could you take a look ?

@@ -4,6 +4,9 @@ event_booth_sale / event.booth / price (float) : is
event_booth_sale / event.type.booth / price (float) : is now stored
event_booth_sale / product.template / detailed_type (False) : selection_keys is now '['consu', 'course', 'event', 'event_booth', 'product', 'service']' ('['consu', 'event', 'event_booth', 'gift', 'product', 'service']')
---XML records in module 'event_booth_sale'---
event.booth.category: event_booth.event_booth_category_premium (noupdate) (noupdate switched)
event.booth.category: event_booth.event_booth_category_standard (noupdate) (noupdate switched)
event.booth.category: event_booth.event_booth_category_vip (noupdate) (noupdate switched)
Copy link
Contributor

@legalsylvain legalsylvain Jul 5, 2023

Choose a reason for hiding this comment

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

I see no changes between :
https://github.com/odoo/odoo/blob/15.0/addons/event_booth_sale/data/event_booth_category_data.xml
https://github.com/odoo/odoo/blob/16.0/addons/event_booth_sale/data/event_booth_category_data.xml
(last changes done sept 2021, in both cases)

Are you sure you use head version for version 15 and version 16 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, i don't know why it has that, I use the head version only lack a few commit but i don't thing that is a problem :(

Copy link
Contributor

Choose a reason for hiding this comment

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

Hum. That's annoying. did you used a fresh DB also ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, i did, brand new DB. I just found a commit that they have changed to noupdate in orignal data, see https://github.com/odoo/odoo/commit/67b149fc3a8dffea37de59c9579d552c559b11f7 but it shouldn't be like this right, maybe i'll delete it anyway :(

Copy link
Contributor

@legalsylvain legalsylvain Jul 5, 2023

Choose a reason for hiding this comment

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

delete what ? commit ?

And Something I don't understand : the two links I mentionned said that the last commit was in 2021. But your commit odoo/odoo@67b149f has been done in 2022. totally lost.

Copy link
Contributor

Choose a reason for hiding this comment

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

No i mean delete those 3 lines

OK. Well, I'd like to understand why there 3 false positive are here. Otherwise, we have a risk to add other false positive. we really have to make sure that these analysis files are correct. that's what the developers work on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok i understood, let's rerun again and see what happen, thank for spending your time to have a look .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just make not sure i did not skip any step, i follow step by step in this link https://oca.github.io/OpenUpgrade/analyse.html , what possibly i can miss here :((

Copy link
Member

@pedrobaeza pedrobaeza Oct 22, 2023

Choose a reason for hiding this comment

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

The problem here is that this data is only adding more values to the existing record that was defined in the module event_booth:

https://github.com/odoo/odoo/blob/aa90123b666784aea039ebb3c1febf0be81f8362/addons/event_booth/data/event_booth_category_data.xml#L4C77-L4C77

and in that definition, the record is noupdate.

This is then a problem of the upgrade_analysis module. I'm going to check if I can fix it, although the expert here is @MiquelRForgeFlow

Copy link
Member

@pedrobaeza pedrobaeza Oct 22, 2023

Choose a reason for hiding this comment

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

It seems this query has not been executed in 15.0:

https://github.com/OCA/server-tools/blob/cd2f828fed05a65aae0dc18eef26bc6f2eb31d39/upgrade_analysis/wizards/upgrade_generate_record_wizard.py#L95

I got my session kicked out though when finished (not sure if this is normal or not), and maybe this side effect it's because one of the flushes overwrites these values. @StefanRijnhart @hbrunn you may know what is happening here.

I'm going to execute manually the code and perform the analysis in that condition.

@ndd-odoo ndd-odoo force-pushed the v16_imp_update_analyst_file branch from 50d03eb to bd2b613 Compare July 6, 2023 09:17
@ndd-odoo
Copy link
Contributor Author

ndd-odoo commented Jul 6, 2023

@legalsylvain Done, i think there is something wrong this code in server-tools
https://github.com/OCA/server-tools/blob/182257bf8670965791abb6c69aeb5ad942f43fb3/upgrade_analysis/wizards/upgrade_generate_record_wizard.py#L94

As when i install all Odoo SA modules, then generate record , many xml id are obviously noupdate but they don't (Note this if you just install some modules not all because i can not identify the root cause then it works fine). So to deal with that i have to run the query in https://github.com/OCA/server-tools/blob/182257bf8670965791abb6c69aeb5ad942f43fb3/upgrade_analysis/wizards/upgrade_generate_record_wizard.py#L94 by hand then start analyzing
Please take a look , thanks in advance

@legalsylvain
Copy link
Contributor

Unfortunately, I'm not expert of that module @duong77476. @StefanRijnhart Do you have some time to take a look on that topic ?

@ndd-odoo ndd-odoo force-pushed the v16_imp_update_analyst_file branch from bd2b613 to 8edef2d Compare July 6, 2023 09:38
@ndd-odoo ndd-odoo force-pushed the v16_imp_update_analyst_file branch from 8edef2d to 6217a9f Compare July 7, 2023 07:48
@legalsylvain legalsylvain added this to the 16.0 milestone Jul 7, 2023
@legalsylvain
Copy link
Contributor

Propose to merge this PR to use the latest analysis files.
Any pov, @remytms, @pedrobaeza, @MiquelRForgeFlow ?

@pedrobaeza
Copy link
Member

I let @MiquelRForgeFlow to check, that is the expert on this.

@MiquelRForgeFlow
Copy link
Contributor

MiquelRForgeFlow commented Jul 10, 2023

My issue is: many PRs have already been done, and once this is merged, I don't know if the people who made those PRs will update the analysis work file correspondingly.

@ndd-odoo
Copy link
Contributor Author

My issue is: many PRs have already been done, and once this is merged, I don't know if the people who made those PRs will update the analysis work file correspondingly.

Yes, i do aware of that issue, i just want to work latest update file especially with my recent PR related to analytic and account , in those PR i use this analysis file to ensure the latest change will be handled.

@legalsylvain
Copy link
Contributor

My issue is: many PRs have already been done, and once this is merged, I don't know if the people who made those PRs will update the analysis work file correspondingly.

this issue is valid. however, if we wait, the problem will be bigger.
how to get ahead ?
If we wanted to get things right, we'd have to review all PRs marked as done / nothing to do and which have an analysis file that has changed, to update the work file.
That sounds like a lot of work.

@MiquelRForgeFlow
Copy link
Contributor

how to get ahead ?

I will review and check the changes when I have a bit of free time.

That sounds like a lot of work.

Unfortunately, yes.

@pedrobaeza
Copy link
Member

On #4180 I have solved the problems found here, so superseding this one. Thanks for starting the update.

@pedrobaeza pedrobaeza closed this Oct 22, 2023
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.

4 participants