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

[IMP] add product options from php serialize strings in sale line #94

Closed
wants to merge 2 commits into from

Conversation

bealdav
Copy link
Member

@bealdav bealdav commented Apr 3, 2015

Here is the type of format
opt

options = [elm['label'] + ': ' + elm['print_value']
for elm in
record['product_options']['options'].values()]
name += '\nOptions:\n - ' + '\n - '.join(options)
Copy link
Member

Choose a reason for hiding this comment

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

What about translations?

@guewen
Copy link
Member

guewen commented Apr 7, 2015

Thanks for the patch.

However, I don't want to add a hard dependency for that since it is not used by many and could be an issue for existing installation.

I propose to replace the import by:

try:
    import phpserialize
except ImportError:
    phpserialize = None

And to check if phpserialize is not None before using it.
This optional dependency being explained in the readme.

@chafique-delli
Copy link

Effectively as told 'Guewen', I think that it would be better to have an optionnal dependency on 'phpserialize'.
May be an option to check in the backend ?

@bealdav
Copy link
Member Author

bealdav commented Apr 17, 2015

HI @guewen, what do you think about @chafique-delli suggestion ? What should be the default behavior ?

@guewen
Copy link
Member

guewen commented Apr 20, 2015

HI @guewen, what do you think about @chafique-delli suggestion ? What should be the default behavior ?

Hi,

No point to have an option in the backend. If the library is there, use it, otherwise just ignore the options.

@bealdav
Copy link
Member Author

bealdav commented Apr 20, 2015

Hi @guewen, thanks but where I put options from previous mapping in case of none ?

@guewen
Copy link
Member

guewen commented Apr 20, 2015

Hi @guewen, thanks but where I put options from previous mapping in case of none ?

I noticed that the previous mapping was writing the options in a field notes which does not exist. Possibly we can just keep the previous mapping in notes in v7 so if someone use it in that way it won't change the behavior.

@bealdav
Copy link
Member Author

bealdav commented Apr 21, 2015

@guewen I fixed it but with by resetting my commit to keep old code.
This first commit is without phpserialize, if ok, I'll modify .travis.yml to add this lib

@@ -521,6 +534,18 @@ def _clean_magento_items(self, resource):

# Group the childs with their parent
for item in resource['items']:
if phpserialize is None:
_logger.info(PHPSERIALIZE_INFO)
Copy link
Member

Choose a reason for hiding this comment

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

This log will be logged at each import, I think it would be better to log it once in the ImportError.

@bealdav
Copy link
Member Author

bealdav commented Apr 21, 2015

@guewen I suggest to add an item in 'Technical points' to alert of phpserialize feature

@guewen
Copy link
Member

guewen commented Apr 21, 2015

@guewen I suggest to add an item in 'Technical points' to alert of phpserialize feature

Yep

@guewen
Copy link
Member

guewen commented Apr 22, 2015

Did you see the error in the tests?

2015-04-21 12:41:27,412 24044 TEST openerp_test openerp.modules.module: Traceback (most recent call last):
2015-04-21 12:41:27,412 24044 TEST openerp_test openerp.modules.module: `   File "/home/travis/build/OCA/connector-magento/magentoerpconnect/tests/test_sale_order.py", line 112, in test_copy_quotation_delay_export_state
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `     binding = self._import_sale_order(900000691)
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `   File "/home/travis/build/OCA/connector-magento/magentoerpconnect/tests/test_sale_order.py", line 44, in _import_sale_order
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `     backend_id, increment_id)
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `   File "/home/travis/build/OCA/connector-magento/magentoerpconnect/unit/import_synchronizer.py", line 378, in import_record
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `     importer.run(magento_id, force=force)
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `   File "/home/travis/build/OCA/connector-magento/magentoerpconnect/unit/import_synchronizer.py", line 206, in run
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `     self.magento_record = self._get_magento_data()
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `   File "/home/travis/build/OCA/connector-magento/magentoerpconnect/sale.py", line 688, in _get_magento_data
2015-04-21 12:41:27,413 24044 TEST openerp_test openerp.modules.module: `     record = self._clean_magento_items(record)
2015-04-21 12:41:27,414 24044 TEST openerp_test openerp.modules.module: `   File "/home/travis/build/OCA/connector-magento/magentoerpconnect/sale.py", line 545, in _clean_magento_items
2015-04-21 12:41:27,414 24044 TEST openerp_test openerp.modules.module: `     item[key].encode('utf-8'), decode_strings=True)
2015-04-21 12:41:27,414 24044 TEST openerp_test openerp.modules.module: ` AttributeError: 'dict' object has no attribute 'encode'

@pedrobaeza
Copy link
Member

This one has conflicts and very old, so I'm closing it. If there's still interest, feel free to reopen it fixing the problems.

@pedrobaeza pedrobaeza closed this Apr 28, 2018
jcoux pushed a commit to camptocamp/connector-magento that referenced this pull request May 14, 2019
…n_picking

[Action #8537] Export Stock-IT picking IN one time
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.

None yet

4 participants