Objects with leadimage are no longer accepted in the carousel tile and SILENTLY ignored

[fredvd]

commit a4ca1c7 States that it adds back functionality, but breaks backwards compatibility by not accepting valid content types with a content lead image behavior. These worked fine until the check for image_size = obj.restrictedTraverse('@@images').getImageSize() was added and the carousel has no problem showing these.

@hvelarde @frapell , should we remove this check on populate_with_object, or is there a better way to 99% check for the availability of an image? Silently ignoring dropped content on a tile without any logging or user feedback is imho worse than having an item without image in the carousel that a user can remove again later.

@frapell : could you please add a descriptive entry to the HISTORY.txt if you "bring back functionality"? That would have saved me quite some time searching as for why my link items with leadimage behavior were not accepted anymore by carousels. :-/

[fredvd]

I checked how images are detected elsewhere in c.cover, PersistentCoverTile has a method _has_image_field(obj) which is called in collection and list tiles and does detect LeadImages. For the sake of conssistency, if we have to keep this check, can we use this one?

[hvelarde]

@runyaga @frapell we need your help on fixing this

[fredvd]

@hvelarde there's already a commit that fixes this in the reference. Sorry, should have updated the ticket. i think using the same method as other parts of c.cover closes this.

[frapell]

@fredvd @hvelarde Sorry, I was on vacation so I'm just seeing this. Let me explain what happened here.

Commit 8df37e0 was bugged, reading it again now, it doesn't seem to be, but I remember the object being added to the internal structure anyway, even if it didn't have an image. It seemed to be working because objects with no image were omitted from the carousel, but they were present in the internal structure... Since this was working like this, and no bugs were reported, I decided to remove the method altogether.

Giving it a bit more of thought, and reading the commit message that was specifically "not allowing objects without image to be added", I commited a4ca1c7 which effectively does not add an object if it doesn't have an image.

I didn't add a note to the HISTORY.txt because it wasn't really "a change" (since this was supposed to be working like this already), and there were no bugs reported either, so I thought "this is not really changing anything". I did leave a reference to the old commit, just in case.

Finally, @fredvd I haven't really tested what I'm about to say, but calling obj.restrictedTraverse('@@images').getImageSize() should use plone.app.imaging, and thus handle objects with images fine, no? (provided that a content type with image is implemented properly)

[hvelarde]

@frapell thanks, I left you a couple of comments about that commit.

[fredvd]

@hvelarde @frapell My apologies, I was wrong when I stated above that this issues was already fixed: I was confused with another Issue I did fix.

I created a branch for this issue: issue_473 where I replaced the try: except and traverse to @@images with a call to self._has_image_field(obj). The branch fails on CI though, I'll check because it does solve the issue in my projects. and _has_image_field is used by other parts in list tile as well.