-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Fix issue 10032 #10593
Fix issue 10032 #10593
Conversation
@@ -113,6 +113,13 @@ public function testCreate() | |||
)->will( | |||
$this->returnSelf() | |||
); | |||
$this->_backupModel->expects( | |||
$this->once() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please reformat similar to change in app/code/Magento/Backup/Model/BackupFactory.php
, current ugly formatting is derived from poor formatting tool.
)->setPath( | ||
$backup->getPath() | ||
); | ||
if (strpos($backup->getId(), $backupId) !== false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we replace this condition with more accurate one? In which case previous $backup->getId() == $backupId
condition didn't work?
For instance, if the string contained additional slash, we should search for '/' . $backupId
and so on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
The reason for using strpos was that the id returned by the collection has the name appended to the time stamp and type.
$backup->getId() ==> 1503256588_nomediasecond (notice no underscore between nomedia and 'second' which is the name of the backup)
Could be a bug in the _generateRow method of Magento\Backup\Model\Fs\Collection.
The files created in the backup folder have the name convention of {{time_stamp}}{{type}}{{name}}.tgz
The id returned by the collection does not have the second underscore.
In the mass delete controller action only the time stamp and type is used to identify the backup.
Magento\Backup\Controller\Adminhtml\Index\MassDelete:39
list($time, $type) = explode('_', $id);
$backupModel = $this->_backupModelFactory->create($time, $type)->deleteFile();
I updated the comparison to check for the type and time stamp only like in the mass delete action.
Hope that is ok.
@orlangur updated |
Description
This fixes the issue where the link to the download of a backup in the admin gives the wrong file.
The wrong file seems to be the latest backup that was generated.
Fixed Issues
Manual testing scenarios
Contribution checklist