Fix for issue 19983 Can't upload customer Image attribute programmatically #19988

Nazar65 · 2018-12-27T09:38:41Z

Description (*)

Could not load image for customer attribute, error: Base64 is not defined "

Fixed Issues (if relevant)

Can't work customer Image attribute programmatically #19983: Could not load image for customer attribute,

Manual testing scenarios (*)

1-Create customer attribute type file ->

$eavSetup = $this->eavSetupFactory->create(['setup' => $setup]);
        $eavSetup->addAttribute(
            \Magento\Customer\Model\Customer::ENTITY,
            'sample_attribute2',
            [
                'type'         => 'text',
                'label'        => 'Sample Attribute',
                'input'        => 'file',
                'required'     => false,
                'visible'      => true,
                'user_defined' => false,
                'position'     => 999,
                'system'       => 0,
            ]
        );
        $sampleAttribute = $this->eavConfig->getAttribute(Customer::ENTITY, 'sample_attribute');

        // more used_in_forms ['adminhtml_checkout','adminhtml_customer','adminhtml_customer_address','customer_account_edit','customer_address_edit','customer_register_address']
        $sampleAttribute->setData(
            'used_in_forms',
            ['adminhtml_customer','customer_register_address','customer_address_edit','customer_account_edit']

        );
        $sampleAttribute->save();

2. try to upload image on adminhtml customer edit.

Expected Result

No errors in Console, Image uploaded

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-engcom-team · 2018-12-27T09:38:42Z

Hi @Nazar65. 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 2.3-develop instance - deploy vanilla Magento instance

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

orlangur · 2018-12-27T11:57:42Z

app/code/Magento/Eav/Model/Attribute/Data/File.php

@@ -146,7 +146,7 @@ protected function _validateByRules($value)
            return $this->_fileValidator->getMessages();
        }

