Skip to content
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

EZP-29119: Display proper validation error message when non-image file uploaded in image FieldType #507

Merged
merged 1 commit into from Jun 18, 2018

Conversation

mikadamczyk
Copy link
Contributor

@mikadamczyk mikadamczyk commented Jun 13, 2018

Question Answer
Tickets https://jira.ez.no/browse/EZP-29119
Bug fix? yes
New feature? no
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

This PR is also fixing doubled error messages (backend).

screen shot 2018-06-13 at 3 32 22 pm

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review

@webhdx
Copy link
Contributor

webhdx commented Jun 14, 2018

By overriding form_label block in src/bundle/Resources/views/content/form_fields.html.twig you are overriding all labels across AdminUI. If there is a problem with Image FieldType you should fix it in the fieldtype without altering any other form elements.

{% set label = name|humanize %}
{%- endif -%}
{%- endif -%}
<{{ element|default('label') }}{% if label_attr %}{% with { attr: label_attr } %}{{ block('attributes') }}{% endwith %}{% endif %}>{{ translation_domain is same as(false) ? label : label|trans({}, translation_domain) }}{% block form_label_errors %}{{- form_errors(form) -}}{% endblock form_label_errors %}</{{ element|default('label') }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it exceeds 140 chars per line, isn't it?

@mikadamczyk mikadamczyk force-pushed the EZP-29119 branch 2 times, most recently from 089bb98 to 1befe3b Compare June 15, 2018 08:57
@micszo micszo self-assigned this Jun 15, 2018
Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Is it intentional that it works once (displaying the warning)? As in second attempt to publish/save creates image w/o file.

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

QA Approved on eZ Platform v2.1.1 with diff.

@micszo micszo removed their assignment Jun 15, 2018
@mikadamczyk mikadamczyk force-pushed the EZP-29119 branch 2 times, most recently from 0179a55 to 0179bcb Compare June 18, 2018 10:46
@lserwatka
Copy link
Member

@micszo re-test is needed here.

@mikadamczyk
Copy link
Contributor Author

This PR affect all field types, especially compound fields

@micszo
Copy link
Member

micszo commented Jun 18, 2018

On eZ Platform EE demo v2.2.0-beta1 with patch from this diff console error occurs when uploading PHP file in Image: "Uncaught TypeError: Cannot read property 'innerHTML' of null".
screen shot 2018-06-18 at 13 59 50
After trying to publish error occurs again:
screen shot 2018-06-18 at 14 02 00

@lserwatka
Copy link
Member

@micszo ready for re-test.

Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Works again.

@lserwatka lserwatka merged commit c7736de into ezsystems:1.1 Jun 18, 2018
@lserwatka
Copy link
Member

@mikadamczyk please merge it up

@lserwatka lserwatka mentioned this pull request Jun 19, 2018
1 task
mateuszdebinski pushed a commit that referenced this pull request Oct 25, 2022
Co-authored-by: katarzynazawada <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants