At the moment, reports cannot be selected from the reports webservice by name, only by report ID. It would be very useful to be able to select a report by name.
Created attachment 10309 [details] [review] Bug 8256: Teach webservice to select reports by name Adds the ability to pass a hash to C4::Reports::Guided::get_saved_report which specifies a name or id to select the report.
Test plan: 1. Create a report (or choose an existing one), and mark it public 2. Run the report using the web service: [IntranetBaseURL]/cgi-bin/koha/svc/report?id=whatever 3. Confirm you get the expected results 4. Run the report by name using the web service: [IntranetBaseURL]/cgi-bin/koha/svc/report?name=[Report name] (keep spaces in the name) 5. Confirm you get the same results 6. Run the report using the public web service: [OPACBaseURL]/cgi-bin/koha/svc/report?id=whatever 7. Confirm you get the same results 8. Run the report by name using the public web service: [OPACBaseURL]/cgi-bin/koha/svc/report?name=[Report name] (keep spaces in the name) 9. Confirm you get the same results 10. Sign off
Hi Jared, I can't get this to work :( The id works nicely for OPAC and for staff, but the name does not work for me. http://localhost:8080/cgi-bin/koha/svc/report?name=Borrowers Can't call method "fetchall_arrayref" on an undefined value at /home/katrin/kohaclone/svc/report line 65. or http://localhost/cgi-bin/koha/svc/report?name=Borrowers Sorry this report is not public With id=2 both URLs work.
Created attachment 10314 [details] [review] Bug 8256: Teach webservice to select reports by name Adds the ability to pass a hash to C4::Reports::Guided::get_saved_report which specifies a name or id to select the report. Test plan: 1. Create a report (or choose an existing one), and mark it public 2. Run the report using the web service: [IntranetBaseURL]/cgi-bin/koha/svc/report?id=whatever 3. Confirm you get the expected results 4. Run the report by name using the web service: [IntranetBaseURL]/cgi-bin/koha/svc/report?name=[Report name] (keep spaces in the name) 5. Confirm you get the same results 6. Run the report using the public web service: [OPACBaseURL]/cgi-bin/koha/svc/report?id=whatever 7. Confirm you get the same results 8. Run the report by name using the public web service: [OPACBaseURL]/cgi-bin/koha/svc/report?name=[Report name] (keep spaces in the name) 9. Confirm you get the same results 10. Sign off
I'm not sure the changes I made will have fixed it, but could you please try again? At the very least, you shouldn't get that error about fetchall_arrayref not working.
Hi Jared, the new feature works fine with the new patch, but I found another big problem. After applying your patch I can not edit existing reports. All fields are empty on the edit screen. Another interesting find is a problem with authentication. I reached the URL for the report today, before I logged into the staff client. I got redirected to the login screen, which is fine. I logged in and got to see the report's results. But the URL changed from http://localhost:8080/cgi-bin/koha/svc/report?id=2 to http://localhost:8080/cgi-bin/koha/svc/report. I opened a new tab and tried again to reach the page with the first URL and got told my session has timed out. Not sure if this is a problem or not, but there is some difference between logging in from one of the webservice pages and logging in from the normal staff client URL.
[Sat Jun 16 11:22:15 2012] [error] [client 127.0.0.1] [Sat Jun 16 11:22:15 2012] report: "my" variable $report_id masks earlier declaration in same scope at /home/katrin/kohaclone/opac/svc/report line 36.
Created attachment 10318 [details] [review] Thanks, Katrin, for testing! Bug 8256: Teach webservice to select reports by name Adds the ability to pass a hash to C4::Reports::Guided::get_saved_report which specifies a name or id to select the report. Test plan: 1. Create a report (or choose an existing one), and mark it public 2. Run the report using the web service: [IntranetBaseURL]/cgi-bin/koha/svc/report?id=whatever 3. Confirm you get the expected results 4. Run the report by name using the web service: [IntranetBaseURL]/cgi-bin/koha/svc/report?name=[Report name] (keep spaces in the name) 5. Confirm you get the same results 6. Run the report using the public web service: [OPACBaseURL]/cgi-bin/koha/svc/report?id=whatever 7. Confirm you get the same results 8. Run the report by name using the public web service: [OPACBaseURL]/cgi-bin/koha/svc/report?name=[Report name] (keep spaces in the name) 9. Confirm you get the same results 10. Sign off
Created attachment 10320 [details] [review] [SIGNED-OFF] Bug 8256: Teach webservice to select reports by name Adds the ability to pass a hash to C4::Reports::Guided::get_saved_report which specifies a name or id to select the report. Test plan: 1. Create a report (or choose an existing one), and mark it public 2. Run the report using the web service: [IntranetBaseURL]/cgi-bin/koha/svc/report?id=whatever 3. Confirm you get the expected results 4. Run the report by name using the web service: [IntranetBaseURL]/cgi-bin/koha/svc/report?name=[Report name] (keep spaces in the name) 5. Confirm you get the same results 6. Run the report using the public web service: [OPACBaseURL]/cgi-bin/koha/svc/report?id=whatever 7. Confirm you get the same results 8. Run the report by name using the public web service: [OPACBaseURL]/cgi-bin/koha/svc/report?name=[Report name] (keep spaces in the name) 9. Confirm you get the same results 10. Sign off Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> - Adding, editing and deleting reports works - id parameter works - new name parameter works - public and non-public works
QA Comment: Works fine. Code looks good to me. Some minor points, just to be strict for this time :-) Routine get_saved_report: Removing the my ($id)=@_ by the more obscure $#_ and $_[0] stuff is no improvement. (Although I understand your motivation..) You are adding several cuddled else's. Like: } else. Not a best practice.. You reduce one line, but also readability. Please use two lines. Just wondering about the use of selector->{id}==0. Could you explain? svc/report line 53-59: First, thanks for commenting! What would be the cost of storing both in the cache BTW? Note that PBP recommends breaking the line before the = operator, and even before reaching the 78 coulmns limit (line 57). About the extended end-of-line comments: For readability, I would prefer here two comment lines starting under the if. (Don't know if that is a PBP or a personal opinion ;) No reasons to block this patch however. Just working on my own PBP knowledge.. Passed QA
This now works.