-        if (!empty($value['tmp_name']) && !is_uploaded_file($value['tmp_name'])) {
+        if (empty($value['tmp_name']) && !is_uploaded_file($value['tmp_name'])) {


Will this ever evaluate to true?

Yes on event where empty value of ['tmp_name'] and file not uploaded

The condition looks correct to me. If you really need a check for empty($value['tmp_name']) - make it a separate condition

However, I'd leave php side as it is

orlangur · 2018-12-27T11:58:40Z

app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js

@@ -16,7 +16,8 @@ define([
    'Magento_Ui/js/form/element/abstract',
    'mage/backend/notification',
    'mage/translate',
-    'jquery/file-uploader'
+    'jquery/file-uploader',
+    'mage/adminhtml/tools'


Why need tools?

"Base64 is not defined" when try upload image -> caused by line 173 ->

if (!file.id && file.name) { file.id = Base64.mageEncode(file.name); }

This is core issue for -> #18688

That's interesting, there are more js files using Base64 without dependency on mage/adminhtml/tools - probably those files require to be fixed too

Nazar65 · 2018-12-27T13:38:46Z

Hi @orlangur I was leave a comment

Nazar65 · 2019-01-08T09:09:58Z

@orlangur any updates ?

sivaschenko · 2019-01-16T20:49:54Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-01-16T20:49:57Z

Hi @sivaschenko. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-01-16T20:50:37Z

Hi @sivaschenko, here is your new Magento instance.
Admin access: https://pr-19988.instances.magento-community.engineering/admin
Login: admin Password: 123123q

sivaschenko · 2019-01-16T20:52:01Z

app/code/Magento/Eav/Model/Attribute/Data/File.php

@@ -146,7 +146,7 @@ protected function _validateByRules($value)
            return $this->_fileValidator->getMessages();
        }

-        if (!empty($value['tmp_name']) && !is_uploaded_file($value['tmp_name'])) {
+        if (empty($value['tmp_name']) && !is_uploaded_file($value['tmp_name'])) {


The condition looks correct to me. If you really need a check for empty($value['tmp_name']) - make it a separate condition

sivaschenko · 2019-01-16T20:52:40Z

app/code/Magento/Eav/Model/Attribute/Data/File.php

@@ -146,7 +146,7 @@ protected function _validateByRules($value)
            return $this->_fileValidator->getMessages();
        }

-        if (!empty($value['tmp_name']) && !is_uploaded_file($value['tmp_name'])) {
+        if (empty($value['tmp_name']) && !is_uploaded_file($value['tmp_name'])) {


However, I'd leave php side as it is

sivaschenko · 2019-01-16T21:07:13Z

app/code/Magento/Ui/view/base/web/js/form/element/file-uploader.js

@@ -16,7 +16,8 @@ define([
    'Magento_Ui/js/form/element/abstract',
    'mage/backend/notification',
    'mage/translate',
-    'jquery/file-uploader'
+    'jquery/file-uploader',
+    'mage/adminhtml/tools'


That's interesting, there are more js files using Base64 without dependency on mage/adminhtml/tools - probably those files require to be fixed too

sivaschenko · 2019-01-16T21:08:05Z

Hi @Nazar65 thanks for the contribution please see my code review notes

Nazar65 · 2019-01-17T08:59:20Z

@sivaschenko Well, I will provide a better fix for this problem, thank you

Nazar65 · 2019-01-17T10:51:31Z

@sivaschenko Base64 defined by adminhtml tools please loook ->

magento2/lib/web/mage/adminhtml/grid.js

Line 6 in 9c36f6f

// also depends on a mage/adminhtml/tools.js for Base64 encoding

There is no difference, all other files are using mage/adminhtml

sivaschenko · 2019-01-17T10:59:25Z

@Nazar65 you are completely right about Base64, I just mentioned that it appears there may be more bugs in Magento because of Base64 usage without dependency on mage/adminhtml/tools in several other js files

Nazar65 · 2019-01-17T22:03:52Z

@sivaschenko i'm added adminhtml/tools where is used but not defined, what about php part ? for me this is the good condition on "if" statement.

sivaschenko

Hi @Nazar65 thanks for your updates, please see my comment with the suggestion regarding the php condition

app/code/Magento/Eav/Model/Attribute/Data/File.php

Co-Authored-By: Nazar65 <nazarn96@gmail.com>

Nazar65 · 2019-01-18T12:36:47Z

@sivaschenko I'm found the core problem of issue, the function is_uploaded_file return false because -> is_uploaded_file — Tells whether the file was uploaded via HTTP POST But the variable $_FILES['tmp_name'] on validate are empty, because file already downloaded, so i was suggest change this to !file_exists($value['tmp_name'])

sivaschenko · 2019-01-18T12:47:47Z

@Nazar65 I am not sure there is a difference between file_exists and is_uploaded_file in case when $value['tmp_name']. is_uploaded_file should work fine in case when tmp_name is set, so I would keep is_uploaded_file

Nazar65 · 2019-01-18T12:53:07Z

@sivaschenko yes, but function is_uploaded_file return false but file exist. this function check if file uploaded via HTTP POST action returns TRUE if the file named by filename was uploaded via HTTP POST but not checks if file exist.

Nazar65 · 2019-01-18T13:00:51Z

@sivaschenko
is_uploaded

file_exists

Nazar65 · 2019-01-18T13:04:04Z

@sivaschenko so basically to fix this issue we need just add the following condition

if (!empty($value['tmp_name']) && !file_exists($value['tmp_name'])) {
            return [__('"%1" is not a valid file.', $label)];
        }

Nazar65 · 2019-02-01T12:09:56Z

@orlangur or @sivaschenko any update on this ?

Zyles · 2019-02-06T11:56:43Z

What is the fix? I can't do any development because I can't change themes.

sivaschenko · 2019-02-06T18:53:40Z

Sorry for delay @Nazar65 the pull request is approved and will be further processed internally

magento-engcom-team · 2019-02-06T18:54:40Z

Hi @sivaschenko, thank you for the review.
ENGCOM-4152 has been created to process this Pull Request

ghost · 2019-02-25T10:48:17Z

Hi @Nazar65, 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.

…e programmatically #19988

sivaschenko · 2019-02-27T15:10:08Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-02-27T15:10:13Z

Hi @sivaschenko. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team added Area: Frontend Component: Ui Release Line: 2.3 labels Dec 27, 2018

Nazar65 added 2 commits December 27, 2018 12:43

Fix for issue 19983

6d5ee63

Fix save customer attribute image

188a3e6

orlangur self-assigned this Dec 27, 2018

orlangur requested changes Dec 27, 2018

View reviewed changes

Nazar65 mentioned this pull request Dec 28, 2018

Update file-uploader.js #19999

Closed

4 tasks

Nazar65 requested a review from sivaschenko January 16, 2019 13:43

sivaschenko requested changes Jan 16, 2019

View reviewed changes

sivaschenko self-assigned this Jan 16, 2019

sivaschenko added the Progress: needs update label Jan 16, 2019

add mage/adminhtml/tools for Base64

20fd589

sivaschenko requested changes Jan 18, 2019

View reviewed changes

app/code/Magento/Eav/Model/Attribute/Data/File.php Outdated Show resolved Hide resolved

Update app/code/Magento/Eav/Model/Attribute/Data/File.php

9ca8077

Co-Authored-By: Nazar65 <nazarn96@gmail.com>

ghost mentioned this pull request Jan 24, 2019

Image uploading not working on products on 2.3 #20570

Closed

Nazar65 added 2 commits January 26, 2019 21:44

Update File.php to resolve issue 19983

a051656

Update File.php

1372203

ghost mentioned this pull request Jan 31, 2019

magento 2.3 Design Configuration > Global page keep loading #19549

Closed

sivaschenko approved these changes Feb 6, 2019

View reviewed changes

sivaschenko removed the Progress: needs update label Feb 6, 2019

magento-engcom-team added Area: Lib/Frontend Component: Eav Component: Variable labels Feb 6, 2019

Nazar65 mentioned this pull request Feb 9, 2019

Customer Image Attribute #21103

Closed

magento-engcom-team added the Progress: accept label Feb 11, 2019

Merge branch '2.3-develop' into issue-19983

acc98ec

magento-engcom-team mentioned this pull request Feb 25, 2019

Can't work customer Image attribute programmatically #19983

Closed

magento-engcom-team merged commit acc98ec into magento:2.3-develop Feb 25, 2019

magento-engcom-team pushed a commit that referenced this pull request Feb 25, 2019

ENGCOM-4152: Fix for issue 19983 Can't upload customer Image attribut…

d5cc340

…e programmatically #19988

magento-engcom-team added the Award: bug fix label Feb 25, 2019

magento-engcom-team added this to the Release: 2.3.2 milestone Feb 25, 2019

Nazar65 mentioned this pull request Feb 25, 2019

[Backport]-issue-195196 Can't upload customer Image attribute programmatically #21437

Merged

4 tasks

Fix for issue 19983 Can't upload customer Image attribute programmatically #19988

Fix for issue 19983 Can't upload customer Image attribute programmatically #19988

Conversation

Nazar65 commented Dec 27, 2018

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Expected Result

Contribution checklist (*)

magento-engcom-team commented Dec 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nazar65 Dec 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nazar65 commented Dec 27, 2018

Nazar65 commented Jan 8, 2019

sivaschenko commented Jan 16, 2019

magento-engcom-team commented Jan 16, 2019

magento-engcom-team commented Jan 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaschenko commented Jan 16, 2019

Nazar65 commented Jan 17, 2019

Nazar65 commented Jan 17, 2019 • edited Loading

sivaschenko commented Jan 17, 2019

Nazar65 commented Jan 17, 2019

sivaschenko left a comment

Choose a reason for hiding this comment

Nazar65 commented Jan 18, 2019 • edited Loading

sivaschenko commented Jan 18, 2019

Nazar65 commented Jan 18, 2019

Nazar65 commented Jan 18, 2019

Nazar65 commented Jan 18, 2019

Nazar65 commented Feb 1, 2019

Zyles commented Feb 6, 2019

sivaschenko commented Feb 6, 2019

magento-engcom-team commented Feb 6, 2019

ghost commented Feb 25, 2019

sivaschenko commented Feb 27, 2019

magento-engcom-team commented Feb 27, 2019

Nazar65 Dec 27, 2018 •

edited

Loading

Nazar65 commented Jan 17, 2019 •

edited

Loading

Nazar65 commented Jan 18, 2019 •

edited

Loading