8.0 greenify product attribute

[gdgellatly]

Updates auto_init to set a default code in case one does not exist. Otherwise it causes failures in setting the unique index.

Addresses #77 and replaces #78

[gdgellatly]

@LeartS @clonedagain @pedrobaeza @lmignon @gurneyalex

This greenifys product attribute and resolves the issue with duplicate key constraints. If we can get it merged then we can work out what we want to do surrounding the PR's below, hopefully by just moving, reusing the code that makes this green.

The install is a nightmare, the demo data doesn't update until after the module is installed, and the write method is called when setting defaults at module install but because the sequence is not loaded, so even if it was called, it just returns '/' anyway as it isn't found. Calling write in auto_init still causes a duplicate violation so I had to use cr.execute and set a manual sequence. However the additional demo data is updated afterwards.

I know I could have used startswith() but it is slow and products are written to a lot, so I chose slice syntax deliberately for the code check.

While this is primarily a work around to greenify this branch, it is also useful in production.

#34
#33
#23

[guewen]

👍

[lmignon]

you should use an install hooks odoo/odoo@4105b5f

[pedrobaeza]

Thanks for solving the problem 👍

[lmignon]

you can fix all the duplicates in 1 SQL query.

WITH product_duplicate_code AS (
SELECT id
      ,CASE WHEN (count(*) OVER (PARTITION BY default_code)) > 1 THEN
                  'mig-' 
                  || row_number() OVER (PARTITION BY default_code ORDER BY id)
                  || '-'
                  || COALESCE(default_code, '') 
       ELSE default_code END AS new_default_code,
       row_number() OVER (PARTITION BY default_code ORDER BY id) as cpt
FROM   product_product)
UPDATE
    product_product AS pp
SET 
    default_code = new_default_code
FROM 
    product_duplicate_code AS pdc
WHERE
    pp.id = pdc.id
    AND pdc.cpt > 1

[lmignon]

@gdgellatly Thank you for the fix! IMO it can be improved by using a pre-init-hook in place of overriding the auto_init method.

• Explanation odoo/odoo@4105b5f
• Sample usage
  • https://github.com/OCA/geospatial/blob/8.0/base_geoengine/__openerp__.py#L48
  • https://github.com/OCA/geospatial/blob/8.0/base_geoengine/__init__.py#L35
  • https://github.com/OCA/geospatial/blob/8.0/base_geoengine/geo_db.py

You can also find and fix duplicates entries in 1 query. (tested on a db with 10k products and run in 52 ms)

[gdgellatly]

@lmignon I have update to use a pre_init_hook as suggested, however given the purpose of this merge the RTE is over the top IMO and changes module behaviour too much and raises the skill level too much for maintainability, not everyone is familiar with RTE's so have just reduced to a single simple query. The main purpose is to greenify for demo data, and I'd rather the install failed than started overwriting already set codes.

[lmignon]

@gdgellatly Thank you for the change. I understand you POV. The main objective of my SQL request is to fix all duplicated entries when the module is installed on an existing instance so the SQL constrain can be applied. IMO it's very important to take care of this kind of situation since the installation will not fail if the constrain is not applied and you'll need to check your log file to detect this kind of Warning.

[gdgellatly]

@lmignon I understand but it is not the point of this pull request which is
to make the whole repo green. I am uncomfortable adding a new feature to a
module that I disagree with, particularly when I have no interest in using
the module. But feel free to make your own pull request.

The Rte destroys data, you feel that is right as successful installation
of the module is more important. I feel the opposite and prefer
documentation and install failure over possible data destruction. It is
merely a perspective.

But regardless, neither is the point here. The failed builds are stopping
functional reviews as we can't access the requests on runbot. I doubt this
module is very popular and I really doubt anyone is retro fitting it.

On Fri, 19 Jun 2015 7:32 PM Laurent Mignon (ACSONE) <
notifications@github.com> wrote:

@gdgellatly https://github.com/gdgellatly Thank you for the change. I
understand you POV. The main objective of my SQL request is to fix all
duplicated entries when the module is installed on an existing instance so
the SQL constrain can be applied. IMO it's very important to take care of
this kind of situation since the installation will not fail if the
constrain is not applied and you'll need to check you log file to detect
this kind of Warning.

—
Reply to this email directly or view it on GitHub
#80 (comment).

[gdgellatly]

just rebased and cleaned up

[yvaucher]

👍