shopinvader / odoo-shopinvader

Odoo Modules. Sorry Magento, Shopinvader is coming
GNU Affero General Public License v3.0
121 stars 103 forks source link

v14 - v16 splitting shopinvader module ? #1372

Open sebastienbeau opened 1 year ago

sebastienbeau commented 1 year ago

In V16 a lot of change have been started (new api based on fastapi, huge refactor on search engine....). A lot of change are breaking change, but as customer can change their odoo version without migrating the shop at the same time. The old API must be installable on V16.

In order to make it possible here is the plan :

Regarding the API

Regarding the product sync with search engine On V16 the concept of binding is removed (no more shopinvader.product/shopinvader.variant) On V14 we must split the shopinvader module in several module:

The migration script will automatically install the module (shopinvader_product and shopinvader_partner) when upgrading shopinvader module

Dependency of shopinvader addons that need the "shopinvader.product/variant/partner" will depend on the related modules

By doing this we do two thing:

@hparfr @lmignon @simahawk @sbidoul it's ok for you ?

sbidoul commented 1 year ago

I agree. We should preserve the REST APIs contracts as much as we can.

I understand some are impossible to preserve when moving to a SPA frontend model because they depend on the locomotive session (e.g. cart).

For some we want to create a v2 because we found genuine design issues (e.g. addresses). Or provide more features and better ergonomy (e.g. async cart).

But in general we must preserve compatibility for frontends, yes.

Also note it is entirely possible to migrate to FastAPI while preserving frontend compatibility. That should be doable gradually.

What is unclear to me is to what extent we can preserve the backend apis.

hparfr commented 1 year ago

My goal is to have lot of new v16 shiny stuff in v14 as well.

If possible, I want fast api in v14.

New dependency will be added to shopinvader module. Concurrent modules with v16 compat will be created in v14. It will allow us to migrate progressively our code base.

There should be no breaking change in v14 , actual api will continue to work in v14.

lmignon commented 1 year ago

@sebastienbeau For sure a split would be welcome but the cost could be high. The creation of shopinvader_partner and could ease the cleanup of place where we explicitly use the shopinvader.partner model and replace-it by the res.partnermodel. I've the feeling that shopinvader_product will be more difficult to extract. The most important point is to keep the API for existing projects. The difficulty is that the lack of documentation of the existing API (no schema for most of the response, no schema for the product description, ...) could lead to unwanted breakage.

sebastienbeau commented 1 year ago

After thinking twice extracting "shopinvader.partner" seem to be complicated on the old V1 API, and also maybe not a good investment as if you use the V1 is mostly because you have a shop on locomitiveCMS and in that case the shopinvader.partner is required to the module will be installed

I am hesitating in three direction

Solution 1

Having the following modules

with a script to install this module when shopinvader is updated

Solution 2

Having the following modules

Solution 3 Wait for V16

No split is done on V14 as it can introduce some breaking change (xmlid of views, models, data will change and will required adaption in custom).

Then on V16 we

On V14 if we want to use the new search-engine, we create a module "shopinvader_search_engine_futur" (or "shopinvader_search_engine_v2") that hide the "shopinvader.product/shopinvader.variant", install the required dependency and provide an helper or migrate the data, then you can uninstall shopinvader_search_engine (the shopinvader.product and shopinvader.variant will be still here but not used).

What do you think @lmignon @sbidoul @simahawk

hparfr commented 1 year ago

+1 on solution 2

lmignon commented 1 year ago

@sebastienbeau I would tend to go with 'solution 2'. The most important consideration to keep in mind is to have a symmetry in the module naming of actual shopinvader addon between 14 and 16 (at least for the maximum of addons) to ease the back/forward port.

simahawk commented 1 year ago

At first glance I tend to go w/ option 2 but is not clear what would be moved to *_v1* and what would you keep in shopinvader. Can you elaborate a bit on this?

In general, to introduce this in v14, it's really critical to maintain full backward compat. How you say in French: "ça va sans dire" :wink:

And possibly having 2 api versions working at the same time (eg: using /shopinvader/v2/* for v2) but I don't think this is going to be a problem.

What is unclear to me is to what extent we can preserve the backend apis.

This will likely be more problematic. We could probably wrap the backend api w/ a component or other tool so that both versions can use the same high level api seamlessly.

sebastienbeau commented 1 year ago

I also tend to go to solution 2 as it allow us to have the "same" code and module on V14 and V16 so backport forwardport will be easier.

I will put somebody from Akretion on this split.

sebastienbeau commented 1 year ago

TODO list

components

models

data

demo (do in last, maybe too many impact on test)

views

services

wizard

tests

Binding the product in the common test will be useless (base module do not have restriction anymore) https://github.com/shopinvader/odoo-shopinvader/blob/d47c427594875f413016ab024abf83e9593fde11/shopinvader/tests/common.py#L97

When test are green

sebastienbeau commented 1 year ago

Work in progress here : https://github.com/shopinvader/odoo-shopinvader/pull/1382/files

qgroulard commented 1 year ago

Migration of shopinvader_restapi to 16 here: https://github.com/shopinvader/odoo-shopinvader/pull/1403

github-actions[bot] commented 3 weeks ago

There hasn't been any activity on this issue in the past 6 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. If you want this issue to never become stale, please ask a PSC member to apply the "no stale" label.