fixed File type custom option value not showing properly on wish list page #24319 #24320

rani-priya · 2019-08-27T14:05:46Z

fixed File type custom option value not showing properly on wish list page #24319

Description (*)

Fixed Issues (if relevant)

File type custom option value not showing properly on wish list page #24319: fixed File type custom option value not showing properly on wish list page

Manual testing scenarios (*)

create a simple product with file type custom option
open product at frontend and then select a file
add it to wish list

Questions or comments

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 are green)

m2-assistant · 2019-08-27T14:05:49Z

Hi @rani-webkul. 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 give me test instance - deploy test instance based on PR changes
@magento give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

VladimirZaets · 2019-08-27T15:43:39Z

@magento give me 2.3-develop instance

magento-engcom-team · 2019-08-27T15:43:42Z

Hi @VladimirZaets. Thank you for your request. I'm working on Magento 2.3-develop instance for you

magento-engcom-team · 2019-08-27T15:44:14Z

Hi @VladimirZaets, here is your Magento instance.
Admin access: https://i-24320-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

magento-engcom-team · 2019-08-27T17:16:04Z

Hi @VladimirZaets, thank you for the review.
ENGCOM-5715 has been created to process this Pull Request
✳️ @VladimirZaets, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

engcom-Alfa · 2019-08-29T07:44:33Z

Hi @rani-webkul

During testing we faced the issue.

Problem: "404 not found" error message occurs in details information when we click on file link

Steps to reproduce:

Create a simple product with file type custom option;
Open product at frontend and then select a file;
Add it to wish list;
Go to Wish list page;
In details information click on file link;

Actual Result:

@rani-webkul Could you take a look, please?

Thanks!

rani-priya · 2019-08-30T18:21:59Z

@engcom-Alfa I have made required changes. Please do have a look.

ghost · 2019-08-31T18:19:17Z

@rani-webkul unfortunately, only members of the maintainers team are allowed to remove progress related labels to the pull request

rani-priya · 2019-08-31T18:21:07Z

@VladimirZaets I have made required changes, Please review.

rani-priya · 2019-09-04T08:44:33Z

@VladimirZaets Please review

rani-priya · 2019-09-10T04:02:48Z

@VladimirZaets Please review

ihor-sviziev · 2019-09-11T05:51:38Z

app/code/Magento/Wishlist/Controller/Index/DownloadCustomOption.php

-                    ['value' => $info['quote_path'], 'type' => 'filename'],
-                    DirectoryList::ROOT,
+                    ['value' => $this->_rootDir->getRelativePath($info['quote_path']), 'type' => 'filename'],
+                    $this->rootDirBasePath,


Hi @rani-webkul,
Wouldn't it be enough to change only DirectoryList::ROOT to DirectoryList::MEDIA?

Looks like that was the main issue why it returned no route page and we don't need to change value

Still it works after this change? :)

yes.. it's working.. I have verified it.

thanks.. 👍

ihor-sviziev

See my comment above

ihor-sviziev

Hi,

Could you revert all not related changes, for instance adding
protected $rootDirBasePath;, additional constructor arguments? We don't really need them anymore.
Just to keep only changes

$option['value'] = $this->escapeHtml($option['value'], ["a"]);

and

DirectoryList::MEDIA,

Also would be great to squash all your changes into single commit, only if you know how to do that.

… page magento#24319 fixed wishlist custom option download issue made required changes removed rootDirBasePath from constructor argument removed unnecessary arguments

rani-priya · 2019-09-11T07:48:11Z

@ihor-sviziev Done in a single commit.

magento-engcom-team · 2019-09-11T08:33:43Z

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

rani-priya · 2019-09-11T08:36:17Z

@ihor-sviziev could you please also review #24390 ?

ihor-sviziev · 2019-09-11T09:33:50Z

Sure, will check it

…

On Wed, Sep 11, 2019 at 11:39 Rani Priya ***@***.***> wrote: @ihor-sviziev <https://github.com/ihor-sviziev> could you please also review #24390 <#24390> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24320?email_source=notifications&email_token=AAOJOUL3RJ4JCTV3TCAJCOLQJCVCLA5CNFSM4IQE5FVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6NXETA#issuecomment-530281036>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAOJOUPJ43PFFPNSQBHYVQDQJCVCLANCNFSM4IQE5FVA> .

engcom-Alfa · 2019-09-11T09:59:44Z

✔️ QA Passed

… on wish list page #24319 #24320

m2-assistant · 2019-09-12T14:02:16Z

Hi @rani-webkul, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento-engcom-team added Component: Wishlist Release Line: 2.3 labels Aug 27, 2019

VladimirZaets approved these changes Aug 27, 2019

View reviewed changes

ghost assigned VladimirZaets Aug 27, 2019

ghost added the Progress: ready for testing label Aug 27, 2019

engcom-Alfa self-assigned this Aug 29, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Aug 29, 2019

