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] stock: migration #3966

Closed

Conversation

hoangtiendung070797
Copy link
Contributor

migration to 16.0

@hoangtiendung070797 hoangtiendung070797 force-pushed the v16_mig_stock branch 2 times, most recently from b90a7b5 to 65d94f3 Compare July 1, 2023 02:14
@legalsylvain
Copy link
Contributor

legalsylvain commented Jul 1, 2023

/ocabot migration stock

Depends on :

  • barcodes_gs1_nomenclature : TODO

Copy link
Member

@huguesdk huguesdk left a comment

Choose a reason for hiding this comment

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

just a fix for an error that happened when i ran the upgrade.

@hoangtiendung070797 hoangtiendung070797 force-pushed the v16_mig_stock branch 2 times, most recently from 1744fcb to fda9382 Compare July 10, 2023 01:46
@jaredkipe
Copy link

I don't know about the dependency, but I've run a very real database through this and there are a few problems.

stock/16.0.1.1/pre-migration.py

  1. I believe we should have either _columns_renames or _fields_renames, both results in bad query: ALTER TABLE "stock_move_line" RENAME "product_qty" TO "reserved_qty" and psycopg2.errors.UndefinedColumn: column "product_qty" does not exist

I just commented out the inside of _columns_renames to move forward.

  1. I get an error coming from the only line/part of the script that mentions sl.warehouse_id as psycopg2.errors.UndefinedColumn: column sl.warehouse_id does not exist

Given where I'm at in my migration, I just returned out of that function early and moved on.

Anyway, just my two cents, THANKS FOR THE MIGRATION!!!

@cheese202
Copy link

Hi!
Any update with sl.warehouse_id errors and other errors above?

@gdgellatly
Copy link

I don't know about the dependency, but I've run a very real database through this and there are a few problems.

stock/16.0.1.1/pre-migration.py

  1. I believe we should have either _columns_renames or _fields_renames, both results in bad query: ALTER TABLE "stock_move_line" RENAME "product_qty" TO "reserved_qty" and psycopg2.errors.UndefinedColumn: column "product_qty" does not exist

I just commented out the inside of _columns_renames to move forward.

  1. I get an error coming from the only line/part of the script that mentions sl.warehouse_id as psycopg2.errors.UndefinedColumn: column sl.warehouse_id does not exist

Given where I'm at in my migration, I just returned out of that function early and moved on.

Anyway, just my two cents, THANKS FOR THE MIGRATION!!!

Can confirm both issues

  1. No need for column rename after field rename is there.
  2. stock location has no field warehouse_id.

openupgrade.rename_tables(env.cr, _tables_renames)
openupgrade.rename_models(env.cr, _models_renames)
openupgrade.rename_fields(env, _fields_renames)
openupgrade.rename_columns(env.cr, _columns_renames)

Choose a reason for hiding this comment

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

This is already covered by the rename_fields as seen by the error of the column already existing

Choose a reason for hiding this comment

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

confirmed, reserved_qty is renamed by openupgrade.rename_fields(env, _fields_renames) and after by openupgrade.rename_columns(env.cr, _columns_renames)

END as replenish_location_status
FROM stock_location sl
LEFT JOIN stock_warehouse sw
ON sw.id = sl.warehouse_id

Choose a reason for hiding this comment

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

The warehouse is a computed value that is new being stored, I tried adding the field using openupgrade.add_fields and calculating the value in this script, however got the following warning

add_fields: There's already an entry for warehouse_id in stock.location. This may mean that there's some misconfiguration, or simply that another module added the same field previously.

Is there a reason this cannot be moved the the post migration? allowing the module to calculate the values for us?

@dansanti
Copy link

I wrote a code to retrieve and update the warehouse_id based on the computation performed in Odoo 15.0. Needs reviews an work around :


_add_fields = [  
  ('warehouse_id', 'stock.location', 'stock_location', 'many2one', False, 'stock')
]

def _compute_stock_location_warehouse(env):
    openupgrade.logged_query(
        env.cr,
        """
        WITH subquery AS (
          SELECT
            UNNEST(STRING_TO_ARRAY(
              COALESCE(NULLIF(TRIM(BOTH '/' FROM parent_path), ''), '0'), '/'
            )::INTEGER[]) AS result
          FROM stock_location
        )

        UPDATE stock_location sl
        SET warehouse_id = (
          SELECT sw.id
          FROM stock_warehouse sw
          WHERE sw.view_location_id IN (SELECT result FROM subquery)
          limit 1
        )
    """)

@openupgrade.migrate()
def migrate(env, version):
    openupgrade.add_fields(env, _add_fields)
    openupgrade.rename_tables(env.cr, _tables_renames)
    openupgrade.rename_models(env.cr, _models_renames)
    openupgrade.rename_fields(env, _fields_renames)
    openupgrade.rename_columns(env.cr, _columns_renames)
    _update_stock_quant_storage_category_id(env)
    _update_stock_quant_package_pack_date(env)
    _update_sol_product_category_name(env)
    _compute_stock_location_warehouse(env)
    _compute_stock_location_replenish_location(env)

Copy link

@percevaq percevaq left a comment

Choose a reason for hiding this comment

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

tested in a real migration from 14 to 15 and 16.

Comment on lines +124 to +127
openupgrade.rename_tables(env.cr, _tables_renames)
openupgrade.rename_models(env.cr, _models_renames)
openupgrade.rename_fields(env, _fields_renames)
openupgrade.rename_columns(env.cr, _columns_renames)
Copy link

