[Bug 24745] New: OPAC news block plugin should evaluate as false if there are no items
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Bug ID: 24745 Summary: OPAC news block plugin should evaluate as false if there are no items Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: oleonard@myacpl.org QA Contact: testopia@bugs.koha-community.org Blocks: 23796 In the OPAC we do this to get a news block for display: [% SET OpacHeader = KohaNews.get( location => "opacheader", lang => lang, library => branchcode, blocktitle => 0 ) %]
From this we're supposed to be able to check for the existence of [% OpacHeader %], but the way the data is returned from KohaNews.pm it evaluates as true.
Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23796 [Bug 23796] Convert OpacCustomSearch system preference to news block -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 --- Comment #1 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 99708 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=99708&action=edit Bug 24745: OPAC news block plugin should evaluate as false if there are no items This patch changes the way news is returned from GetNewsToDisplay in KohaNews.pm so that the template variable will evaluate as false instead of simply being an empty data structure. To reproduce the bug, remove your opacheader news entry and add this to OpacUserCSS: Go to the OPAC and you should see an empty blue-bordered box at the top of the page. To test the fix, apply the patch and reload the OPAC page. The box should disappear. Add content to the opacheader news item and confirm that it displays correctly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24746 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #2 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Hi Owen, can't reproduce. Could you please add the missing block of CSS in the test plan? The <div id="opacheader"> is not showing if there are now news (for opacheader) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 --- Comment #3 from Owen Leonard <oleonard@myacpl.org> --- Whoops, the "#" turned my CSS into a commented line in the commit message :( Also my test plan depended on Bug 24746 still existing! Now to verify the bug you have to look at the page source for a couple of empty <div>s in the header, right after '</div> <!-- /navbar -->' <div class="container-fluid"> <div class="row-fluid"> </div> </div> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #99708|0 |1 is obsolete| | --- Comment #4 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 100151 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100151&action=edit Bug 24745: OPAC news block plugin should evaluate as false if there are no items This patch changes the way news is returned from GetNewsToDisplay in KohaNews.pm so that the template variable will evaluate as false instead of simply being an empty data structure. To verify the bug, remove your opacheader news entry and view source on any page in the OPAC. You should find some empty markup right after '</div> <!-- /navbar -->' <div class="container-fluid"> <div class="row-fluid"> </div> </div> To test the fix, apply the patch and reload the OPAC page. The empty divs should be gone. Add content to the opacheader news item and confirm that it displays correctly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100151|0 |1 is obsolete| | --- Comment #5 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 100152 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=100152&action=edit Bug 24745: OPAC news block plugin should evaluate as false if there are no items This patch changes the way news is returned from GetNewsToDisplay in KohaNews.pm so that the template variable will evaluate as false instead of simply being an empty data structure. To verify the bug, remove your opacheader news entry and view source on any page in the OPAC. You should find some empty markup right after '</div> <!-- /navbar -->' <div class="container-fluid"> <div class="row-fluid"> </div> </div> To test the fix, apply the patch and reload the OPAC page. The empty divs should be gone. Add content to the opacheader news item and confirm that it displays correctly. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Divs gone, no errors. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + } else { + return 0; + } Wouldnt it be better to return undef or empty string in case of template use? If it is concatenated somewhere, we get a 0 in between ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 --- Comment #7 from Owen Leonard <oleonard@myacpl.org> --- (In reply to Marcel de Rooy from comment #6)
Wouldnt it be better to return undef or empty string in case of template use? If it is concatenated somewhere, we get a 0 in between ?
Returning undef triggered this error in the QA tool: "# Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s)." I don't think there is a scenario where this variable would be concatenated with something else, but if you think it's worth preparing for I would be happy to sign off on a follow-up. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 --- Comment #8 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Owen Leonard from comment #7)
(In reply to Marcel de Rooy from comment #6)
Wouldnt it be better to return undef or empty string in case of template use? If it is concatenated somewhere, we get a 0 in between ?
Returning undef triggered this error in the QA tool:
"# Subroutines::ProhibitExplicitReturnUndef: Got 1 violation(s)."
This has to do with syntax. You should not write "return undef" but you write "return;". In scalar context you return undef, in list context empty list. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 --- Comment #9 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 101558 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101558&action=edit Bug 24745: (follow-up) Correct perl syntax Subroutine should "return;" instead of "return 0;" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 --- Comment #10 from Owen Leonard <oleonard@myacpl.org> --- Thanks Marcel! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #100152|0 |1 is obsolete| | Attachment #101558|0 |1 is obsolete| | --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 101586 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101586&action=edit Bug 24745: OPAC news block plugin should evaluate as false if there are no items This patch changes the way news is returned from GetNewsToDisplay in KohaNews.pm so that the template variable will evaluate as false instead of simply being an empty data structure. To verify the bug, remove your opacheader news entry and view source on any page in the OPAC. You should find some empty markup right after '</div> <!-- /navbar -->' <div class="container-fluid"> <div class="row-fluid"> </div> </div> To test the fix, apply the patch and reload the OPAC page. The empty divs should be gone. Add content to the opacheader news item and confirm that it displays correctly. Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Divs gone, no errors. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 --- Comment #12 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 101587 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=101587&action=edit Bug 24745: (follow-up) Correct return statement in news plugin Subroutine should "return;" instead of "return 0;" We also prevent ProhibitExplicitReturnUndef by not returning "undef" explicitly. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Commit message edited. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |24223 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24223 [Bug 24223] Convert OpacNav system preference to news block -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24745 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #14 from Joy Nelson <joy@bywatersolutions.com> --- patches do not apply to 19.11.x not backported. please rebase if needed. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org