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.

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

[17.0][MIG] component + test_component: Migration to 17.0 #474

Merged
merged 154 commits into from
Dec 8, 2023

Conversation

nguyenminhchien
Copy link

@nguyenminhchien nguyenminhchien commented Nov 24, 2023

Ref: BSRD-701

guewen and others added 30 commits November 23, 2023 15:54
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.
It the previous commit, @lmignon added the possibility to load all
components of an addon in a Component Registry.  This commit takes
benefit of this feature to simplify the existing tests and to add a base
test case for the Connector (that loads all components of 'component',
'component_event', 'connector'). It can be used in implementations using
the connector.
This method can be used to filter out some components. It is executed on
every candidate component returned by a lookup for a
collection/usage/model so must be kept light to execute if possible.

A use case can be for instance to have more than one kind of export in a
connector for a model.

Say that you have a different way to export customers and suppliers:

    with self.work_on('res.partner', kind='customer') as work:
        exporter = work.component(usage='record.exporter)
        exporter.run()

    with self.work_on('res.partner', kind='supplier') as work:
        exporter = work.component(usage='record.exporter)
        exporter.run()

You can declare your components as:

  class CustomerExporter(Component):

      _name = 'customer.exporter'
      _inherit = ['base.exporter']
      _usage = 'record.exporter'
      _apply_on = ['res.partner']

      @classmethod
      def _component_match(cls, work):
          return work.kind == 'customer'

      def run(self):
          # export customer

  class SupplierExporter(Component):

      _name = 'supplier.exporter'
      _inherit = ['base.exporter']
      _usage = 'record.exporter'
      _apply_on = ['res.partner']

      @classmethod
      def _component_match(cls, work):
          return work.kind == 'supplier'

      def run(self):
          # export supplier
When we want to test the implementation of a connector, it must be
simple. Add 2 new test cases for TransactionCase and SavepointCase which
load all the components of the dependencies addons on the tested addons.
Only the components of the addons in state 'installed' are loaded, so we
don't load components of addons that depend on the tested addon.

The tests must call _init_global_registry and build_registry instead of
_register_hook, so the registry is not set to ready. If the global
registry is set to ready during tests, the event will trigger during the
installation of addons which is not what we want.

The existing test case is meant only to be used when one has to
manipulate components in a custom registry.

See discussion on
guewen@447b22f#commitcomment-22851711
damdam-s and others added 21 commits November 30, 2023 09:49
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: connector-16.0/connector-16.0-test_component
Translate-URL: https://translation.odoo-community.org/projects/connector-16-0/connector-16-0-test_component/
Currently translated at 77.7% (7 of 9 strings)

Translation: connector-16.0/connector-16.0-test_component
Translate-URL: https://translation.odoo-community.org/projects/connector-16-0/connector-16-0-test_component/it/
Currently translated at 88.8% (8 of 9 strings)

Translation: connector-16.0/connector-16.0-test_component
Translate-URL: https://translation.odoo-community.org/projects/connector-16-0/connector-16-0-test_component/it/
Currently translated at 100.0% (9 of 9 strings)

Translation: connector-16.0/connector-16.0-test_component
Translate-URL: https://translation.odoo-community.org/projects/connector-16-0/connector-16-0-test_component/es/
Currently translated at 100.0% (9 of 9 strings)

Translation: connector-16.0/connector-16.0-test_component
Translate-URL: https://translation.odoo-community.org/projects/connector-16-0/connector-16-0-test_component/it/
@gurneyalex
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 17.0-ocabot-merge-pr-474-by-gurneyalex-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

@gurneyalex The merge process could not be finalized, because command git push origin 17.0-ocabot-merge-pr-474-by-gurneyalex-bump-nobump:17.0 failed with output:

To https://github.com/OCA/connector
 ! [remote rejected]   17.0-ocabot-merge-pr-474-by-gurneyalex-bump-nobump -> 17.0 (cannot lock ref 'refs/heads/17.0': is at c76dccdf6f7f2202d93c4b77aa1351cdc956ceaf but expected 6f10c340db1e5e52211300b3d26d07ffbdfac909)
error: failed to push some refs to 'https://***@github.com/OCA/connector'

@OCA-git-bot OCA-git-bot merged commit 78fadad into OCA:17.0 Dec 8, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.