Choose a reason for hiding this comment

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

the order of execution of methods is not correct and gives errors

Suggested change
openupgrade.rename_tables(env.cr, _tables_renames)
openupgrade.rename_models(env.cr, _models_renames)
openupgrade.rename_fields(env, _fields_renames)
openupgrade.rename_columns(env.cr, _columns_renames)
openupgrade.rename_columns(env.cr, _columns_renames)
openupgrade.rename_fields(env, _fields_renames)
openupgrade.rename_models(env.cr, _models_renames)
openupgrade.rename_tables(env.cr, _tables_renames)

Comment on lines +90 to +91
WHEN sl.usage = 'internal' AND sl.id = sw.lot_stock_id THEN True
ELSE FALSE
Copy link

Choose a reason for hiding this comment

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

Suggested change
WHEN sl.usage = 'internal' AND sl.id = sw.lot_stock_id THEN True
ELSE FALSE
WHEN sl.usage = 'internal' AND sl.id = sw.lot_stock_id THEN TRUE
ELSE FALSE

END as replenish_location_status
FROM stock_location sl
LEFT JOIN stock_warehouse sw
ON sw.id = sl.warehouse_id
Copy link

Choose a reason for hiding this comment

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

This left join does not work because the field does not exist and if it does exist it will be empty.

you have to change it to

Suggested change
ON sw.id = sl.warehouse_id
ON sw.lot_stock_id = sl.id

Choose a reason for hiding this comment

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

Can confirm this fix

@MeeleCode
Copy link

Hello! We are migrating from v13 to v16. Can you share the status of migration scripts v15 to v16? Can we help in any way to finish it?

@marielejeune
Copy link
Contributor

Hello, do you plan to work on the previous comments or does someone plan to work on it? I also need this script for a migration. Thanks!

@percevaq
Copy link

I think the comment thread has not been checked. Hopefully, someone in charge of OCA will review these cases or tell us how to proceed.

@percevaq
Copy link

The problem is that I don't know how to proceed to upload those changes to another user's proposal.

If there is OCA documentation for this I have not found it.

There have been suggestions for changes that can be applied but I don't know if this is the right way to go about it.

Can someone from OCA point us in the right direction?

@crazybolillo
Copy link

diff --git a/openupgrade_scripts/scripts/stock/16.0.1.1/pre-migration.py b/openupgrade_scripts/scripts/stock/16.0.1.1/pre-migration.py
index f287c09..ff023b5 100644
--- a/openupgrade_scripts/scripts/stock/16.0.1.1/pre-migration.py
+++ b/openupgrade_scripts/scripts/stock/16.0.1.1/pre-migration.py
@@ -92,7 +92,7 @@ def _compute_stock_location_replenish_location(env):
             END as replenish_location_status
             FROM stock_location sl
             LEFT JOIN stock_warehouse sw
-            ON sw.id = sl.warehouse_id
+            ON sw.lot_stock_id = sl.id
         )
         UPDATE stock_location sl
         SET replenish_location = info.replenish_location_status
@@ -121,10 +121,10 @@ def _update_stock_quant_package_pack_date(env):
 
 @openupgrade.migrate()
 def migrate(env, version):
-    openupgrade.rename_tables(env.cr, _tables_renames)
-    openupgrade.rename_models(env.cr, _models_renames)
-    openupgrade.rename_fields(env, _fields_renames)
     openupgrade.rename_columns(env.cr, _columns_renames)
+    openupgrade.rename_fields(env, _fields_renames)
+    openupgrade.rename_models(env.cr, _models_renames)
+    openupgrade.rename_tables(env.cr, _tables_renames)
     _update_stock_quant_storage_category_id(env)
     _update_stock_quant_package_pack_date(env)
     _update_sol_product_category_name(env)

Those set of changes (which were commented on this thread) apparently fixed migration errors for me.

@marielejeune
Copy link
Contributor

The problem is that I don't know how to proceed to upload those changes to another user's proposal.

If there is OCA documentation for this I have not found it.

There have been suggestions for changes that can be applied but I don't know if this is the right way to go about it.

Can someone from OCA point us in the right direction?

Hi,

I think the only way to do that is to open a new PR. You can cherry-pick this commit or set @hoangtiendung070797 as co-author of your commit.

@tsipizic
Copy link

tsipizic commented Jan 15, 2024

I can confirm this PR with @crazybolillo 's patch works

@percevaq
Copy link

Thank you for appreciating my review and follow-up work.

More of the same.

@bendem
Copy link

bendem commented Jan 24, 2024

Is there any plans to merge this, @percevaq can you open a PR with these changes cherry-picked and your fixes on top?

@zamberjo
Copy link
Member

@hoangtiendung070797 can you apply the patch that @crazybolillo indicates to see if the error is solved and we can give a boost to this?

@marielejeune
Copy link
Contributor

Hi everybody,
Since nothing evolves here, I've decided to create a new PR where I cherry-picked the commit from here, and fixed both issues you all mentioned.
Feel free to review here: #4291

@andreampiovesana
Copy link

@pedrobaeza we close this PR for : #4291 ?

@pedrobaeza pedrobaeza closed this Feb 22, 2024
Copy link

@onurugur onurugur left a comment

Choose a reason for hiding this comment

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

lgtm

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.