| Summary: | Add where parameter to sitemap.pl | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
| Component: | Command-line Utilities | Assignee: | Marcel de Rooy <m.de.rooy> |
| Status: | RESOLVED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | fridolin.somers, m.de.rooy, robin |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
23.11.00
|
|
| Circulation function: | |||
| Bug Depends on: | 33870 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 33871: Use Koha object in Sitemapper, add optional filter
Bug 33871: Test where parameter in Sitemapper.t Bug 33871: Add try catch in case where parameter is not valid SQL Bug 33871: Use Koha object in Sitemapper, add optional filter Bug 33871: Test where parameter in Sitemapper.t Bug 33871: Use Koha object in Sitemapper, add optional filter Bug 33871: Test where parameter in Sitemapper.t |
||
|
Description
Marcel de Rooy
2023-05-31 13:15:31 UTC
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 |