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.

Sign up for GitHub

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

Jump to bottom

[10.0]No more foreign keys in all tables when using this repoo #200

Closed
flotho opened this issue Aug 17, 2017 · 24 comments
Closed

[10.0]No more foreign keys in all tables when using this repoo #200

flotho opened this issue Aug 17, 2017 · 24 comments
Labels
bug
Milestone
10.0

Comments

@flotho
Copy link
Member

flotho commented Aug 17, 2017

Hi everyone,

This one seems really weird. First I suspected that Odoo CE has removed FK in fresh databases : odoo/odoo#18880.
Yet starting with a fresh install with no specific addons in path make the things going well. Then repo by repo I added the dependencies and try to create new DB.
At last, this repo OCA/pos seeems to cause a creation with no Foreign keys.
I've updated the latest OCA/pos check all the depenecies, requirements etc, try many times to create with or without the dependencies. The result is the same, using OCA/pos cause no FK in the db!
Here are the logs, nothing special for me,any help would be appreciated.
odoo.log.txt
@flotho flotho changed the title [10.0]Stange conflict causing no foreign keys [10.0]No more foreign keys in all tables when using this repoo Aug 19, 2017
@flotho
Copy link
Member Author

flotho commented Aug 19, 2017

Hi,
I made additionnal tests :

  • OCA/pos : 10.0 ddfab70 OCA Transbot updated translations from Transifex
  • odoo/odoo : 10.0 89cade3bffc [FIX] pos_restaurant: wrap product names on OrderChangeReceipt

Same result, all the FK disappearing from tables, Does anyone reproduce this ?

@pedrobaeza
Copy link
Member

Which module from OCA/pos? Check in runbot if it happens the same.

@flotho
Copy link
Member Author

flotho commented Aug 19, 2017

I'm trying each module one by one to identify

@flotho
Copy link
Member Author

flotho commented Aug 19, 2017

Well it looks like that pos_remove_category caused the problem. I'll recheck

@flotho
Copy link
Member Author

flotho commented Aug 19, 2017

Well, I confirm that having the module pos_remove_category cause the problem. I made those tests :

  • Creating db with adding module from oca/pos one by one, retsart odoo between each addition
    ** => everything work fine except when pos_remove_category is present in the addons path
  • Creating db with all the module except pos_remove_category
    ** => Everything Ok
  • creating db with only pos_remove_categ => make FK disappear

i'm suspecting the ir.module.module dependency so that I tried to comment the init part.

There's something I can't log/ identify more without your support. Thanks in advance.
regards

@flotho
Copy link
Member Author

flotho commented Aug 19, 2017

@pedrobaeza it's not really easy for me to try on runbot because no error make the creation failed. Sadly there also nothing special in the logs. The db is created but the table structure haven't got anymore FK. I discovered that when a product.template has been deleted and its associated product.product still living its own ....
not easy to debug this one

@legalsylvain legalsylvain added this to the 10.0 milestone Aug 20, 2017
@legalsylvain
Copy link
Contributor

legalsylvain commented Aug 20, 2017 •
edited
Loading

Thanks @flotho for reporting this problem.

This line seems to create your bug. (not tested)
https://github.com/OCA/pos/blob/10.0/pos_remove_pos_category/models/product.py#L89

Solution 1 : full refactoring
My point of view is that this module is not very clean, and should be refactored :

  • do not alter database.
  • while loading product, remove pos_categ_id field, add the product_categ_id, and at the end of the load, initialize the pos_categ_id with the product_categ_id
  • drop auto_end function and all product overload.
  • product_category.available_in_pos : could be computed. (optional) (if category has products that can be sold in PoS.)

Solution 2 : more easy.

  • disable _auto_end function. (The module will not work anymore correctly), but the repo will be safe.

@simone, @sylvainc. Could you take a look ?

Note : this problem is present on odoo8 too.
https://github.com/OCA/pos/blob/8.0/pos_remove_pos_category/product.py#L105

@flotho
Copy link
Member Author

flotho commented Aug 21, 2017

Hi everyone,
Additionnal question, once this module is not present in the addons_path, do you think that an -u all will re-add all the FK on an already created database?
regards

@guewen
Copy link
Member

guewen commented Aug 21, 2017

Worth to note this module has another big design issue. It replaces the Manyone towards pos.category on the field product_template.pos_categ_id by a Many2one towards product.category. As soon as you will want to upgrade the base module point_of_sale, this one will try to create a Foreign Key towards product.category and fail (letting the database blocked in an intermediate state) because the ids might not exist in this table.

IMO this module should either be reworked in a clean way, either be removed.

@pedrobaeza
Copy link
Member

