Bug 7520 - JSON script to return advanced search types
Summary: JSON script to return advanced search types
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Srdjan Jankovic
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-10 02:13 UTC by Srdjan Jankovic
Modified: 2023-12-28 20:42 UTC (History)
6 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
patch (2.91 KB, patch)
2012-02-10 08:07 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (2.91 KB, patch)
2012-02-13 07:22 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (2.91 KB, patch)
2012-02-13 07:29 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
patch (2.90 KB, patch)
2012-02-13 08:02 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
Signed-off patch (3.07 KB, patch)
2012-02-13 08:14 UTC, Frédéric Demians
Details | Diff | Splinter Review
bug_7520 Added helper "syspref" to Mojolicious handlers (658 bytes, patch)
2016-08-02 01:43 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug_7520 Added Meta handler to API v1 for metadata lists (7.20 KB, patch)
2016-08-02 01:44 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug_7520 Added helper "syspref" to Mojolicious handlers (618 bytes, patch)
2017-02-22 00:48 UTC, Srdjan Jankovic
Details | Diff | Splinter Review
bug_7520 Added Meta handler to API v1 for metadata lists (7.65 KB, patch)
2017-02-22 00:49 UTC, Srdjan Jankovic
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Srdjan Jankovic 2012-02-10 02:13:47 UTC
Small script to return list of available item types, useful for building item type lists in custom code, eg provided in sysprefs
Comment 1 Srdjan Jankovic 2012-02-10 08:07:27 UTC Comment hidden (obsolete)
Comment 2 Frédéric Demians 2012-02-10 08:18:17 UTC
When calling:

  cgi-bin/koha/advanced_search_types.json

