Created attachment 86772 [details] [review] Bug 22544: Move get_opac_new to Koha namespace
Created attachment 86773 [details] [review] Bug 22544: Move get_opac_news to Koha namespace
Created attachment 86774 [details] [review] Bug 22544: Move del_opac_new to Koha namespace
Created attachment 86775 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace
Created attachment 86776 [details] [review] Bug 22544: Move add_opac_item to Koha namespace
Created attachment 86777 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace
Created attachment 86780 [details] [review] Bug 22544: Add tests
Created attachment 86781 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t
Hi, patch doesn't apply: Applying: Bug 22544: Move get_opac_new to Koha namespace Applying: Bug 22544: Move get_opac_news to Koha namespace Applying: Bug 22544: Move del_opac_new to Koha namespace Applying: Bug 22544: Move upd_opac_new to Koha namespace Applying: Bug 22544: Move add_opac_item to Koha namespace Applying: Bug 22544: Move GetNewsToDisplay to Koha namespace error: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt). error: could not build fake ancestor Patch failed at 0001 Bug 22544: Move GetNewsToDisplay to Koha namespace
Created attachment 90103 [details] [review] Bug 22544: Move get_opac_new to Koha namespace
Created attachment 90104 [details] [review] Bug 22544: Move get_opac_news to Koha namespace
Created attachment 90105 [details] [review] Bug 22544: Move del_opac_new to Koha namespace
Created attachment 90106 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace
Created attachment 90107 [details] [review] Bug 22544: Move add_opac_item to Koha namespace
Created attachment 90108 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace
Created attachment 90109 [details] [review] Bug 22544: Add tests
Created attachment 90110 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t
Rebased on master
Hi Josef, Everything looks good for the test plan, except that when I get to run the test I get this error: t/db_dependent/Koha/News.t .. 4/5 # Failed test 'Active and not expired news' # at t/db_dependent/Koha/News.t line 235. # got: '0' # expected: '4' # Looks like you planned 12 tests but ran 1. # Looks like you failed 1 test of 1 run. t/db_dependent/Koha/News.t .. 5/5 # Failed test '->search_for_display' # at t/db_dependent/Koha/News.t line 270. Can't call method "number" on an undefined value at t/db_dependent/Koha/News.t line 236. # Looks like your test exited with 255 just after 5. t/db_dependent/Koha/News.t .. Dubious, test returned 255 (wstat 65280, 0xff00) Failed 1/5 subtests Test Summary Report ------------------- t/db_dependent/Koha/News.t (Wstat: 65280 Tests: 5 Failed: 1) Failed test: 5 Non-zero exit status: 255 Files=1, Tests=5, 6 wallclock secs ( 0.03 usr 0.01 sys + 3.92 cusr 0.82 csys = 4.78 CPU) Result: FAIL Please let me know if there's something I may have missed here!
Created attachment 92005 [details] [review] Bug 22544: Move get_opac_new to Koha namespace
Created attachment 92006 [details] [review] Bug 22544: Move get_opac_news to Koha namespace
Created attachment 92007 [details] [review] Bug 22544: Move del_opac_new to Koha namespace
Created attachment 92008 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace
Created attachment 92009 [details] [review] Bug 22544: Move add_opac_item to Koha namespace
Created attachment 92010 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace
Created attachment 92011 [details] [review] Bug 22544: Add tests
Created attachment 92012 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t
Tested posting news items to librarian interface and the OPAC - can add news items without entering a position (before the patch was applied the news items just 'disappeared'). However, the news items don't display where they should - even after a flush_memcached and restart_all. Tested on koha-testing-docker against latest master. Tested on Ubuntu 18.04 with latest Firefox and Google Chrome.
Created attachment 95613 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace
Created attachment 95614 [details] [review] Bug 22544: Add tests
Created attachment 95615 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t
(In reply to David Nind from comment #28) > However, the news items don't display where they should - even after a > flush_memcached and restart_all. Can you detail? I see the news on the intranet mainpage. I did see a weird thing: "Lost connection to MySQL server during query" appeared in the logs and the news did not show up. Then I restart_all and everything went fine.
OPAC main page, I see: https://snipboard.io/CW4Bep.jpg It does not appear without the patchset.
Created attachment 95627 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 95628 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 95629 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 95630 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 95631 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 95632 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 95633 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 95634 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com>
Hi Jonathan. Figured out why the news items weren't displaying - I was picking today's date when adding, and my local machine time was ahead of koha-testing-docker. Changed date to an early one to resolve this. The only thing I noticed was the filter for 'Display location'. If you select the Librarian Interface there are no results. However, this was happening before the patches were applied. I can create a new bug for this. David
Patch no longer applies - please rebase!
Created attachment 99849 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99850 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99851 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99852 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99853 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99854 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99855 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99856 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 99857 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
(In reply to Katrin Fischer from comment #43) > Patch no longer applies - please rebase! Rebased on master. Thanks Katrin
Created attachment 104176 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 104177 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 104178 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 104179 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 104180 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 104181 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 104182 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 104183 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 104184 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Changed status backed to signed off - patch now applies.
I've looked at this a fair bit the past couple of days, and it certainly does move us from C4 to Koha.. and does an amount of tidying.. However, I'm torn as I feel we could afford to go a fair bit further. The field names are out right confusing and the logic is somewhat weird. Are you intending on treating this as a first step Josef or was this as far as you intended going? I think we've missed 20.05 now I'm afraid, but as we're moving lots of preferences over to this area perhaps it's time to focus on it more and further clean and improve both the db structure and logic early next release.?
(In reply to Martin Renvoize from comment #65) > I've looked at this a fair bit the past couple of days, and it certainly > does move us from C4 to Koha.. and does an amount of tidying.. > > However, I'm torn as I feel we could afford to go a fair bit further. The > field names are out right confusing and the logic is somewhat weird. > > Are you intending on treating this as a first step Josef or was this as far > as you intended going? > > I think we've missed 20.05 now I'm afraid, but as we're moving lots of > preferences over to this area perhaps it's time to focus on it more and > further clean and improve both the db structure and logic early next > release.? Setting to Failed QA to trigger a comment from Josef :)
Created attachment 111012 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 111013 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 111014 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 111015 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 111016 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 111017 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 111018 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 111019 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 111020 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 111021 [details] [review] Bug 22544: Update to current codebase
Created attachment 111022 [details] [review] Bug 22544: Update search_for_display to be able to take more types
(In reply to Martin Renvoize from comment #65) > I've looked at this a fair bit the past couple of days, and it certainly > does move us from C4 to Koha.. and does an amount of tidying.. > > However, I'm torn as I feel we could afford to go a fair bit further. The > field names are out right confusing and the logic is somewhat weird. > > Are you intending on treating this as a first step Josef or was this as far > as you intended going? > > I think we've missed 20.05 now I'm afraid, but as we're moving lots of > preferences over to this area perhaps it's time to focus on it more and > further clean and improve both the db structure and logic early next > release.? I rebased and fixed patchset on top of current master There was a change in db structure that removed timestamp column and added updated_on and published_on columns, which is definitely great move... The main problem now is the weird logic on 'lang' column... it is also used for location and language combinations... I would like to change this and add the new 'location' column - this could simplify the logic of searching for new and add more flexibility (we could differentiate between languages in news for slips based on patrons preferred language in the future... for example)
Created attachment 111023 [details] [review] Bug 22544: Fix filtering by location on tools -> news management
Anyone interested in getting this into 21.05? I would see it as a very first step to rebuild the "news" thing, and make it what it is now: an editor (and translation tool) for different Koha content.
Created attachment 115135 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115136 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115137 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115138 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115139 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115140 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115141 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115142 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 115143 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 115144 [details] [review] Bug 22544: Update to current codebase
Created attachment 115145 [details] [review] Bug 22544: Update search_for_display to be able to take more types
Created attachment 115146 [details] [review] Bug 22544: Fix filtering by location on tools -> news management
Created attachment 115218 [details] [review] Bug 22544: Fix NewsItem::author
I added bug 27451 for tracking lang field split. I do think we could try to test and merge this, before we can continue on following reports...
Hi Josef, trying to dive into this one here :) Could you please look at the comments before I continue testing? 1) QA Tools show one fail: FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt FAIL valid_template The type and lang parameters combination is not valid 2) search_for_display Trying to understand the code, I stumbled a bit on the POD for this one: +=head3 search_for_display +You can limit the results by type(lang) and library by optional params I am not sure I follow you here (type(lang)?) - maybe you changed the signature of the method after this was written? + +library_id should be valid branchcode of defined library + +type is one of this: +- slip - for ISSUESLIP notice +- koha - for intranet I think "koha" is not good for intranet, I think useing "intranet" or "staff" instead, would be better. +- opac - for online catalogue +- OpacNavRight - Right column in the online catalogue +- OpacLoginInstructions +- OpacMainUserBlock +- OpacCustomSearch +- opacheader +- opaccredits +lang is language code - it is used only when type is opac or OpacNavRight For future enh: I think once we have an "all languages" selection we should enable lang for the staff interface as well (I filed bug 27459) I guess the other prefs hadn't been migrated yet when this comment was written. 3) koha-news.tt + Librarian interface (koha) I'd like to see the (koha) removed here again - both the OPAC and staff interface are Koha. My suggestion would be to change "Librarian interface" to "Staff interface" as this is the standard terminology we have agreed to. 4) html_helpers.inc I like the use of the patron-title.inc here - maybe we should use it in koha-news.tt too? I am not sure about the hide_patron_infos_if_needed=1. I think we should only apply the NewsAuthorDisplay here, the difference being that these are staff users, not library patrons. - <td>[% opac_new.author_title | html %] [% opac_new.author_firstname | html %] [% opac_new.author_surname | html %]</td> + <td>[% IF ( opac_new.author) %][% INCLUDE 'patron-title.inc' patron=opac_new.author hide_patron_infos_if_needed=1 %][% END %]</td>
(In reply to Josef Moravec from comment #94) > I added bug 27451 for tracking lang field split. I do think we could try to > test and merge this, before we can continue on following reports... I am with you there, I think we could start with this one and will have an easier time adding more onto it later. @Martin - could you be a bit more specific about the things you were worrying about in comment#65? Would bug 27451 go in the right direction as a next step for you?
(In reply to Katrin Fischer from comment #95) > Hi Josef, > > trying to dive into this one here :) Could you please look at the comments > before I continue testing? > > 1) QA Tools show one fail: > > FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-main.tt > FAIL valid_template > The type and lang parameters combination is not valid I'll fix it > > 2) search_for_display > > Trying to understand the code, I stumbled a bit on the POD for this one: > > +=head3 search_for_display > +You can limit the results by type(lang) and library by optional params > > I am not sure I follow you here (type(lang)?) - maybe you changed the > signature of the method after this was written? > I will polish the pod in follow-up patch > + > +library_id should be valid branchcode of defined library > + > +type is one of this: > +- slip - for ISSUESLIP notice > +- koha - for intranet > > I think "koha" is not good for intranet, I think useing "intranet" or > "staff" instead, would be better. I will do that on (or after) bug 27451, 'koha' string is saved in BD, so it makes sense to change it in DB not only in UI (in it would be easier) > > +- opac - for online catalogue > +- OpacNavRight - Right column in the online catalogue > +- OpacLoginInstructions > +- OpacMainUserBlock > +- OpacCustomSearch > +- opacheader > +- opaccredits > > +lang is language code - it is used only when type is opac or OpacNavRight > > For future enh: I think once we have an "all languages" selection we should > enable lang for the staff interface as well (I filed bug 27459) Absolutely agree > > I guess the other prefs hadn't been migrated yet when this comment was > written. True, I'll fix pod > > 3) koha-news.tt > > + Librarian interface (koha) > > I'd like to see the (koha) removed here again - both the OPAC and staff > interface are Koha. My suggestion would be to change "Librarian interface" > to "Staff interface" as this is the standard terminology we have agreed to. Will do after/on bug 27451 > > 4) html_helpers.inc > > I like the use of the patron-title.inc here - maybe we should use it in > koha-news.tt too? > I am not sure about the hide_patron_infos_if_needed=1. I think we should > only apply the NewsAuthorDisplay here, the difference being that these are > staff users, not library patrons. > > - <td>[% opac_new.author_title | html %] [% > opac_new.author_firstname | html %] [% opac_new.author_surname | html %]</td> > + <td>[% IF ( opac_new.author) %][% INCLUDE > 'patron-title.inc' patron=opac_new.author hide_patron_infos_if_needed=1 %][% > END %]</td> Does make sense, I'll change it
Created attachment 115296 [details] [review] Bug 22544: Clarify documentation and change param 'type' to 'location'
Created attachment 115297 [details] [review] Bug 22544: Enhance exception description
Created attachment 115298 [details] [review] Bug 22544: Add default language value to KohaNews template plugin
Created attachment 115299 [details] [review] Bug 22544: Use template plugin for news author display
I think it is ready for next round
(In reply to Jonathan Druart from comment #33) > OPAC main page, I see: https://snipboard.io/CW4Bep.jpg > > It does not appear without the patchset. This is still valid. Worst, the header search is missing: https://snipboard.io/P4XI0L.jpg
1. cf last comment 2. - $_->{'timestamp'} = $_->{'newdate'}; you are loosing the ability to use "timestamp" in the the letter 3. + my $all = $n->unblessed_all_relateds; you should not use unblessed_all_relateds. Here we need the attributes of the news and the patron's info. We could built it easily before passing it to the letter. 4. + $search_params->{published_on} = { '<=' => \'NOW()' }; + $search_params->{-or} = [ expirationdate => { '>=' => \'NOW()' }, use dt_from_string instead of NOW 5. sub author { my ( $self ) = @_; - return Koha::Patron->_new_from_dbic($self->_result->borrowernumber); + my $author_rs = $self->_result->borrowernumber; + return unless $author_rs; + return Koha::Patron->_new_from_dbic( $author_rs ); } This is actually a bugfix that could go on its own bug report.
Created attachment 116933 [details] [review] Bug 22544: TT plugin should return hashref only if news exist It restore the header and fix the RSS feeds (see comment 103)
Created attachment 116934 [details] [review] Bug 22544: Fix OpacLoginInstructions Moved recently
Created attachment 116935 [details] [review] Bug 22544: Restore newdate and timestamp for slip notices
(In reply to Jonathan Druart from comment #104) > 1. cf last comment Done. > 2. - $_->{'timestamp'} = $_->{'newdate'}; > you are loosing the ability to use "timestamp" in the the letter Done. > 3. + my $all = $n->unblessed_all_relateds; > you should not use unblessed_all_relateds. > Here we need the attributes of the news and the patron's info. We could > built it easily before passing it to the letter. Done. > 4. > + $search_params->{published_on} = { '<=' => \'NOW()' }; > + $search_params->{-or} = [ expirationdate => { '>=' => \'NOW()' }, > > use dt_from_string instead of NOW Not done, this was not valid, we can use NOW. > 5. > sub author { > my ( $self ) = @_; > - return Koha::Patron->_new_from_dbic($self->_result->borrowernumber); > + my $author_rs = $self->_result->borrowernumber; > + return unless $author_rs; > + return Koha::Patron->_new_from_dbic( $author_rs ); > } > > This is actually a bugfix that could go on its own bug report. Done, see bug 27714.
Created attachment 117479 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117480 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117481 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117482 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117483 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117484 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117485 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117486 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117487 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 117488 [details] [review] Bug 22544: Update to current codebase
Created attachment 117489 [details] [review] Bug 22544: Update search_for_display to be able to take more types
Created attachment 117490 [details] [review] Bug 22544: Fix filtering by location on tools -> news management
Created attachment 117491 [details] [review] Bug 22544: Clarify documentation and change param 'type' to 'location'
Created attachment 117492 [details] [review] Bug 22544: Enhance exception description
Created attachment 117493 [details] [review] Bug 22544: Add default language value to KohaNews template plugin
Created attachment 117494 [details] [review] Bug 22544: Use template plugin for news author display
Created attachment 117495 [details] [review] Bug 22544: TT plugin should return hashref only if news exist It restore the header and fix the RSS feeds (see comment 103)
Created attachment 117496 [details] [review] Bug 22544: Fix OpacLoginInstructions Moved recently
Created attachment 117497 [details] [review] Bug 22544: Restore newdate and timestamp for slip notices
Sorry.. needs another rebase :(
Created attachment 118054 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 118055 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 118056 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 118057 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 118058 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 118059 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com>
Created attachment 118060 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 118061 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 118062 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 118063 [details] [review] Bug 22544: Update to current codebase
Created attachment 118064 [details] [review] Bug 22544: Update search_for_display to be able to take more types
Created attachment 118065 [details] [review] Bug 22544: Fix filtering by location on tools -> news management
Created attachment 118066 [details] [review] Bug 22544: Clarify documentation and change param 'type' to 'location'
Created attachment 118067 [details] [review] Bug 22544: Enhance exception description
Created attachment 118068 [details] [review] Bug 22544: Add default language value to KohaNews template plugin
Created attachment 118069 [details] [review] Bug 22544: Use template plugin for news author display
Created attachment 118070 [details] [review] Bug 22544: TT plugin should return hashref only if news exist It restore the header and fix the RSS feeds (see comment 103)
Created attachment 118071 [details] [review] Bug 22544: Fix OpacLoginInstructions Moved recently
Created attachment 118072 [details] [review] Bug 22544: Restore newdate and timestamp for slip notices
Created attachment 118073 [details] [review] Bug 22544: fix count call - to squash
Created attachment 118087 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118088 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118089 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118090 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118091 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118092 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118093 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118094 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118095 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118096 [details] [review] Bug 22544: Update to current codebase Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118097 [details] [review] Bug 22544: Update search_for_display to be able to take more types Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118098 [details] [review] Bug 22544: Fix filtering by location on tools -> news management Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118099 [details] [review] Bug 22544: Clarify documentation and change param 'type' to 'location' Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118100 [details] [review] Bug 22544: Enhance exception description Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118101 [details] [review] Bug 22544: Add default language value to KohaNews template plugin Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118102 [details] [review] Bug 22544: Use template plugin for news author display Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118103 [details] [review] Bug 22544: TT plugin should return hashref only if news exist It restore the header and fix the RSS feeds (see comment 103) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118104 [details] [review] Bug 22544: Fix OpacLoginInstructions Moved recently Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118105 [details] [review] Bug 22544: Restore newdate and timestamp for slip notices Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 118106 [details] [review] Bug 22544: fix count call - to squash Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This all appears to work as I expect.. I've tried a number of different blocks in a few different translations. I think it's a good start... signing off
Can we please have a rebase? Apply? [(y)es, (n)o, (i)nteractive] < Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 22544: Move get_opac_new to Koha namespace Applying: Bug 22544: Move get_opac_news to Koha namespace Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/tools/koha-news.tt
Created attachment 120377 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120378 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120379 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120380 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120381 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120382 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120383 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120384 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120385 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120386 [details] [review] Bug 22544: Update to current codebase Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120387 [details] [review] Bug 22544: Update search_for_display to be able to take more types Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120388 [details] [review] Bug 22544: Fix filtering by location on tools -> news management Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120389 [details] [review] Bug 22544: Clarify documentation and change param 'type' to 'location' Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120390 [details] [review] Bug 22544: Enhance exception description Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120391 [details] [review] Bug 22544: Add default language value to KohaNews template plugin Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120392 [details] [review] Bug 22544: Use template plugin for news author display Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120393 [details] [review] Bug 22544: TT plugin should return hashref only if news exist It restore the header and fix the RSS feeds (see comment 103) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120394 [details] [review] Bug 22544: Fix OpacLoginInstructions Moved recently Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120395 [details] [review] Bug 22544: Restore newdate and timestamp for slip notices Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 120396 [details] [review] Bug 22544: fix count call - to squash Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
QA: Looking here
+ <td data-order="[% opac_new.timestamp | html %]">[% opac_new.timestamp | $KohaDates %]</td> We removed timestamp from opac_news on bug 21066. Familiar author?
Subject: [PATCH 18/20] Bug 22544: Fix OpacLoginInstructions What does this patch actually fix ? - [% IF ( OpacLoginInstructions ) %] + [% IF OpacLoginInstructions %]
+ my $published_on_dt = output_pref({ dt => dt_from_string( $all->{pubished_on} ), dateonly => 1 }); Will amend the patch now.
Created attachment 122396 [details] [review] Bug 22544: Move get_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122397 [details] [review] Bug 22544: Move get_opac_news to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122398 [details] [review] Bug 22544: Move del_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122399 [details] [review] Bug 22544: Move upd_opac_new to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122400 [details] [review] Bug 22544: Move add_opac_item to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122401 [details] [review] Bug 22544: Move GetNewsToDisplay to Koha namespace Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122402 [details] [review] Bug 22544: Add tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122403 [details] [review] Bug 22544: Refactor searching of news items Test plan: 1) Go to tools and define some news 2) Try different parameters, try to edit new items, and delete some 3) Go to all places where news are presented and ensure that there are the right ones shown: opac main page - based on language opac righ column (formerly syspref OpacNavRight) - based on language opac news rss feed circulation slip (not quick slip) intranet main page 4) run tests: prove t/db_dependent/Koha/News.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122404 [details] [review] Bug 22544: (QA follow-up) Fix spelling Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122405 [details] [review] Bug 22544: Update to current codebase Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122406 [details] [review] Bug 22544: Update search_for_display to be able to take more types Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122407 [details] [review] Bug 22544: Fix filtering by location on tools -> news management Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122408 [details] [review] Bug 22544: Clarify documentation and change param 'type' to 'location' Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122409 [details] [review] Bug 22544: Enhance exception description Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122410 [details] [review] Bug 22544: Add default language value to KohaNews template plugin Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122411 [details] [review] Bug 22544: Use template plugin for news author display Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122412 [details] [review] Bug 22544: TT plugin should return hashref only if news exist It restore the header and fix the RSS feeds (see comment 103) Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122413 [details] [review] Bug 22544: Fix OpacLoginInstructions Moved recently Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 122414 [details] [review] Bug 22544: Restore newdate and timestamp for slip notices Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Fixed typo pubished_on
Created attachment 122415 [details] [review] Bug 22544: fix count call - to squash Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Great work !
(In reply to Marcel de Rooy from comment #193) > Subject: [PATCH 18/20] Bug 22544: Fix OpacLoginInstructions > > What does this patch actually fix ? > > - [% IF ( OpacLoginInstructions ) %] > + [% IF OpacLoginInstructions %] Nothing, it should be skipped. It has been fixed in the meanwhile by commit c45d15e231efd11d5f19f3a1186013daef19d300 Bug 28193: (follow-up) Correct OpacLoginInstructions in login modal
Thx, Marcel. Quite happy to see this one move forward.
Pushed to master for 21.11, thanks to everybody involved!
Enhancement not backported to 21.05
I think this patch set may have inadvertently broke logging of news items which was add in Bug 26205.