Fix EZP-23207: image alt text not modified if file is not set.

[joaoinacio]

JIRA: https://jira.ez.no/browse/EZP-23207

This fixes a regression in EZP-22615 , that causes an image attribute's alt text to not be updated if the file is not modified as well.

[nunofcnoliveira]

My tests indicate the PR resolves this issue, but also re-introduces https://jira.ez.no/browse/EZP-22615 bug.

[andrerom]

Sure that it should return true if there is no file?

[joaoinacio]

Modifying alternative text should make it so, I believe.
Actually, should the file always exist (if it's not being uploaded)?

[joaoinacio]

The problem is that ALL custom actions are considered as 'store' actions by default, and 'delete_image' is no exception...

Needs further investigation.

[joaoinacio]

Updated to allow image removal yet again.
Edit:
Actually, it should be possible to simply revert fix for EZP-22615 and apply the one commit above.

[andrerom]

Actually, it should be possible to simply revert fix for EZP-22615 and apply the one commit above.

You mean 4fa0c1a? If so why don't we do that (as first commit to this branch) and try to minimize changes done here(as second commit to this branch).

[joaoinacio]

Done, hope this is OK. (not sure about messages either)...

[andrerom]

ok, but did you do a native git revert here? (preferable)

[joaoinacio]

Yes, I used git revert

[glye]

Confirmed that the bug is introduced by EZP-22615 and fixed by the PR. However, image removal doesn't work in my test.

[andrerom]

@joaoinacio same issue for you when testing last iteration of patch?

[joaoinacio]

Yes, i'm doing more tests now and unable to remove the file for some reason, even though the logic is pretty much the same...

[glye]

Just noticed that as I click "Remove image", the "Store draft" button appears in the top bar, and immediately disappears again. The page url doesn't change, but it looks like a quick redirect.

[joaoinacio]

PR Updated, my tests seem OK now.
A bit OT, but on removal of the image file the alt text is also cleared (which didn't happen before) - is this OK?

[glye]

If you mean after 19f829c, I tested that and no change on my end, can't delete images. The error.log says:
[ Jul 29 2014 16:51:13 ] [10.127.40.35] eZContentObjectAttribute:
Undefined attribute 'alternative_text', cannot set
[ Jul 29 2014 16:51:13 ] [10.127.40.35] eZContentObjectAttribute:
Undefined attribute 'content', cannot set
To expand on this: The image does disappear from the content/edit page, though the line with filename and mime type remains. On the content/view page I see no change, image is still visible.

[joaoinacio]

Final, tested, working, fix. promise.
The problem ultimately comes from the data_text (from the image xml data in ImageAliasHandler) no longer being set on the attribute itself when removing.
Ref: b072583#diff-d76ed5a1e5d4d3e5e2414d99d48c6fdeL834 .
This forcefully calls store() to "push" those changes.

[andrerom]

review ping @bdunogier @yannickroger

[glye]

My test 1: Click remove image, the image itself is gone but the "current image" line with filename and mimetype remains. Publish, and the image is gone.
Test 2: Click remove image, the image itself is gone but the "current image" line with filename and mimetype remains. Click remove image again, and the line is gone. Publish, all good.

So there seems to be a minor remaining issue with the refreshing of the "current image" line.
Edit: Not sure if this is really related, or a different issue.

[joaoinacio]

@glye: double-checked in different installations for 5.3 and master, and everything seems to be working OK.
Edit:
local phpunit tests are also OK besides date/time issues, no idea why the travis failure.

[glye]

@joaoinacio I trust you then, my test env. may not be 100% correct. The Travis fail is afaik an ongoing issue and nothing to worry about.
Edit: Confirmed, what I describe happens with and without your patch, so it's unrelated.
+1

review ping @bdunogier @yannickroger

[lolautruche]

Looks OK +1