identification is required. You have to log in to the OPAC as a Koha patron. I don't think it's normal. Is it?
Comment 3 Srdjan Jankovic 2012-02-12 23:09:53 UTC
If the opac requires authentication user must be logged in and auth cookie sent in order to get a response. So if you just put the url in your browser it won't work. However, if you visit the login page first and log in, it should be fine afterwards.
Comment 4 Frédéric Demians 2012-02-13 06:19:32 UTC
(In reply to comment #3)
> If the opac requires authentication user must be logged in and auth cookie sent
> in order to get a response. So if you just put the url in your browser it won't
> work. However, if you visit the login page first and log in, it should be fine
> afterwards.

The OPAC doesn't require authentication except for accessing pages displaying the borrower account. I now how to get a JSON result from your script, and I get a valid response. But since the returned data doesn't contain any confidential information (you find them on OPAC advanced search page which is public), I don't think authentication should be required.
Comment 5 Chris Cormack 2012-02-13 06:29:24 UTC
It should only require authentication when opacpublic is turned off. (There is a syspref to make the whole opac require authentication, this should of course obey that)
But if the opac is public then yes, I don't see the need for auth. Maybe it isn't checking this syspref correctly?
Comment 6 Srdjan Jankovic 2012-02-13 07:22:07 UTC Comment hidden (obsolete)
Comment 7 Srdjan Jankovic 2012-02-13 07:29:29 UTC Comment hidden (obsolete)
Comment 8 Frédéric Demians 2012-02-13 07:53:32 UTC
Thanks Chris for OpacPublic syspref.

There is a little typo. This line:

checkauth($input, !C4::Context->preference("OpacPublic"), {}, "opac");

should be:

checkauth($input, C4::Context->preference("OpacPublic"), {}, "opac");

IF the OPAC is public THEN no auth check.
Comment 9 Srdjan Jankovic 2012-02-13 08:02:36 UTC Comment hidden (obsolete)
Comment 10 Frédéric Demians 2012-02-13 08:14:11 UTC Comment hidden (obsolete)
Comment 11 Paul Poulain 2012-02-21 13:52:33 UTC
QA comment
* the code itself is OK, use JSON, that is already a Koha dependancy
* perlcritic compliant

2 things that make me mark this patch "failed QA" though:
* a unit test for this script is needed (and should be easy to write)
* an example of how/where to use this would be cool.

A question: this script can be considered as a webservice right ? why not putting it into a specific directory where we could put this kind of script ? I don't request it, just suggesting.
Comment 12 Srdjan Jankovic 2012-02-22 06:19:28 UTC
> * a unit test for this script is needed (and should be easy to write)

I have no idea how to do that. Can you please point me to an example of a koha test for a CGI request handler.
In general, I have huge problems running Koha test suite. Is there a wiki page or similar on how to run tests when developing?

> * an example of how/where to use this would be cool.

Sure. In the code?

> A question: this script can be considered as a webservice right ? why not
> putting it into a specific directory where we could put this kind of script ? I
> don't request it, just suggesting.

I wanted to do that myself, but found similar service(s) in the same place. So I'm open to suggestions. I feel adventurous already for naming it .json
Comment 13 Mark Tompsett 2015-04-18 04:05:21 UTC
*ping* Perhaps this could tie into the REST API development?
Comment 14 Liz Rea 2016-06-09 03:41:05 UTC
An example of how one could use this, is as follows - example is of a drop down that shows recent acquisitions by item type:

In your OpacMainUserBlock:

Something like

<div id="new_orders">
        <form name="searchform" method="get" action="/cgi-bin/koha/opac-search.pl">
            <input type="hidden" name="sort_by" value="acqdate_dsc">

            <input type="hidden" name="do" value="OK">
            <input type="hidden" name="idx" value="location">
            <input type="hidden" name="q" value="ORD">
            <select name="limit" id="new_orders_limit" onChange="this.form.submit()">
                <option>-- Please choose --</option>
            </select>
        </form>
</div>



In your OpacUserJS, to fetch the list:

$(document).ready(function() {
    $.getJSON("/cgi-bin/koha/advanced_search_types.json", function(data){
        $.each(data.types, function (index, value) {
            var opt = $('<option value="mc-ccode:' + value.code + '">').text(value.description).appendTo("#new_orders_limit");
        }); 
    });
Comment 15 Liz Rea 2016-06-09 03:41:44 UTC
ccode, I can't read.
Comment 16 Liz Rea 2016-08-01 02:34:17 UTC
Hi, 

I think this deserves another look by QA.

Cheers,
Liz
Comment 17 Mark Tompsett 2016-08-01 03:17:05 UTC
Wasn't there some itype itemtype removal on something elsewhere recently?
Is the permissions check correct still?
Also, still stand by my question about the REST API.
Comment 18 Katrin Fischer 2016-08-01 21:51:42 UTC
Hm, not sure - but couldn't you use a public SQL report to achieve the same?
Comment 19 Srdjan Jankovic 2016-08-02 01:43:46 UTC
Created attachment 53865 [details] [review]
bug_7520 Added helper "syspref" to Mojolicious handlers
Comment 20 Srdjan Jankovic 2016-08-02 01:44:59 UTC
Created attachment 53867 [details] [review]
bug_7520 Added Meta handler to API v1 for metadata lists

Useful for building drop-downs etc.

Kick off with /meta/advanced-search-types
Comment 21 Srdjan Jankovic 2016-08-02 01:49:12 UTC
(In reply to Katrin Fischer from comment #18)
> Hm, not sure - but couldn't you use a public SQL report to achieve the same?

It would be a complex query that goes off two totally different tables depending on a syspref.
If that's our policy I can certainly try.
Comment 22 Srdjan Jankovic 2016-08-02 01:51:13 UTC
Correction to the comment #14: url is now

/api/v1/meta/advanced-search-types
Comment 23 Srdjan Jankovic 2017-02-22 00:48:55 UTC
Created attachment 60538 [details] [review]
bug_7520 Added helper "syspref" to Mojolicious handlers
Comment 24 Srdjan Jankovic 2017-02-22 00:49:52 UTC
Created attachment 60539 [details] [review]
bug_7520 Added Meta handler to API v1 for metadata lists

Useful for building drop-downs etc.

Kick off with /meta/advanced-search-types
Comment 25 Mark Tompsett 2017-06-08 01:26:10 UTC
Comment on attachment 60539 [details] [review]
bug_7520 Added Meta handler to API v1 for metadata lists

Review of attachment 60539 [details] [review]:
-----------------------------------------------------------------

::: Koha/REST/V1/Meta.pm
@@ +12,5 @@
> +# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License along
> +# with Koha; if not, write to the Free Software Foundation, Inc.,
> +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

Since this is new, please use https://wiki.koha-community.org/wiki/Coding_Guidelines#Licence

::: t/db_dependent/api/v1/meta.t
@@ +12,5 @@
> +# A PARTICULAR PURPOSE.  See the GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License along
> +# with Koha; if not, write to the Free Software Foundation, Inc.,
> +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

Since this is new, please use https://wiki.koha-community.org/wiki/Coding_Guidelines#Licence

@@ +32,5 @@
> +my $builder = t::lib::TestBuilder->new();
> +
> +my $dbh = C4::Context->dbh;
> +$dbh->{AutoCommit} = 0;
> +$dbh->{RaiseError} = 1;

This isn't necessary, since you can do it with the schema. No sense switching between DBI and DBIx ways of doing things. It gets ugly if they conflict.

@@ +37,5 @@
> +
> +my $t = Test::Mojo->new('Koha::REST::V1');
> +
> +my $categorycode = Koha::Database->new()->schema()->resultset('Category')->first()->categorycode();
> +my $branchcode = Koha::Database->new()->schema()->resultset('Branch')->first()->branchcode();

my $schema = Koha::Database->new()->schema();
$schema->storage->txn_begin();
Then use $schema for the categorycode and branchcode.

@@ +77,5 @@
> +  ->json_has('/types/0/code')
> +  ->json_has('/types/0/description')
> +  ->json_has('/types/0/imageurl');
> +
> +$dbh->rollback;

$schema->storage->txn_rollback();
Comment 26 Mark Tompsett 2017-06-08 01:29:48 UTC
I'm going to be picky about the License. The DBIx vs DBI suggestions probably could slide, though QA might fault you. Also, rather than assume the DB's have category codes and branches, consider the use of TestBuilder.
Comment 27 Srdjan Jankovic 2017-06-08 01:42:21 UTC
Heh, 5+ years ago when I created it there was no DBIx...
Comment 28 Katrin Fischer 2023-01-07 23:51:07 UTC
I think this should probably be implemented as a public REST API route nowadays. 
It should also take into account if an itemtype has been marked to not display in advanced search in configuration.

As this is really old I'll mark as WONTFIX, please reopen if you disagree!