Yeah, this is very critical. Who was the original author/migrator? Is he/she (or anyone else) going to attend this? If not, we should remove it.

@sylvainc
Copy link

sylvainc commented Aug 21, 2017 via email

legalsylvain added a commit to grap/pos that referenced this issue Aug 21, 2017
@legalsylvain
[REM] remove pos_remove_pos_category Fix : OCA#200
2d4b510
@legalsylvain legalsylvain mentioned this issue Aug 21, 2017
Merged
legalsylvain added a commit to grap/pos that referenced this issue Aug 21, 2017
@legalsylvain
[REM] remove pos_remove_pos_category Fix : OCA#200
88e1081
@legalsylvain legalsylvain mentioned this issue Aug 21, 2017
Merged
legalsylvain added a commit to grap/pos that referenced this issue Aug 21, 2017
@legalsylvain
[REM] remove pos_remove_pos_category Fix : OCA#200
5825f0f
@legalsylvain legalsylvain mentioned this issue Aug 21, 2017
Merged
@legalsylvain
Copy link
Contributor

Done : PR to review :
8.0 : #201
9.0 : #202
10.0 : #203

pedrobaeza added a commit that referenced this issue Aug 21, 2017
@pedrobaeza
Merge pull request #201 from grap/8.0_REM_pos_remove_category
2930351 
[REM][8.0] remove pos_remove_pos_category Fix : #200
pedrobaeza added a commit that referenced this issue Aug 21, 2017
@pedrobaeza
Merge pull request #202 from grap/9.0_REM_pos_remove_category
1cbc216 
[REM][9.0] remove pos_remove_pos_category Fix : #200
pedrobaeza added a commit that referenced this issue Aug 21, 2017
@pedrobaeza
Merge pull request #203 from grap/10.0_REM_pos_remove_category
333a289 
[REM][10.0] remove pos_remove_pos_category Fix : #200
@flotho
Copy link
Member Author

flotho commented Aug 21, 2017

Thanks folks, needless to say that it had badside effects on my actual prod databases. Hopefully, -u all re-added all the FK.
regards

@simahawk
Copy link

@flotho thanks for spotting this! 👍 for removal.

@simahawk simahawk mentioned this issue Aug 22, 2017
Closed
@StefanRijnhart
Copy link
Member

In case you were wondering like I was, this problem did not affect 8.0 or 9.0. The bug was introduced here, during the migration to 10.0, by not returning a call to super but returning only a reference to the super method instead: https://github.com/OCA/pos/pull/158/files#diff-a2bb6a42189587ba52241d71ea03f98eL104

Thank you @flotho for doing all this investigation and pinpointing the problem to this particular module.

@legalsylvain
Copy link
Contributor

Thanks @StefanRijnhart for this explanation.

@simahawk
Copy link

holy 💩 I'm gonna bury myself... 😢
sorry for the mess guys... 🔫

@Cedric-Pigeon
Copy link

@pedrobaeza @sylvainc @flotho @legalsylvain @simahawk Can you restore this addon. The fix is proposed for a long time and works fine... #182

@pedrobaeza
Copy link
Member

Please restore the addon and add the patch in your PR, and if everything is correct, we merge at once.

@Cedric-Pigeon
Copy link

Please revert your merge, it is more simple. I pointed everyone in my PR months ago.

@pedrobaeza
Copy link
Member

Sorry, but we can't allowed a module that breaks whole installation. Restore it in your PR and when merged, everything will work correctly. You can press yourself in revert commit, and GitHub will propose a new PR. Cherry-pick your commit in top of it and it's done.

Cedric-Pigeon pushed a commit to acsone/pos that referenced this issue Aug 23, 2017
Revert "[REM] remove pos_remove_pos_category Fix : OCA#200"
5b5dd74 
This reverts commit 5825f0f.
@Cedric-Pigeon
Copy link

Ok you are right. I renamed my PR description and add the revert of the removal.

@pedrobaeza
Copy link
Member

Thanks for the understanding, @Cedric-Pigeon

legalsylvain added a commit that referenced this issue Aug 23, 2017
@legalsylvain
Revert "[REM][8.0] remove pos_remove_pos_category Fix : #200"
1bf9eba
@legalsylvain legalsylvain mentioned this issue Aug 23, 2017
Merged
legalsylvain added a commit that referenced this issue Aug 23, 2017
@legalsylvain
Revert "[REM][9.0] remove pos_remove_pos_category Fix : #200"
4174f91
@legalsylvain legalsylvain mentioned this issue Aug 23, 2017
Closed
@sylvainc
Copy link

Thanks you all

