[10.0][FIX] fix issue when creating binding from the main record #607

sebastienbeau · 2020-04-03T20:39:47Z

Indeed the implementation of write on one2many (see odoo/fields.py:2235)
call the method create/write in mode norecompute
This mean that the field is_urls_sync_required is not computed after
the call to super of write/create of the binding but later during the
create/write of the main record.

To solve the issue we sync the url after calling the method
"_recompute_done".
This also simplify the code \o/

sebastienbeau · 2020-04-03T20:42:50Z

@lmignon @rousseldenis

sebastienbeau · 2020-04-03T20:43:27Z

@bguillot

codecov-io · 2020-04-04T07:52:01Z

Codecov Report

Merging #607 into 10.0 will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             10.0     #607      +/-   ##
==========================================
+ Coverage   90.39%   90.45%   +0.06%     
==========================================
  Files         151      151              
  Lines        4048     4054       +6     
==========================================
+ Hits         3659     3667       +8     
+ Misses        389      387       -2

Impacted Files	Coverage Δ
base_url/models/abstract_url.py	`92.30% <100.00%> (-0.36%)`	⬇️
shopinvader/services/abstract_sale.py	`100.00% <0.00%> (ø)`
...nvader/wizards/shopinvader_partner_binding_line.py	`100.00% <0.00%> (ø)`
...ader_delivery_carrier/services/delivery_carrier.py	`100.00% <0.00%> (ø)`
shopinvader_delivery_carrier/services/cart.py	`92.30% <0.00%> (+0.64%)`	⬆️
shopinvader/models/shopinvader_category.py	`97.18% <0.00%> (+1.40%)`	⬆️
...invader_delivery_carrier/services/abstract_sale.py	`96.87% <0.00%> (+3.12%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b81b921...9e0b939. Read the comment docs.

Indeed the implementation of write on one2many (see odoo/fields.py:2235) call the method create/write in mode norecompute This mean that the field is_urls_sync_required is not computed after the call to super of write/create of the binding but later during the create/write of the main record. To solve the issue we sync the url after calling the method "_recompute_done". This also simplify the code \o/

sebastienbeau · 2020-04-05T12:23:10Z

After several hours of fight again several issue when loading fake class I try to implement a new version. Indeed fake model that inherit existing class was not loading the field correctly (if you active the option custom='manual', field are not loaded correctly, if you do not active it field still exist in the registry). Try an alternative approche. For now to not merge yet. Play with the same code here : OCA/search-engine#46

simahawk · 2020-04-06T07:35:03Z

which issue did you have?
If the issue is related to adding fields to existing records, you could probably solve it with this https://github.com/OCA/search-engine/blob/12.0/connector_search_engine/tests/models_mixin.py#L35

sebastienbeau · 2020-04-07T05:17:45Z

@simahawk I have seen this hack
So I was adding the field "binding_ids" as a "custom='manual' and also adding in _test_purge_fields.
But custom field are not loaded correctly and so I had issue when using the field "partner.binding_ids" didn't work correctly.

I try to remove custom option but I was not able to purge the registry correctly.
so I start to try to understand what was happening.

As the file "model" is loaded at python execution the odoo class are already processed.
This mean that the variable "MetaModel.module_to_models" already "keep in mind" that there is odoo class to use.

When we call the _build_model it will register for ever the ResPartnerFake into the registry 'by modifying the base property of the registry['res.partner'] (you can see it this by looking in registry['res.partner'].mro()).
So this mean that every python code put in the Fake model that inherit existing class need to be purge also...
As it's start to be hard to purge everything correctly I try an alternative way and try to restore the initial base of the class inherited
I still have some issue with search-engine case, but I am not very far.

sebastienbeau · 2020-04-07T05:18:48Z

I will work on it at this end of the week and try to publish the code in a python lib compatible for odoo 10/11/12/13 so we can reuse it everywhere without copy/paste

hparfr

I tested this fix in v12.
The binding issue was not related to this fix. But since it looks to be shorter and cleaner I approve it :-)

Btw the problem (at least in v12) was in automatic_url_key calc definition. I will provide a patch soon. After my fix, and this one, it works well.

[EDIT]: I've tested with my fix and without this one, and it's true this fix resolve another issue. So both fixes are needed.

sebastienbeau · 2020-04-17T20:10:48Z

@simahawk it's done now we have a generic helper for loading fake model https://github.com/akretion/odoo-test-helper working on version 10, 11, 12, 13 !

simahawk · 2020-04-19T08:44:24Z

@simahawk it's done now we have a generic helper for loading fake model https://github.com/akretion/odoo-test-helper working on version 10, 11, 12, 13 !

great! Some things missing (eg: xmlid generation, ACL generation, etc) but it's good start!
Can we propose it to OCA?

shopinvader-git-bot · 2020-04-19T09:00:01Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

simahawk · 2020-04-19T10:53:36Z

@sebastienbeau I'm testing the test helper somewhere else. I found some issue -> I'll open a PR.

simahawk · 2020-04-19T11:45:28Z

@sebastienbeau OCA/odoo-test-helper#1

lmignon · 2020-04-20T07:19:15Z

@sebastienbeau I'm a little overwhelmed at the moment but it would be nice in your library to take credit for this work on Simone and me too. I've spent a lot of time on it and as far as I know I'm the original author of this code.
It's a great idea to make it an external lib compatible between versions but it shouldn't hide the work done by others.

Friendly,

lmi

simahawk · 2020-04-20T07:43:11Z

@sebastienbeau I'm a little overwhelmed at the moment but it would be nice in your library to take credit for this work on Simone and me too. I've spent a lot of time on it and as far as I know I'm the original author of this code.
It's a great idea to make it an external lib compatible between versions but it shouldn't hide the work done by others.

Friendly,

lmi

👍 I planned to open a PR for this 😉
Also, it could be move to this org maybe, before pushing for OCA adoption.

sebastienbeau · 2020-04-20T11:47:30Z

@lmignon I am going to fix it,
@simahawk I am ok for moving it to OCA or Shopinvader, what do yo prefer?
@simahawk I am going to test your PR right now

lmignon · 2020-04-20T12:33:55Z

@lmignon I am going to fix it,

Thank you

@simahawk I am ok for moving it to OCA or Shopinvader, what do yo prefer?

OCA is IMO the right choice.

sebastienbeau · 2020-04-20T13:25:20Z

I also think OCA will be the best place.
Regarding credit : OCA/odoo-test-helper#5 tell me if it's ok

simahawk · 2020-04-20T13:49:51Z

OCA is fine, that's the final plan. I only said "before pushing for OCA adoption" ;)
Thanks for the change! I'l check later

sebastienbeau · 2020-04-24T19:52:44Z

/ocabot merge minor

shopinvader-git-bot · 2020-04-24T19:53:15Z

On my way to merge this fine PR!
Prepared branch 10.0-ocabot-merge-pr-607-by-sebastienbeau-bump-minor, awaiting test results.

shopinvader-git-bot · 2020-04-24T20:31:17Z

Congratulations, your PR was merged at d9accf6. Thanks a lot for contributing to shopinvader. ❤️

sebastienbeau mentioned this pull request Apr 3, 2020

[10.0] [FIX] Fix bug when creating a chopinvader categoy from category #419

Closed

sebastienbeau changed the title ~~[FIX] fix issue when creating binding from the main record~~ [10.0][FIX] fix issue when creating binding from the main record Apr 3, 2020

sebastienbeau added 10.0 bug needs review labels Apr 3, 2020

sebastienbeau force-pushed the 10.0-fix-sync-url branch 2 times, most recently from 5cb868b to 40aada4 Compare April 4, 2020 07:51

sebastienbeau force-pushed the 10.0-fix-sync-url branch from 40aada4 to 9e342c2 Compare April 4, 2020 09:06

sebastienbeau force-pushed the 10.0-fix-sync-url branch 2 times, most recently from 2e65a6a to 9264bb2 Compare April 4, 2020 23:36

sebastienbeau mentioned this pull request Apr 5, 2020

[10.0] Refactor test by using odoo-test-lib OCA/search-engine#46

Closed

hparfr approved these changes Apr 17, 2020

View reviewed changes

hparfr mentioned this pull request Apr 17, 2020

[12.0][FIX] shopinvader - Fix issue when bind from the product page #613

Closed

sebastienbeau force-pushed the 10.0-fix-sync-url branch 2 times, most recently from 8eb628f to 78afce8 Compare April 17, 2020 19:06

simahawk approved these changes Apr 19, 2020

View reviewed changes

shopinvader-git-bot added approved ready to merge labels Apr 19, 2020

sebastienbeau force-pushed the 10.0-fix-sync-url branch from 78afce8 to f387422 Compare April 24, 2020 17:10

[FIX] fix test and use use odoo-test-helper lib \o/

9e0b939

sebastienbeau force-pushed the 10.0-fix-sync-url branch from f387422 to 9e0b939 Compare April 24, 2020 19:08

shopinvader-git-bot merged commit 6aad99a into shopinvader:10.0 Apr 24, 2020

shopinvader-git-bot added the merged 🎉 label Apr 24, 2020

lmignon mentioned this pull request Apr 25, 2020

Forward-port / Back-port TODO #416

Closed

77 tasks

This was referenced May 26, 2020

[13.0][PORT] #607 port from 10.0: [FIX] fix issue when creating binding from the main record #668

Merged

[12.0][PORT] #607 and #340 port from 10.0: [FIX] fix issue when creating binding from the main record #669

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10.0][FIX] fix issue when creating binding from the main record #607

[10.0][FIX] fix issue when creating binding from the main record #607

sebastienbeau commented Apr 3, 2020

sebastienbeau commented Apr 3, 2020

sebastienbeau commented Apr 3, 2020

codecov-io commented Apr 4, 2020 •

edited

Loading

sebastienbeau commented Apr 5, 2020

simahawk commented Apr 6, 2020 •

edited

Loading

sebastienbeau commented Apr 7, 2020

sebastienbeau commented Apr 7, 2020

hparfr left a comment •

edited

Loading

sebastienbeau commented Apr 17, 2020

simahawk commented Apr 19, 2020 •

edited

Loading

shopinvader-git-bot commented Apr 19, 2020

simahawk commented Apr 19, 2020

simahawk commented Apr 19, 2020

lmignon commented Apr 20, 2020

simahawk commented Apr 20, 2020

sebastienbeau commented Apr 20, 2020

lmignon commented Apr 20, 2020

sebastienbeau commented Apr 20, 2020

simahawk commented Apr 20, 2020

sebastienbeau commented Apr 24, 2020

shopinvader-git-bot commented Apr 24, 2020

shopinvader-git-bot commented Apr 24, 2020

[10.0][FIX] fix issue when creating binding from the main record #607

[10.0][FIX] fix issue when creating binding from the main record #607

Conversation

sebastienbeau commented Apr 3, 2020

sebastienbeau commented Apr 3, 2020

sebastienbeau commented Apr 3, 2020

codecov-io commented Apr 4, 2020 • edited Loading

Codecov Report

sebastienbeau commented Apr 5, 2020

simahawk commented Apr 6, 2020 • edited Loading

sebastienbeau commented Apr 7, 2020

sebastienbeau commented Apr 7, 2020

hparfr left a comment • edited Loading

Choose a reason for hiding this comment

sebastienbeau commented Apr 17, 2020

simahawk commented Apr 19, 2020 • edited Loading

shopinvader-git-bot commented Apr 19, 2020

simahawk commented Apr 19, 2020

simahawk commented Apr 19, 2020

lmignon commented Apr 20, 2020

simahawk commented Apr 20, 2020

sebastienbeau commented Apr 20, 2020

lmignon commented Apr 20, 2020

sebastienbeau commented Apr 20, 2020

simahawk commented Apr 20, 2020

sebastienbeau commented Apr 24, 2020

shopinvader-git-bot commented Apr 24, 2020

shopinvader-git-bot commented Apr 24, 2020

codecov-io commented Apr 4, 2020 •

edited

Loading

simahawk commented Apr 6, 2020 •

edited

Loading

hparfr left a comment •

edited

Loading

simahawk commented Apr 19, 2020 •

edited

Loading