zookal / harrisstreet-impex Goto Github PK

When we added this to the require section of composer.json, we get the following error:

The requested package zookal/harris-street-impex could not be found in any version, there may be a typo in the package name.

It appears the package isn't actually available for composer?

The YAML export escapes single quotes with a \ which is incorrect and causes an error to occur on the import of any such strings.

In YAML, a double quote '' (i.e. 2 singles) is how a single ' quote is escaped.

Suggested resolution in src/HarrisStreet/CoreConfigData/Exporter/Yaml.php change:

return '\'' . addcslashes($value, '\'') . '\'';

to

return sprintf("'%s'", str_replace('\'', '\'\'', $value));

Using YAML export a string like '000000' is exported without quotes. This means upon import it is then treated as a numeric value and is imported as 0. You can see how this can cause problems.

Magento core_config_data treats everything like a string so it makes sense if the export / import does also.

Possible solution: in src/HarrisStreet/CoreConfigData/Exporter/Yaml.php remove the is_numeric test and treat everything like a string.

Example YAML:

some_settings/setting/config_value:
  default:
    0: '-15px'

Trying to import the above results in the following error:

[RuntimeException]               
  The "-1" option does not exist.  

Seems to be the same for any value starting with a minus sign.

This is counter-intuitive if you want to write to stdout.

For example the following command does not work:

$ n98-magerun.phar hs:ccd:export --include shipping/origin -fphp://stdout

because the following filename becomes created:

Failed to write: php://stdout.yaml

Automatically adding ".yaml" destroys the portability of the switch. I suggest to remove that feature for -f / --filename and add a new switch that brings it back: --prepend-filename-with-dot-yaml-extension.

I'd like to get some feedback on this suggestion before I consider to provide a PR. Thanks.

This is counter-intuitive if you want to write to stdout.

For example the following command does not work:

$ n98-magerun.phar hs:ccd:export --include shipping/origin -fphp://stdout

because the following filename becomes created:

Failed to write: php://stdout.yaml

Automatically adding ".yaml" destroys the portability of the switch. I suggest to remove that feature for -f / --filename and add a new switch that brings it back: --prepend-filename-with-dot-yaml-extension.

Like the DNS system uses names instead of numbers to make the system more robust in a distributed environment, I see great use in using store and store-view codes (which are names) instead of IDs (which are numbers) when importing data (same can be applied to exporting, too).

This change would be backwards compatible because it can be clearly differentiated between numeric ID and alpha-numeric store code (if you follow the rule to not start it with a number or you don't only use numbers).

I plan to file a PR as I got positive feedback for this feature already from my peers at work. I'm pretty sure it is much appreciated.