Skip to content

Conversation

@haroldclaus
Copy link
Contributor

@haroldclaus haroldclaus commented Sep 27, 2018

Original Pull Request

#18322

Description

The bug caused issues during the export of analytics data from advanced
reporting. The SCANDIR has a parameter where the sort can be determined, but
using UNSORTED gives you unwanted lists of files/dirs. Therefor deleting
the first 2 via array_shift, is not a proper way of dealing with this. With this PR, it doesn't matter how you sort the results of the SCANDIR function, you'll always remove in a proper way the "." and "..".

Fixed Issues (if relevant)

No issues were found about this issue

Manual testing scenarios

  • Running of "analytics_collect_data" cronjob and check if the "." and ".." were removed in the result of the scandir PHP function.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Sep 27, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @haroldclaus. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

The bug caused issues during the export of analytics data from advanced
reporting. The scandir has parameter were the sort can be placed, but
using UNSORTED gives you unwanted lists of files/dirs. Therefor deleting
the first 2 via array_shift, is not a proper way of dealing with this

Signed-off-by: Harold Claus <[email protected]>
@haroldclaus haroldclaus force-pushed the hotfix/archiveTarRemoveDirsFix branch from 52edb59 to 45f22ac Compare September 27, 2018 12:00
@miguelbalparda
Copy link
Contributor

Hi @haroldclaus! Would you mind reformatting this to use the correct template? Also, we are trying to have everything merged into 2.3 before having it submitted to 2.2

@miguelbalparda
Copy link
Contributor

Thanks for the reformatting @haroldclaus!

@haroldclaus haroldclaus changed the base branch from 2.2-develop to 2.3-develop September 28, 2018 08:31
@haroldclaus haroldclaus changed the base branch from 2.3-develop to 2.2-develop September 28, 2018 08:31
@haroldclaus
Copy link
Contributor Author

Travis CI build failing... It went flawlessly yesterday.
Current error message = "Warning: PDOStatement::execute(): MySQL server has gone away in /home/travis/build/magento/magento2/vendor/magento/zendframework1/library/Zend/Db/Statement/Pdo.php:228"
Can't really help with this one.

Also : "We are trying to have everything merged into 2.3 before having it submitted to 2.2"
Should I do anything specific? Do I need to create a new PR for 2.3-develop?

@okorshenko
Copy link
Contributor

okorshenko commented Sep 28, 2018

Hi @haroldclaus
Thank you for your first-time contribution! 👍

Could you please create another pull request to 2.3-develop branch?
We will merge PR to 2.3-develop and then we will accept this one.
Thank you!

@haroldclaus
Copy link
Contributor Author

@okorshenko Done ;)

#18322

@ihor-sviziev ihor-sviziev self-assigned this Oct 1, 2018
@ihor-sviziev ihor-sviziev changed the title Fix for removing the dirs ('/./', '/../') while creating a TAR archive [Backport] Fix for removing the dirs ('/./', '/../') while creating a TAR archive Oct 1, 2018
@ihor-sviziev ihor-sviziev changed the title [Backport] Fix for removing the dirs ('/./', '/../') while creating a TAR archive [Backport] Fix for removing the dirs while creating a TAR archive Oct 1, 2018
@ihor-sviziev
Copy link
Contributor

Hi @haroldclaus,
Thank you so much! Will process you pull requests ASAP

@magento-engcom-team magento-engcom-team added this to the Release: 2.2.8 milestone Oct 4, 2018
@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, thank you for the review.
ENGCOM-3087 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

Hi @haroldclaus. Thank you for your contribution.
We will aim to release these changes as part of 2.2.8.
Please check the release notes for final confirmation.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.

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.

6 participants