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

[ADD] manage multi-company backend #6

Closed

Conversation

Cedric-Pigeon
Copy link

Hi @guewen, could you consider this PR in the migration of the magento connector. It comes from OCA#212 on v8.0.

It ran on production for more than a year so it should be safe.

Kind regards,

Copy link
Owner

@guewen guewen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! I'll definitely integrate it before we merge my PR. Just have one question of the new dependency :)

@@ -7,6 +7,7 @@
'version': '10.0.1.0.0',
'category': 'Connector',
'depends': ['account',
'base_technical_user',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is it required? I can't find this addon

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry it comes from OCA/server-tools#858

It avoids to launch jobs with admin user and uses a dedicated technical company user.
It intends to replace https://github.com/OCA/connector-magento/pull/212/files#diff-131bcecd325b739cf356543a96bb2747R11 in a more generic way.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I see! I added a comment on OCA/server-tools#858

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you plan to change something about OCA/server-tools#858 (comment)?

@guewen guewen force-pushed the 10.0-components branch from b5ec5e5 to 7d0c67f Compare July 10, 2017 13:55
@guewen guewen force-pushed the 10.0-components branch from 368ad1f to fa1b4f2 Compare July 11, 2017 11:40
@guewen guewen force-pushed the 10.0-components branch from 9bc4191 to 6061eb8 Compare July 11, 2017 13:34
@Cedric-Pigeon Cedric-Pigeon force-pushed the 10.0-multi_company-cpi branch from ce2d8b2 to 4e13a9c Compare July 11, 2017 14:03
@guewen guewen force-pushed the 10.0-components branch from 6061eb8 to efec23b Compare July 11, 2017 14:26
@guewen
Copy link
Owner

guewen commented Jul 11, 2017

@Cedric-Pigeon do you have the cassette file for the test test_import_partner_individual_2_addresses_multi_company?

@Cedric-Pigeon
Copy link
Author

Do I have to make a cassette for each test ? I don't understand well how it works ...

@guewen
Copy link
Owner

guewen commented Jul 11, 2017

The cassette is recorded automatically when the test is run and doesn't error. The other test doesn't need a cassette has it doesn't interact with magento. If you are interested: http://vcrpy.readthedocs.io/en/latest/usage.html this is a great lib to know :)

@guewen
Copy link
Owner

guewen commented Jul 11, 2017

I have the feeling that you could either just add your asserts in the existing test_import_partner_individual_2_addresses or reuse the cassetteof this test with something like @recorder.use_cassette('test_import_partner_individual_2_addresses') (but them import the same partner id)

@guewen
Copy link
Owner

guewen commented Jul 12, 2017

I merged at 8f425de. I changed the test with @recorder.use_cassette('test_import_partner_individual_2_addresses') and it works, it reuses the same cassette than the test_import_partner_individual_2_addresses test.

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.

2 participants