MBS-13613: Show secondary art if no poster is available#3731
MBS-13613: Show secondary art if no poster is available#3731IvanPisquiy06 wants to merge 3 commits intometabrainz:masterfrom
Conversation
Ticket MBS-13613: Currently only if an image is a Poster can be shown in SideBar of an event. This makes it that if no Poster available, a Flyer or next a Banner can be shown.
mwiencek
left a comment
There was a problem hiding this comment.
Thank you for the PR! You should be able to implement this more easily (without additional database queries) by updating $extra_conditions in Data::Role::Art::find_by_entity based on $entity_type. Plus the ORDER BY, so that it sorts things in "poster, flyer, banner" order.
Ticket MBS-13613: Past method required db queries. It now filters directly on find_by_entity and sorts it following the main hierarchy.
|
Thanks for the feedback @mwiencek ! I changed the method following your advice. It was easier as you pointed out, appreciate it! |
| } $art_archive_model->art_archive_type_booleans); | ||
|
|
||
| my $extra_conditions = ''; | ||
| my $order_by = "ORDER BY $art_schema.index_listing.ordering"; |
There was a problem hiding this comment.
You're not actually using $order_by anywhere in the query, so there's no sorting right now. (You can also remove ORDER BY from this string and leave it in the query, since it's always present.)
| $extra_conditions .= " AND (is_front = TRUE OR 'Flyer' = ANY($art_schema.index_listing.types) OR 'Banner' = ANY($art_schema.index_listing.types))"; | ||
|
|
||
| $order_by = "ORDER BY is_front DESC, 'Flyer' = ANY($art_schema.index_listing.types) DESC, 'Banner' = ANY($art_schema.index_listing.types) DESC, $art_schema.index_listing.ordering"; | ||
| } else { |
There was a problem hiding this comment.
We try to keep lines under 80 chars. (A lot of existing code does go over that limit, but 193 chars is a bit too excessive.)
| $extra_conditions .= " AND (is_front = TRUE OR 'Flyer' = ANY($art_schema.index_listing.types) OR 'Banner' = ANY($art_schema.index_listing.types))"; | |
| $order_by = "ORDER BY is_front DESC, 'Flyer' = ANY($art_schema.index_listing.types) DESC, 'Banner' = ANY($art_schema.index_listing.types) DESC, $art_schema.index_listing.ordering"; | |
| } else { | |
| my $types_column = "$art_schema.index_listing.types"; | |
| $extra_conditions .= 'AND (' . | |
| 'is_front = TRUE ' . | |
| "OR 'Flyer' = ANY($types_column) " . | |
| "OR 'Banner' = ANY($types_column)" . | |
| ')'; | |
| $order_by = 'is_front DESC, ' . | |
| "'Flyer' = ANY($types_column) DESC, " . | |
| "'Banner' = ANY($types_column) DESC, " . | |
| $order_by; | |
| } else { |
| SQL | ||
| } | ||
|
|
||
| sub has_artwork_type { |
| {eventArtPresence === 'present' ? ( | ||
| <> | ||
| {l('No poster available.')} | ||
| {l('No main event art available.')} |
There was a problem hiding this comment.
Instead of "main," maybe we ought to stick with "front," since that's the prevalent terminology.
Ticket MBS-13613: The sub has_artwork_type is eliminated and find_by_entity code is cleaned for better organized and cleaner code
|
You are right @mwiencek , now that you pointed it out it looks not that well organized and makes the code more readable. Thanks for the tips on making it cleaner. Will definitely apply this in the future. |
Problem
Fixes MBS-13613.
Currently, when an event lacks a
Frontartwork (Poster), the sidebar displays a generic "No poster available" placeholder, even if there areFlyerorBannerimages associated with the event.Solution
Implemented a fallback logic natively in the database query to display a Flyer (priority 1) or a Banner (priority 2) when the Front image is missing.
Data::Role::Art::find_by_entityto dynamically update$extra_conditionsand theORDER BYclause when querying for anevententity with$opts{is_front}.Front(Poster),Flyer, orBannerusingANY(types)and sorts the results by priority (ORDER BY is_front DESC, 'Flyer' = ANY(...) DESC, 'Banner' = ANY(...) DESC, ordering).MusicBrainz::Server::Controller::Eventclean.Testing
Tested locally on event pages by injecting mock event art records via SQL to verify the new
ORDER BYlogic:Documenting
N/A
Further action