Bug 7249 - Report webservices
Summary: Report webservices
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: 3.10
Hardware: All All
: P3 enhancement (vote)
Assignee: Galen Charlton
QA Contact: Ian Walls
URL:
Keywords:
Depends on: 7248 8128
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-20 23:09 UTC by Chris Hall
Modified: 2013-12-05 20:04 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
git format patch adding report webservice, caching support, and public visibility setting (11.15 KB, patch)
2011-11-20 23:09 UTC, Chris Hall
Details | Diff | Splinter Review
Fixed bugs in previous patch, added better user input and feedback (36.89 KB, patch)
2011-11-21 02:45 UTC, Chris Hall
Details | Diff | Splinter Review
Bug 7249 : Allow reports to be called through a restful interface (29.48 KB, patch)
2011-11-21 03:10 UTC, Chris Cormack
Details | Diff | Splinter Review
Patch adds missing kohastructure entry (877 bytes, patch)
2011-11-28 21:23 UTC, Chris Hall
Details | Diff | Splinter Review
New interface for sql reports (19.89 KB, image/png)
2011-12-14 07:24 UTC, Katrin Fischer
Details
[SIGNED-OFF] Bug 7249 : Allow reports to be called through a restful interface (29.57 KB, patch)
2011-12-14 07:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7249: added saved_sql fields to kohastructure (1.96 KB, patch)
2011-12-14 07:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Changes true/false to yes/no (4.66 KB, patch)
2011-12-15 04:23 UTC, Chris Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7249: Small useability fix as per request (4.88 KB, patch)
2011-12-15 05:47 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7249 : Allow reports to be called through a restful interface (30.99 KB, patch)
2012-04-29 14:11 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 7249 : Allow reports to be called through a restful interface (31.24 KB, patch)
2012-04-29 19:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7249: Syspref to control number of rows in web service results (4.56 KB, patch)
2012-04-29 19:44 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7249: Syspref to control number of rows in web service results (4.59 KB, patch)
2012-04-29 20:01 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Hall 2011-11-20 23:09:34 UTC Comment hidden (obsolete)
Comment 1 Chris Hall 2011-11-21 02:45:55 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2011-11-21 02:54:32 UTC
Hi Chris,

Here's what I get

