Skip to content

[15.0][MIG] component#420

Closed
ajaniszewska-dev wants to merge 96 commits intoOCA:15.0from
ajaniszewska-dev:15.0-mig-component
Closed

[15.0][MIG] component#420
ajaniszewska-dev wants to merge 96 commits intoOCA:15.0from
ajaniszewska-dev:15.0-mig-component

Conversation

@ajaniszewska-dev
Copy link
Copy Markdown

No description provided.

OCA-git-bot and others added 30 commits October 17, 2021 16:36
The favorite lookup should be by 'usage' (kinda interface) and by model
name ('components()' method).  The lookup by component name should
normally not be used as it reduces the flexibility.  Using a different
method for this lookup discourage its usage.  Also, the lookup by
component name completely ignores the current collection, which is bad
or not bad depending of what you want to achieve.
when inheriting from multiple components, we might inadvertly override a
value with None
Used for instance by the 'ecommerce' components
The mapping methods are now built by the component initialization when
the "aggregated" class is built.
And remove automatic naming from class name, not explicit enough,
especially when we have to inherit from one.

We could take the total opposite and only use class names components:

class MyComponent(Component):
    ...

class MyComponentExtended(Component):
    _inherit = 'MyComponent'

But it would be less close to the odoo's API.
============================================================================================================= test session starts =============================================================================================================
platform linux2 -- Python 2.7.9, pytest-3.0.7, py-1.4.33, pluggy-0.4.0
run-last-failure: run all (no recorded failures)
rootdir: /opt/odoo, inifile:
plugins: cov-2.5.1, odoo-0.2.2
collected 25 items
odoo/external-src/connector/component/tests/test_build_component.py .........
odoo/external-src/connector/component/tests/test_component.py ........
odoo/external-src/connector/component/tests/test_lookup.py ......
odoo/external-src/connector/component/tests/test_work_on.py ..
---------- coverage: platform linux2, python 2.7.9-final-0 -----------
Name                                                                  Stmts   Miss  Cover
-----------------------------------------------------------------------------------------
odoo/external-src/connector/component/__init__.py                         4      0   100%
odoo/external-src/connector/component/__manifest__.py                     1      1     0%
odoo/external-src/connector/component/base_components.py                  3      0   100%
odoo/external-src/connector/component/builder.py                         18      0   100%
odoo/external-src/connector/component/core.py                           155      6    96%
odoo/external-src/connector/component/exception.py                        3      0   100%
odoo/external-src/connector/component/models/__init__.py                  1      0   100%
odoo/external-src/connector/component/models/collection.py                8      0   100%
odoo/external-src/connector/component/tests/__init__.py                   4      0   100%
odoo/external-src/connector/component/tests/common.py                    26      2    92%
odoo/external-src/connector/component/tests/test_build_component.py     102      0   100%
odoo/external-src/connector/component/tests/test_component.py            56      0   100%
odoo/external-src/connector/component/tests/test_lookup.py               84      0   100%
odoo/external-src/connector/component/tests/test_work_on.py              26      0   100%
-----------------------------------------------------------------------------------------
TOTAL                                                                   491      9    98%
========================================================================================================== 25 passed in 0.76 seconds ==========================================================================================================
the 'components' method had 2 different return types depending of the
'multi' argument.

Now we have 'component' or 'many_components' that return a Component
instance or a list of Component instances.
Proposing a new API based on components for the events
Because they can have different addons
* Move the methods to get components from the components to the
WorkContext, it should really be its responsibilities to get them.
There are shorcuts on the components though.
My first implementation was like that, then I moved the lookup methods
on the components thinking that it would allow to customize them. But if
we did that, we would have a different behavior from component to
component which is bad, so if one really wanted to do that, better have
to work with a different subclass of WorkContext, which would have its
own behavior, consistently throughout all the work session.

* Allow to switch the collection using work_on(), not only the
  model_name
Tests using odoo transactions must run post install, because during the
install the registry is not ready, so the components aren't neither.
For consistency, this is where components should go (as for models,
views, ...)
Checking that the odoo registry is ready is not working:
in tests, the events are not working because they are run
just before the odoo registry is set to ready.
(might be empty for EventWorkContext)
It allows to open/close objects on start/end of a synchro / work
session. The base method does none of that, but this is a really common
use case. Example: at the beginning of the work session, I open a
webservice client that is available in our WorkContext for the duration
of our work. At the end, I close the client to end the connection.
…ule is loaded with test-enable=True, the components of this module are not registerd by the call to the method _register_hook of the component.builder. Indeed the state of the module is still *to install*. This change allows to call the method *load_components* on the component.builder for a specific module by taking care to not reload the components of a module already loaded.
@ajaniszewska-dev ajaniszewska-dev marked this pull request as ready for review November 19, 2021 10:51
@simahawk simahawk changed the title 15.0 mig component [15.0][MIG] component Nov 19, 2021
@simahawk
Copy link
Copy Markdown
Contributor

pls check pre-commit too

@ajaniszewska-dev ajaniszewska-dev force-pushed the 15.0-mig-component branch 3 times, most recently from a99aecd to 7ddaa9b Compare November 23, 2021 09:13
@simahawk
Copy link
Copy Markdown
Contributor

@ajaniszewska-dev if you delete commits w/ push force, the comment done on specific commit lose reference. Just use fixup for now 🙏

@simahawk
Copy link
Copy Markdown
Contributor

@ajaniszewska-dev if you delete commits w/ push force, the comment done on specific commit lose reference. Just use fixup for now pray

or maybe is github is just crazy here because is the 1st time I see this messed up summary of commits 🤔

@simahawk
Copy link
Copy Markdown
Contributor

@ajaniszewska-dev there are still things to be fixed. Could you please propose a PR to add the module as not installable first? We can fast track it and then we'll have an easier review here.

@lmignon
Copy link
Copy Markdown
Contributor

lmignon commented Nov 25, 2021

To ease the migration and avoid all this mess with the commit histrory, I propose to recreate the 15.0 branch from the 14.0 with all the addons declared with `"installable": False. What do you think about @simahawk @guewen @yankinmax

@simahawk
Copy link
Copy Markdown
Contributor

@lmignon I was thinking the same but I had some issues w/ my MT setup and I had no time yesterday.
I'll see if I can do it today, unless any of you can make it before 😉

@lmignon
Copy link
Copy Markdown
Contributor

lmignon commented Nov 25, 2021

@simahawk I'll do it into the coming hours

@lmignon
Copy link
Copy Markdown
Contributor

lmignon commented Nov 25, 2021

@yankinmax @simahawk @guewen 15.0 is now initialized from 14.0...

@dreispt
Copy link
Copy Markdown
Member

dreispt commented Nov 26, 2021

@liweijie0812 @simahawk @guewen I updated #418 accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.