Summary: | ListSets doesn't return noSetHierarchy when appropriate | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi <tomascohen> |
Component: | Web services | Assignee: | Tomás Cohen Arazi <tomascohen> |
Status: | CLOSED FIXED | QA Contact: | Jonathan Druart <jonathan.druart> |
Severity: | normal | ||
Priority: | P5 - low | CC: | andrewfh, david, jonathan.druart, kyle, tomascohen, victor |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This fixes Koha's OAI-PMH server so that it returns the appropriate error code when no sets are defined.
|
Version(s) released in: |
22.05.00,21.11.01,21.05.08
|
Bug Depends on: | |||
Bug Blocks: | 21102 | ||
Attachments: |
Bug 29484: Regression tests
Bug 29484: Make ListSets return noSetHierarchy when no sets defined Bug 29484: Regression tests Bug 29484: Make ListSets return noSetHierarchy when no sets defined Bug 29484: Regression tests Bug 29484: Make ListSets return noSetHierarchy when no sets defined |
Description
Tomás Cohen Arazi
2021-11-15 12:50:51 UTC
Created attachment 127637 [details] [review] Bug 29484: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 127638 [details] [review] Bug 29484: Make ListSets return noSetHierarchy when no sets defined This simple patch makes our OAI-PMH server return the appropriate error code when no sets are defined. To test: 1. Apply the regression tests 2. Run: $ kshell k$ t/db_dependent/OAI/Server.t => FAIL: Tests fail. Boo! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! YAY! 5. Sign off :-D Bonus: you can check with the tool mentioned on the bug report, that the error is gone. No sets need to be defined. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 127995 [details] [review] Bug 29484: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 127996 [details] [review] Bug 29484: Make ListSets return noSetHierarchy when no sets defined This simple patch makes our OAI-PMH server return the appropriate error code when no sets are defined. To test: 1. Apply the regression tests 2. Run: $ kshell k$ t/db_dependent/OAI/Server.t => FAIL: Tests fail. Boo! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! YAY! 5. Sign off :-D Bonus: you can check with the tool mentioned on the bug report, that the error is gone. No sets need to be defined. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> I will amend the patch with - ) unless scalar @{$sets} > 0; + ) unless @$sets; $ cpanm HTTP::OAIPMH::Validator This fails for me Created attachment 128169 [details] [review] Bug 29484: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 128170 [details] [review] Bug 29484: Make ListSets return noSetHierarchy when no sets defined This simple patch makes our OAI-PMH server return the appropriate error code when no sets are defined. To test: 1. Apply the regression tests 2. Run: $ kshell k$ t/db_dependent/OAI/Server.t => FAIL: Tests fail. Boo! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! YAY! 5. Sign off :-D Bonus: you can check with the tool mentioned on the bug report, that the error is gone. No sets need to be defined. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> JD amended patch - ) unless scalar @{$sets} > 0; + ) unless @$sets; Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> (In reply to Jonathan Druart from comment #5) > I will amend the patch with > - ) unless scalar @{$sets} > 0; > + ) unless @$sets; Why? I prefer what I wrote and have been adding it all the time. If it makes more sense to do it your way, we should make sure it is in the guidelines. I still prefer to have the reader understand we are talking about the list size. (In reply to Tomás Cohen Arazi from comment #9) > (In reply to Jonathan Druart from comment #5) > > I will amend the patch with > > - ) unless scalar @{$sets} > 0; > > + ) unless @$sets; > > Why? I prefer what I wrote and have been adding it all the time. If it makes > more sense to do it your way, we should make sure it is in the guidelines. I > still prefer to have the reader understand we are talking about the list > size. ok, reverted. @$sets does exactly what you are describing, just less verbose. Nevermind. (In reply to Jonathan Druart from comment #10) > (In reply to Tomás Cohen Arazi from comment #9) > > (In reply to Jonathan Druart from comment #5) > > > I will amend the patch with > > > - ) unless scalar @{$sets} > 0; > > > + ) unless @$sets; > > > > Why? I prefer what I wrote and have been adding it all the time. If it makes > > more sense to do it your way, we should make sure it is in the guidelines. I > > still prefer to have the reader understand we are talking about the list > > size. > > ok, reverted. > > @$sets does exactly what you are describing, just less verbose. Nevermind. I like your approach, is more Perl-ish, and we should adopt it as a guideline. Pushed to master for 22.05, thanks to everybody involved 🦄 Pushed to 21.11.x for 21.11.01 Pushed to 21.05.x for 21.05.08 Not backported to oldoldstable (20.11.x). Feel free to ask if it's needed. |