Follow-up on 33870
Created attachment 151879 [details] [review] Bug 33871: Use Koha object in Sitemapper, add optional filter Test plan: Run misc/cronjobs/sitemap.pl -where "biblionumber>X" (replace X by some clever value) Verify results by browsing sitemap files. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 151880 [details] [review] Bug 33871: Test where parameter in Sitemapper.t This makes it possible to remove the ugly global deletes at the start. Test plan: Run t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 153458 [details] [review] Bug 33871: Add try catch in case where parameter is not valid SQL Hi Marcel is this try catch a good idea or irrelevant and unecessary? Also, shoul we keep at least 1 test for ->run() (without params) in Sitemapper.t? The where param in Sitemapper->run is optional, I think should still test for ->run()
(In reply to Pedro Amorim from comment #3) > Created attachment 153458 [details] [review] [review] > Bug 33871: Add try catch in case where parameter is not valid SQL > > Hi Marcel is this try catch a good idea or irrelevant and unecessary? > > Also, shoul we keep at least 1 test for ->run() (without params) in > Sitemapper.t? > The where param in Sitemapper->run is optional, I think should still test > for ->run() Sounds good to me
> (In reply to Pedro Amorim from comment #3) > > Created attachment 153458 [details] [review] [review] [review] > > Bug 33871: Add try catch in case where parameter is not valid SQL > > > > Hi Marcel is this try catch a good idea or irrelevant and unecessary? On second thought I would not add it. Note that we are just writing to a file here while reading db records. The DBIx exception is not really expected. > > Also, shoul we keep at least 1 test for ->run() (without params) in > > Sitemapper.t? > > The where param in Sitemapper->run is optional, I think should still test > > for ->run() I will have a look.
> > > Also, shoul we keep at least 1 test for ->run() (without params) in > > > Sitemapper.t? > > > The where param in Sitemapper->run is optional, I think should still test > > > for ->run() Strictly speaking perhaps yes. But looking at it, I think that we should be pragmatic. We would be testing if an empty where, i.e. a Koha objects search with empty parameters {} would return all records. To make the test perform on a larger database without deleting all records, would again need a mock on ->process. Probably too much complexity for a very small gain to confirm something that we know and it is tested elsewhere too..
Created attachment 153534 [details] [review] Bug 33871: Use Koha object in Sitemapper, add optional filter Test plan: Run misc/cronjobs/sitemap.pl -where "biblionumber>X" (replace X by some clever value) Verify results by browsing sitemap files. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Created attachment 153535 [details] [review] Bug 33871: Test where parameter in Sitemapper.t This makes it possible to remove the ugly global deletes at the start. Test plan: Run t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
(In reply to Pedro Amorim from comment #8) > Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Thx !
Created attachment 155107 [details] [review] Bug 33871: Use Koha object in Sitemapper, add optional filter Test plan: Run misc/cronjobs/sitemap.pl -where "biblionumber>X" (replace X by some clever value) Verify results by browsing sitemap files. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 155108 [details] [review] Bug 33871: Test where parameter in Sitemapper.t This makes it possible to remove the ugly global deletes at the start. Test plan: Run t/db_dependent/Sitemapper.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 23.11. Nice work everyone, thanks!
Not backported to 23.05.x