Skip to content
This repository has been archived by the owner on Apr 29, 2019. It is now read-only.

Added $connection property to class #141

Merged
merged 4 commits into from
Dec 6, 2018
Merged

Added $connection property to class #141

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac90f7a
Added $connection property to class
maxalmonte14 Oct 26, 2018
6789eb4
Removed unused connection property, added PHPMD.UnusedFormalParameter…
maxalmonte14 Oct 30, 2018
7f3808c
Added public connection property
maxalmonte14 Nov 10, 2018
5ff88c0
Removed PHPMD suppress warning directive
maxalmonte14 Nov 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/code/Magento/DownloadableImportExport/Helper/Uploader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class Uploader extends \Magento\Framework\App\Helper\AbstractHelper
* @param \Magento\CatalogImportExport\Model\Import\UploaderFactory $uploaderFactory
* @param \Magento\Framework\App\ResourceConnection $resource
* @param \Magento\Framework\Filesystem $filesystem
*
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess now we do not need this suppress warning since the connection is being set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

*/
public function __construct(
\Magento\Framework\App\Helper\Context $context,
Expand All @@ -60,7 +62,6 @@ public function __construct(
$this->fileUploader->init();
$this->fileUploader->setAllowedExtensions($this->getAllowedExtensions());
$this->fileUploader->removeValidateCallback('catalog_product_image');
$this->connection = $resource->getConnection('write');
Copy link
Contributor

Choose a reason for hiding this comment

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

We are not allowed to remove public properties, because according to PHP Property Visibility: Methods declared without any explicit visibility keyword are defined as public.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah forgot about that. @maxalmonte14 would you be able to add the $connection as public then in future versions we can deprecate it. But it would be better at least to have it shown and public now. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I add the property as public with a default null value, or should I leave the implementation as before?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add the property without a default and then put this implementation back as it was before. Unless @sidolov thinks best otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, now the $connection property is public.

$this->mediaDirectory = $filesystem->getDirectoryWrite(DirectoryList::ROOT);
}

Expand Down