StefanRijnhart added a commit that referenced this issue Aug 28, 2017
@StefanRijnhart
Merge pull request #205 from OCA/revert-201-8.0_REM_pos_remove_category
09bcf32 
Revert "[REM][8.0] remove pos_remove_pos_category Fix : #200"
@pedrobaeza pedrobaeza mentioned this issue Oct 10, 2017
Closed
21 tasks
hparfr added a commit to akretion/pos that referenced this issue Apr 27, 2018
@hparfr
10.0 mig product template (#7)
deb2bf5 
* [ADD] setup.py

* [UPD] addons table in README.md

* [FIX] travis: Update travis file to new standard
* Add new matrix
* Add transifex support
* Switch coveralls to travis_after_tests_success
* Move pip requirements to requirements.txt

* OCA Transbot updated translations from Transifex

* OCA Transbot updated translations from Transifex

* [UPD] addons table in README.md

* [UPD] addons table in README.md

* Make pos_customer_display odoo9 compatible

* Fix version number of pos_customer_display

* Port pos_customer_display to v10

Remove the "Show total to customer" button, which is not need given the new user interface of the POS: the big "Payment" button has this role now.
Add README.rst

* Display relevant info on LCD when Qty/price/discount is forced via the buttons of the POS frontend

* Remove comment

* [UPD] addons table in README.md

* [ADD] setup.py

* new module pos_margin

* [PORT] 10.0 pos_margin

* [UPD] addons table in README.md

* [ADD] setup.py

* OCA Transbot updated translations from Transifex

* pos_remove_pos_category (OCA#148)

* migrate to v9 and add italian translation

* short headers

* migrate JS

* [fix] be defensive: do not assume we have a product category

* OCA Transbot updated translations from Transifex

* [MIG] pos_remove_pos_category: Migrated to 10.0

* [FIX] static javascript assets

* [UPD] addons table in README.md

* [ADD] setup.py

* OCA Transbot updated translations from Transifex

* [ADD] pos_loyalty: loyalty programs for POS (OCA#185)

* [ADD] pos_loyalty: loyalty programs for POS

* [FIX] Limit to one discount reward per order

* [FIX] Remove unused variables and returns

* [IMP] Refactoring rules loading

* [UPD] addons table in README.md

* [ADD] setup.py

* OCA Transbot updated translations from Transifex

* OCA Transbot updated translations from Transifex

* [REM] remove pos_remove_pos_category Fix : OCA#200

* [UPD] addons table in README.md

* Revert "[REM] remove pos_remove_pos_category Fix : OCA#200"

This reverts commit 5825f0f.

* [FIX] avoid monkey patch and error on foreign key constraint

* [UPD] addons table in README.md

* pos_remove_pos_category: inherit the proper view of product.template

* [fix] compability between pos_customer_display and pos_pricelist (OCA#210)

* [fix] compability between pos_customer_display and pos_pricelist

* [UPD] addons table in README.md

* [ADD] PoS session pay invoice module

* [UPD] addons table in README.md

* [add] pos_default_payment_method

* fixup! [add] pos_default_payment_method

* [UPD] addons table in README.md

* [ADD] setup.py

* [FIX] Fix pos_customer_display with Floors in Restaurant

* Add mobule pos_backend_partner

* Update readme

Fix link to github

* Improve readme and manifest

* Add OCA dependencies for allowing tests on a not merged PR

* Fix readme, fix indent, fix bug when choosing a customer in payment pane.

* Add screenshots

* Fix screenshot path

* Update readme

* Hide button when not opened by pos

* [10.0] Remove hw_customer_display and hw_telium_payment_terminal (OCA#231)

* Remove hw_customer_display and hw_telium_payment_terminal

POSbox image runs Odoo v8, so we should maintain hw_* modules only in the 8.0 branch for the moment.

* Update requirements.txt following removal of hw_* modules

* [UPD] addons table in README.md

* update oca_dependencies

* update readme

* Update __init__

* update manifest

* update __init

* fix copyright

* fix copyright2

* update roadmap

* OCA Transbot updated translations from Transifex

* Add pos_backend_communication module

* Update readme

Fix github link

* Update readme

* Update readme.rst

* Update common.js

Fix typo

* Update readme

* [UPD] addons table in README.md

* [ADD] setup.py

* [ADD] module pos_price_to_weight (OCA#171)

* [ADD] module pos_price_to_weight

* [FIX] jslint / pylint / flake8 issues

* OCA Transbot updated translations from Transifex

* [MIG] pos_price_to_weight: Migration to 10.0

* [FIX] in v10, we can add value to selection fieldw

* [UPD] addons table in README.md

* [ADD] setup.py

* [FIX] website URI

* [FIX] website URI

* remove duplicated act_windows

* [ADD] new module pos_quick_logout (OCA#116)

* OCA Transbot updated translations from Transifex

* OCA Transbot updated translations from Transifex

* [MIG] pos_quick_logout: Migration to 10.0

[FIX] Fix Travis

[FIX] Fix Licence

[FIX] rename pos_quick_logout.py to pos_config.py

[FIX] Travis

[FIX] fix javascript syntax + __manifest__ typo

[FIX] fix python syntax

[FIX] rename pos_quick_logout_view.xml to pos_config_view.xml

[FIX] Travis : remove unused imports

* [IMP] add autologout to pos_quick_logout

[IMP] it's better to launch timeout fonction with start

* [UPD] addons table in README.md

* [ADD] setup.py

* [FIX] remove outdated dependencies

* OCA Transbot updated translations from Transifex

* [ADD] new module pos_timeout

* [UPD] addons table in README.md

* [ADD] setup.py

* [FIX] fixes currencies with decimals <>2

* [MIG] pos_pricelist: Migrate to v10 by backporting from v11 (OCA#247)

This module included previously support for taxes and pricelists.

Taxes are now included in upstream Odoo v10, so that feature is removed.

The pricelist feature, however, is supported in upstream Odoo v11. Thus, to ease the migration path, that functionality has been scalpel-backported from there.

This also means a great diff between previous OCA-only code and current Odoo-backport-only code, but with the advantage that the end user gets exactly the behavior that he would have when updating to v11.

Since most of the code has been copied from Odoo, the addon now has to become LGPL, and Odoo has been added as an author.

* [FIX] pos_pricelist: Fix language file

* [UPD] addons table in README.md

* [ADD] setup.py

* [FIX] pos_pricelist: Use lower-version date comparison

The removed function was introduced in momentjs in version 2.11.0, but Odoo 10.0 ships 2.8.1, so I'm using a different comparison system here that should yield the same results without exceptions.

* [UPD] addons table in README.md

* OCA Transbot updated translations from Transifex

* [10.0][ADD] pos_lot_selection: New module (OCA#256)

* [UPD] addons table in README.md

* [ADD] setup.py

* [ADD] pos_stock_picking_invoice_link: New Module

* Add module pos_default_empty_image

Big perf improvement for loading POS if you have thousand of products
without image

* Limit lines to < 80 chars

* No new line at end of file

* Add missing credits

* description file;

* Fix line length in readme

Remove __openerp.py_.description
Remove _name
Remove unneeded contents in init

* [PORT] pos_default_empty_image courtesy @invitu

* [IMP] pos_default_empty_image courtesy @invitu

* [FIX] set has_image on 'product.product' model
[IMP] make has_image stored
[REF] improve description and add screenshots
[REF] OCA convention

* [FIX] wording + remove useless JS code

* [REF] cosmetic points

* Fix readme remarks

* [UPD] addons table in README.md

* [ADD] setup.py

* [UPD] addons table in README.md

* [ADD] setup.py

* Updated pos_product_template addon to Odoo 9

* Added Spanish translation to pos_product_template (es.po)

* [FIX] Change the version number to 9 and make module installable.

* Add readme, update manifest,

switch to short copyright headers

* Some refactorings

* Refactore db

* Refactore set_product_list

* Remove comments

* Add pos_fix_search_limit module

* add pos_fix_search_limit in manifest

* [UPD] addons table in README.md

* [ADD] setup.py

* Make the ppt inheritable
sebastienbeau pushed a commit to akretion/pos that referenced this issue Oct 12, 2020
@legalsylvain @sebastienbeau
[REM] remove pos_remove_pos_category Fix : OCA#200
9fa968b
sebastienbeau pushed a commit to akretion/pos that referenced this issue Oct 12, 2020
@sebastienbeau
Revert "[REM] remove pos_remove_pos_category Fix : OCA#200"
c9fce7f 
This reverts commit 5825f0f.
sebastienbeau pushed a commit to akretion/pos that referenced this issue Dec 18, 2020
@legalsylvain @sebastienbeau
[REM] remove pos_remove_pos_category Fix : OCA#200
bebfd80
sebastienbeau pushed a commit to akretion/pos that referenced this issue Dec 18, 2020
@sebastienbeau
Revert "[REM] remove pos_remove_pos_category Fix : OCA#200"
fa0f681 
This reverts commit 5825f0f.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
10.0
Development

No branches or pull requests
8 participants
@simahawk @guewen @StefanRijnhart @flotho @sylvainc @legalsylvain @pedrobaeza @Cedric-Pigeon