Commit Body is:
--------------------------
Starting work on webservices
--------------------------
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Starting work on webservices
Commit Body is:
--------------------------
Working on caching for services
--------------------------
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Working on caching for services
Commit Body is:
--------------------------
Added caching to intranet report webservices, added cache expiry and public options to reports
--------------------------
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Added caching to intranet report webservices, added cache expiry and public options to reports
fatal: sha1 information is lacking or useless (installer/data/mysql/updatedatabase.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0003 Added caching to intranet report webservices, added cache expiry and public options to reports
Comment 3 Chris Cormack 2011-11-21 02:56:16 UTC
Ahh my fault, this one won't apply with out 7248

Sorry, I should read better.
Comment 4 Chris Cormack 2011-11-21 03:10:30 UTC
Signed off, but still needs a sign off outside catalyst
Comment 5 Chris Cormack 2011-11-21 03:10:45 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2011-11-21 06:13:56 UTC
Hi Chris C./Chris H. - can you please add a description of what this is supposed to do and how to test it? Thx!
Comment 7 Chris Hall 2011-11-21 19:54:42 UTC
Hey Katrin

This allows you to visit the following urls:
opac:
 http://localhost/cgi-bin/koha/svc/report?id=1
intranet:
 http://localhost:8080/cgi-bin/koha/svc/report?id=1

(replace localhost and localhost:8080 with the appropriate URLs)

To fetch JSON output form previously created reports (in the above cases you are viewing report 1).
The report is only visible via the opac interface if it has been explicitly set to public (you can do this when creating a report or from the use saved report -> edit page).
Comment 8 Katrin Fischer 2011-11-28 05:24:48 UTC
I was looking at the patch - I think it's missing the update to kohastructure. sql to reflect the changes made by the updatedatabase?
Comment 9 Chris Hall 2011-11-28 21:23:27 UTC Comment hidden (obsolete)
Comment 10 Katrin Fischer 2011-11-30 06:40:02 UTC
Hi Chris, 
I am sorry, the first patch does not apply:
git am -is3 /tmp/Bug-7249--Allow-reports-to-be-called-through-a-res-aUdhoC.patch
Apply? [y]es/[n]o/[e]dit/[v]iew patch/[a]ccept all y
Applying: Bug 7249 : Allow reports to be called through a restful interface
fatal: sha1 information is lacking or useless (installer/data/mysql/updatedatabase.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 7249 : Allow reports to be called through a restful interface

Can you check what's going wrong here?
Comment 11 Chris Hall 2011-12-08 21:18:27 UTC
Hey Katrin

This patch is dependent on 7248 being applied first.

On kc/master I am able to apply the 7248 patch and then this one without any errors, does this work for you?

Thanks,
~Chris
Comment 12 Katrin Fischer 2011-12-14 07:24:08 UTC
Created attachment 6773 [details]
New interface for sql reports

Small enhancement request: Use yes/no instead of false/true for the pull down.
Comment 13 Katrin Fischer 2011-12-14 07:46:08 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2011-12-14 07:46:16 UTC Comment hidden (obsolete)
Comment 15 Chris Hall 2011-12-15 04:23:25 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2011-12-15 05:42:49 UTC
Hi Chris, small thing: don't forget to put the bug number in the subject line of your patches. I will fix it :)
Comment 17 Katrin Fischer 2011-12-15 05:47:11 UTC Comment hidden (obsolete)
Comment 18 Paul Poulain 2012-02-01 13:24:29 UTC
QA question: if i'm understanding correctly, the cache duration is something meaningfull only if you have memcache set. I haven't tested the patch, but does the field disappear from the screen cache is OFF ?
Comment 19 Paul Poulain 2012-02-10 18:20:19 UTC
Seems this patch does not apply anymore (and the message look unusual to me)

fatal: sha1 information is lacking or useless (installer/data/mysql/updatedatabase.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 7249 : Allow reports to be called through a restful interface
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Comment 20 Paul Poulain 2012-02-10 18:23:07 UTC
my wrong: this patch must be applied AFTER 7248, back to signed off & going to QA 7248
Comment 21 Paul Poulain 2012-02-21 12:47:33 UTC
This patch won't apply anymore because the bug 7248 won't be applied because bug 6193 has been applied after discussion on how to handle caching in Koha: we do that at apache configuration level.

So, either update this patch to use
eval {
    if (C4::Context->ismemcached) {
        require Memoize::Memcached;
        import Memoize::Memcached qw(memoize_memcached);

        memoize_memcached('getTranslatedLanguages', memcached => C4::Context->memcached);
    }
};

in your script, or rewrite patch of bug 7248 to be able to have this one valid again.
Comment 22 Jared Camins-Esakov 2012-04-29 14:11:52 UTC Comment hidden (obsolete)
Comment 23 Katrin Fischer 2012-04-29 19:38:56 UTC
Created attachment 9351 [details] [review]
Bug 7249 : Allow reports to be called through a restful interface

Hands back JSON, and supports caching

Squashed commit of the following:

commit 5c93506079738b9a13139417d3a0734289a4b007
Author: Chris Hall <chrish@catalyst.net.nz>
Date:   Mon Nov 21 15:41:14 2011 +1300

    Fixed bugs when creating reports, added more user friendly input and
    feedback

commit c62430bb358ee4af6ee5331b4d3a2ed67f723032
Author: Chris Hall <chrish@catalyst.net.nz>
Date:   Mon Nov 21 12:00:27 2011 +1300

    Added caching to intranet report webservices, added cache expiry and public
    options to reports

commit 2d89f0777d95b26bf08635782070b6367d0698f3
Author: Chris Cormack <chris@bigballofwax.co.nz>
Date:   Wed Nov 9 20:29:26 2011 +1300

    Working on caching for services

commit e0511f180cebd81747858ad776433fe3a1cf6854
Author: Chris Cormack <chris@bigballofwax.co.nz>
Date:   Wed Nov 9 08:28:26 2011 +1300

    Starting work on webservices

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Patch includes database update adding 2 new fields to the saved_sql table.

1) Checked that adding/deleting/editing of sql reports still works and new
fields are correctly saved to the database.

2) The saved reports page now lists the new fields correctly.

3) Checking URLs with JSON output
- if report is not public http://localhost/cgi-bin/koha/svc/report?id=2 fails,
but http://localhost:8080/cgi-bin/koha/svc/report?id=2 works
- if report is public, both links work

4) Checking results are cached
Created a report listing my borrowers, setting caching to 60 seconds and
added new borrowers before doing following tests.
- URL doesn't show new borrower immediately - correct
- Turning off the usecache system preference updates data - correct
- Restarting memcached updates data '/etc/init.d/memcached restart' - correct
- Waiting until cache time runs out updates data - correct

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Retested the feature and renewing my sign-off.

There are some things that should be noted:
- currently there is a hardcoded limit of 10 results for the reports webservice
- reports with runtime parameters are currently not supported
Comment 24 Jared Camins-Esakov 2012-04-29 19:44:36 UTC Comment hidden (obsolete)
Comment 25 Katrin Fischer 2012-04-29 20:01:24 UTC
Created attachment 9355 [details] [review]
[SIGNED-OFF] Bug 7249: Syspref to control number of rows in web service results

The original patches for bug 7249 had a hardcoded limit of ten rows in the
results returned by the web service. This patch adds a SvcMaxReportRows syspref
which allows the librarian to choose how many rows should be returned by the
web service.

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 26 Jonathan Druart 2012-05-04 12:49:19 UTC
Qa comment:
All seems good.
Paul, could you remove backquote (kohastructure.sql) before pushing please ?

Marked as Passed QA
Comment 27 Paul Poulain 2012-05-11 08:59:55 UTC
Patch pushed, with a small follow-up on online help, to explain how to access the reports. Please check that there is no english mistake ;-)
Comment 28 Jared Camins-Esakov 2012-12-31 01:01:03 UTC
There have been no further reports of problems so I am marking this bug resolved.