ghost added Progress: needs update and removed Progress: testing in progress labels Aug 29, 2019

rani-priya requested a review from VladimirZaets August 30, 2019 06:06

rani-priya added Partner - contribution and removed Progress: needs update labels Aug 31, 2019

ghost added the Progress: needs update label Aug 31, 2019

sidolov removed the Partner - contribution label Sep 3, 2019

ihor-sviziev reviewed Sep 11, 2019

View reviewed changes

ihor-sviziev requested changes Sep 11, 2019

View reviewed changes

ghost assigned ihor-sviziev Sep 11, 2019

rani-priya requested a review from paliarush as a code owner September 11, 2019 06:27

ihor-sviziev requested changes Sep 11, 2019

View reviewed changes

fixed File type custom option value not showing properly on wish list…

62c216c

… page magento#24319 fixed wishlist custom option download issue made required changes removed rootDirBasePath from constructor argument removed unnecessary arguments

rani-priya force-pushed the 24319 branch from cd2a760 to 62c216c Compare September 11, 2019 07:45

ihor-sviziev approved these changes Sep 11, 2019

View reviewed changes

ghost added Progress: ready for testing and removed Progress: needs update labels Sep 11, 2019

ihor-sviziev added the Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests label Sep 11, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Sep 11, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Sep 11, 2019

engcom-Foxtrot self-assigned this Sep 11, 2019

ghost added Progress: accept and removed Progress: extended testing labels Sep 11, 2019

magento-engcom-team added the Award: bug fix label Sep 11, 2019

magento-engcom-team mentioned this pull request Sep 12, 2019

File type custom option value not showing properly on wish list page #24319

Closed

magento-engcom-team pushed a commit that referenced this pull request Sep 12, 2019

ENGCOM-5715: fixed File type custom option value not showing properly…

19371e3

… on wish list page #24319 #24320

magento-engcom-team merged commit 62c216c into magento:2.3-develop Sep 12, 2019

magento-engcom-team added this to the Release: 2.3.4 milestone Sep 12, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixed File type custom option value not showing properly on wish list page #24319 #24320

fixed File type custom option value not showing properly on wish list page #24319 #24320

rani-priya commented Aug 27, 2019

m2-assistant bot commented Aug 27, 2019

VladimirZaets commented Aug 27, 2019

magento-engcom-team commented Aug 27, 2019

magento-engcom-team commented Aug 27, 2019

magento-engcom-team commented Aug 27, 2019

engcom-Alfa commented Aug 29, 2019

rani-priya commented Aug 30, 2019

ghost commented Aug 31, 2019

rani-priya commented Aug 31, 2019

rani-priya commented Sep 4, 2019

rani-priya commented Sep 10, 2019

ihor-sviziev Sep 11, 2019 •

edited

Loading

rani-priya Sep 11, 2019

ihor-sviziev Sep 11, 2019

rani-priya Sep 11, 2019

rani-priya Sep 11, 2019

ihor-sviziev left a comment

ihor-sviziev left a comment

rani-priya commented Sep 11, 2019

magento-engcom-team commented Sep 11, 2019

rani-priya commented Sep 11, 2019

ihor-sviziev commented Sep 11, 2019 via email

engcom-Alfa commented Sep 11, 2019

m2-assistant bot commented Sep 12, 2019

fixed File type custom option value not showing properly on wish list page #24319 #24320

fixed File type custom option value not showing properly on wish list page #24319 #24320

Conversation

rani-priya commented Aug 27, 2019

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Aug 27, 2019

VladimirZaets commented Aug 27, 2019

magento-engcom-team commented Aug 27, 2019

magento-engcom-team commented Aug 27, 2019

magento-engcom-team commented Aug 27, 2019

engcom-Alfa commented Aug 29, 2019

rani-priya commented Aug 30, 2019

ghost commented Aug 31, 2019

rani-priya commented Aug 31, 2019

rani-priya commented Sep 4, 2019

rani-priya commented Sep 10, 2019

ihor-sviziev Sep 11, 2019 • edited Loading

Choose a reason for hiding this comment

rani-priya Sep 11, 2019

Choose a reason for hiding this comment

ihor-sviziev Sep 11, 2019

Choose a reason for hiding this comment

rani-priya Sep 11, 2019

Choose a reason for hiding this comment

rani-priya Sep 11, 2019

Choose a reason for hiding this comment

ihor-sviziev left a comment

Choose a reason for hiding this comment

ihor-sviziev left a comment

Choose a reason for hiding this comment

rani-priya commented Sep 11, 2019

magento-engcom-team commented Sep 11, 2019

rani-priya commented Sep 11, 2019

ihor-sviziev commented Sep 11, 2019 via email

engcom-Alfa commented Sep 11, 2019

m2-assistant bot commented Sep 12, 2019

ihor-sviziev Sep 11, 2019 •

edited

Loading