-
-
Notifications
You must be signed in to change notification settings - Fork 437
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
Fixed faulty media gallery label/description management for mutistore projects #2481
Conversation
Could somebody test this? It's a proper bug we're having since the beginning of time. |
There are a lot of check failures in the "Files changed' section (at the end). Could you please investigate if they are real warnings? |
I'll check it out asap! |
actually those notice are in the "Unchanged files with check annotations", those files weren't changed in this PR so I wouldn't do anything about those notices. |
@azambon I'd love your comment on this |
Sory for the massive delay, I completely forgot about this! I did try to reproduce the same scenario that I described in the issue, but I don't think that this PR actually solves it. So, unless I'm missing something, this does not seem to fix issue #685 . At least not yet. As I see it, there are a couple of things left to solve:
|
Hi @azambon and thanks for the test,
|
@azambon I tested the frotend and the image labels seem to be correct to me, both in default and the single store view, the labels changes and are the same I see in the backend. This is tested on RWD theme. |
I performed the scenario that I described in the issue once more, taking some screenshots. As you can see, when I uncheck the new "use default" checkbox that you added in order to set the label CCC for the non-default store view, the data is saved correctly in the This is with a fresh installation of OpenMage with the default theme, no customizations at all and all caches disabled. |
sorry for the stupid question, did you reindex/recache? cause I checked it yesterday and I see the right things in frontend, if I didn't understand something wrong :-\ |
All caches are disabled and I did reindex (even though reindexing should not affect the values in the catalog_product_entity_* tables, only in the catalog_product_flat_* tables, if flat tables are enabled). |
I've flat disabled, just in case |
app/design/adminhtml/default/default/template/catalog/product/helper/gallery.phtml
Outdated
Show resolved
Hide resolved
i've fixed the reported issues |
* fix tpl media gallery * fix save eav attribute labels * fix use label default * remove unecessary var * clean --------- Co-authored-by: Fabrizio Balliano <[email protected]>
I've integrated @empiricompany latest patch to this PR and retested it, it seems to be working correctly to me. |
* fix tpl media gallery * fix save eav attribute labels * fix use label default * remove unecessary var * clean * fix use default checked * fix use default --------- Co-authored-by: Fabrizio Balliano <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested, seems working.
From #685, this does not work: "In the admin add another image to the product with the label DDD in the default store view. Mark it as the default base image, small image and thumbnail and set its label to DDD" I must save again the product at store view level to have the right label.
Another thing, perhaps already the case, not sure, when I upload a new image, if I select it as default images, when I save the product, I must select again it as default images and save again, else the selection is lost.
true, this needs to be addressed:
|
I've fixed the last remaining point, it was only 1 line that was removed (and now reintroduced). Since there were already 2 gray check I'll merge this, it's an important bugfix too long overdue (and a huge amount of work to develop it). |
We're talking about the "images tab" of the product management page:
The complete problem explanation and testing scenario is explained in issue #685
It wasn't easy to fix, I worked on it quite some hours. Hope I didn't miss anything.
Fixed Issues