sub load_browse {
my ($self) = @_;
+ $self->ctx->{more_forward} = 0;
+ $self->ctx->{more_back} = 0;
+
if ($self->cgi->param('qtype') and $self->cgi->param('bterm')) {
- # Let's get some browse results!
+ no warnings 'uninitialized';
+
+ my $limit = int($self->cgi->param('limit') || 10);
+ my $offset = int($self->cgi->param('offset') || 0);
- $self->ctx->{results} = $self->editor->json_query({
+ my $browse_results = $self->editor->json_query({
from => [
"metabib.browse",
- scalar $self->cgi->param('qtype'),
- scalar $self->cgi->param('bterm'),
+ scalar($self->cgi->param('qtype')),
+ scalar($self->cgi->param('bterm')),
$self->ctx->{copy_location_group_org},
$self->ctx->{copy_location_group},
- scalar $self->cgi->param('limit'),
- scalar $self->cgi->param('offset')
+ ($offset >= 0) ? $limit + 1 : $limit,
+ $offset # NOTE Yes metabib.browse() allows negative offsets.
]
- }) or $self->apache->log->warn(
- "error in browse: " . $self->editor->event->{textcode}
- );
+ });
+
+ if (not $browse_results) { # DB error, not empty result set.
+
+ # The choice of ->warn instead of ->error for the Apache logs is a
+ # conscious one. Let's save ->error for more process-ending stuff.
+ # We're not necessarily crashing in this case.
+
+ $self->apache->log->warn(
+ "error in browse: " . $self->editor->event->{textcode}
+ );
+ $self->ctx->{browse_error} = 1;
+
+ return Apache2::Const::OK;
+ }
+
+ while (scalar @$browse_results > $limit) {
+ pop @$browse_results;
+ $self->ctx->{more_forward} = 1;
+ }
+
+ # No, this comparison for setting more_forward does not fold into
+ # those for setting more_back.
+ if ($offset < 0) {
+ $self->ctx->{more_forward} = 1;
+ }
+
+ if ($offset > 0) {
+ $self->ctx->{more_back} = 1;
+ } elsif (scalar @$browse_results < $limit) {
+ $self->ctx->{more_back} = 0;
+ } else {
+ $self->ctx->{more_back} = 1;
+ }
+
+ $self->ctx->{browse_results} = $browse_results;
}
return Apache2::Const::OK;
ALTER TABLE metabib.browse_entry ALTER COLUMN sort_value SET NOT NULL;
-CREATE INDEX browse_entry_sort_value_idx
- ON metabib.browse_entry USING BTREE (sort_value ASC);
+CREATE INDEX CONCURRENTLY browse_entry_sort_value_idx
+ ON metabib.browse_entry USING BTREE (sort_value);
+
+-- NOTE If I understand ordered indices correctly, an index on sort_value DESC
+-- is not actually needed, even though we do have a query that does ORDER BY
+-- on this column in that direction. The previous index serves for both
+-- directions, and ordering in an index is only helpful for multi-column
+-- indices.
+
+-- CREATE INDEX CONCURRENTLY browse_entry_sort_value_idx_desc
+-- ON metabib.browse_entry USING BTREE (sort_value DESC);
CREATE TYPE metabib.slim_browse_entry AS (
browse_entry BIGINT,
where_clause := '';
IF context_org IS NOT NULL OR context_loc_group IS NOT NULL THEN
- -- XXX At some point we surely need more comprehensive/correct
+ -- XXX TODO At some point we surely need more comprehensive/correct
-- holdings testing that just a join on aovc (located URIs, etc).
-- And we probably need to support a browse equivalent of search's
-- #staff modifier, i.e. scope but don't limit on visibility.
result TEXT;
BEGIN
joins_and_where := metabib._browse_joins_and_where(
- search_field, browse_term,
- context_org, context_loc_group
+ search_field, browse_term, context_org, context_loc_group
);
joins := joins_and_where[1];
EXECUTE 'SELECT mbe.sort_value FROM metabib.browse_entry mbe ' ||
joins || 'WHERE ' || where_clause ||
- ' mbe.sort_value >= ' || quote_literal(browse_term) ||
- ' ORDER BY mbe.sort_value LIMIT 1 ' INTO result;
+ ' mbe.sort_value >= public.search_normalize(' ||
+ quote_literal(browse_term) ||
+ ') ORDER BY mbe.sort_value LIMIT 1 ' INTO result;
RETURN result; -- note, can be NULL
END;
context_org INT DEFAULT NULL,
context_loc_group INT DEFAULT NULL,
result_limit INT DEFAULT 10,
- result_offset INT DEFAULT 0
+ result_offset INT DEFAULT 0 -- Can be negative!
) RETURNS SETOF metabib.slim_browse_entry AS $p$
DECLARE
joins TEXT;
pivot_sort_value TEXT;
f RECORD;
r metabib.slim_browse_entry%ROWTYPE;
+ use_offset INT;
BEGIN
pivot_sort_value := metabib.browse_pivot(
- search_field, browse_term,
- context_org, context_loc_group
+ search_field, browse_term, context_org, context_loc_group
);
IF pivot_sort_value IS NULL THEN
END IF;
joins_and_where := metabib._browse_joins_and_where(
- search_field, browse_term,
- context_org, context_loc_group
+ search_field, browse_term, context_org, context_loc_group
);
joins := joins_and_where[1];
where_clause := joins_and_where[2];
- -- Now we query for the "page" of records starting at the pivot. If we
- -- wanted to put the pivot in the middle, I guess we'd need two queries?
-
- -- XXX I really don't see the value of finding the pivot point as we could
- -- get the exact same results below by just using the browse_term
- -- directly where we use pivot_sort_value. If we wanted before and after
- -- context, we'd still need two queries (or maybe fancy windowing?) exactly
- -- as we would even if we didn't find the pivot first.
-
result_query :=
'SELECT mbe.id, mbe.value, mbedm.authority, mbe.sort_value
FROM metabib.browse_entry mbe ' ||
joins ||
- 'WHERE ' || where_clause ||
+ 'WHERE ' || where_clause;
+
+ -- PostgreSQL is not magic. We can't actually pass a negative offset.
+ IF result_offset >= 0 THEN
+ use_offset := result_offset;
+ result_query := result_query ||
' mbe.sort_value >= ' || quote_literal(pivot_sort_value) ||
- ' GROUP BY 1,2,3,4 ORDER BY mbe.sort_value ' ||
- ' LIMIT ' || result_limit ||
- ' OFFSET ' || result_offset;
+ ' GROUP BY 1,2,3,4 ORDER BY mbe.sort_value ';
+ ELSE
+ -- Step 1 of 2 to deliver what the user wants with a negative offset:
+ result_query := result_query ||
+ ' mbe.sort_value < ' || quote_literal(pivot_sort_value) ||
+ ' GROUP BY 1,2,3,4 ORDER BY mbe.sort_value DESC ';
+
+ use_offset := ABS(result_offset) - result_limit;
+ IF use_offset < 0 THEN
+ -- When result_offset is negative, it is required that
+ -- result_limit <= ABS(result_offset)
+
+ -- Higher level interfaces should not lead users down
+ -- this path anyway.
+
+ RAISE EXCEPTION 'Straddling the pivot point is not supported.';
+ END IF;
+ END IF;
+
+ result_query := result_query ||
+ ' LIMIT ' || result_limit || ' OFFSET ' || use_offset;
+
+ IF result_offset < 0 THEN
+ -- Step 2 of 2 to deliver what the user wants with a negative offset:
+ result_query := 'SELECT * FROM (' || result_query ||
+ ') x ORDER BY sort_value ASC'; -- Un-reverse the result set.
+ END IF;
FOR f IN EXECUTE result_query LOOP
r.browse_entry := f.id;
context_org INT DEFAULT NULL,
context_loc_group INT DEFAULT NULL,
result_limit INT DEFAULT 10,
- result_offset INT DEFAULT 0
+ result_offset INT DEFAULT 0 -- Can be negative!
) RETURNS SETOF metabib.slim_browse_entry AS $p$
BEGIN
RETURN QUERY SELECT * FROM metabib.browse(
PROCESS "opac/parts/org_selector.tt2";
WRAPPER "opac/parts/base.tt2";
INCLUDE "opac/parts/topnav.tt2";
- ctx.page_title = l("Browse the Catalog"); %]
+ ctx.page_title = l("Browse the Catalog");
+ limit = CGI.param('limit') || 10;
+ offset = CGI.param('offset') || 0;
+ %]
+
<div id="search-wrapper">
- [%# XXX TODO Give searchbar smarts # INCLUDE "opac/parts/searchbar.tt2" %]
+ [%# XXX TODO Give searchbar smarts so we can just do:
+ # INCLUDE "opac/parts/searchbar.tt2" %]
<div id="search-tools">
<span class="search_catalog_lbl">[% l('Search the Catalog') %]</span>
<span><a href="[% mkurl(ctx.opac_root _ '/advanced') %]"
<div id="main-content">
<div id="browse-the-catalog">
<div id="browse-controls">
- <label for="browse-search-class">[% l('Browse by') %]</label>
- [% INCLUDE "opac/parts/qtype_selector.tt2" id="browse-search-class" browse_only=1 %]
+ <form method="get">
+ <input type="hidden" name="limit"
+ value="[% limit %]" />
+
+ <label for="browse-search-class">[%
+ l('Browse by') %]</label>
+ [% INCLUDE "opac/parts/qtype_selector.tt2"
+ id="browse-search-class" browse_only=1 %]
- <input type="text" name="bterm" />
+ <label for="browse-term">[% l('for') %]</label>
+ <input type="text" name="bterm" id="browse-term"
+ value="[% CGI.param('bterm') | html %]" />
- <label for="browse-context">[% l('held under') %]</label>
- [% INCLUDE build_org_selector id='browse-context' show_loc_groups=1 arialabel=l('Select holding library') %]
+ <label for="browse-context">[%
+ l('held under') %]</label>
+ [% INCLUDE build_org_selector id='browse-context'
+ show_loc_groups=1
+ arialabel=l('Select holding library') %]
- <input type="submit" value="[% l('Go') %]" />
+ <input type="submit" value="[% l('Go') %]" />
+ </form>
</div>
[% BLOCK browse_pager %]
<div class="browse-pager">
- <a href="javascript:alert('XXX TODO');">← [%l ('Back') %]</a>
+ [% IF ctx.more_back %]
+ <a class="opac-button" href="[% mkurl('', {offset => CGI.param('offset') - limit}) %]">← [%l ('Back') %]</a>
+ [% END %]
<span class="browse-english-shortcuts">
0-9
A
[%# XXX TODO. Make off by default in config.tt2 %]
</span>
- <a href="javascript:alert('XXX TODO');">[%l ('Forward') %] →</a>
+ [% IF ctx.more_forward %]
+ <a class="opac-button" href="[% mkurl('', {offset => CGI.param('offset') + limit}) %]">[%l ('Forward') %] →</a>
+ [% END %]
</div>
[% END %]
[% PROCESS browse_pager %]
<div id="browse-results">
- [% FOR result IN browse_results %]
- [% result %]</br><!-- XXX TODO better HTML -->
+ [% IF ctx.browse_error %]
+ <span class="browse-error">
+ [% l("An error occurred browsing records. " _
+ "Please try again in a moment or report the issue " _
+ "to library staff.") %]
+ </span>
+ [% ELSE %]
+ [% FOR result IN ctx.browse_results %]
+ [% result.value | html %]</br><!-- XXX TODO serious -->
+ [% END %]
[% END %]
</div>
projects / evergreen / equinox.git / commitdiff