From c0dea517395c733b77e8eb219d2c3f43f6098d42 Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Thu, 13 Jun 2013 16:22:13 -0400 Subject: [PATCH] OPAC Browse: some squashed commits from LP #1177810 MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Namely: OPAC Browse: Various interface improvements OPAC Browse: Put best-matched browse entry in middle of result set OPAC Browse: Don't try to build hyperlinks from 680 ‡a OPAC Browse: add a CSS class to the best-matching result when not paging Remove unwanted space before question makr in "Did you mean" message OPAC Browse: Fix browse interface's use of hits-per-page user setting Signed-off-by: Lebbeous Fogle-Weekley Signed-off-by: Kathy Lussier Signed-off-by: Jason Stephenson --- .../lib/OpenILS/WWW/EGCatLoader/Browse.pm | 162 ++++++---------- Open-ILS/src/sql/Pg/030.schema.metabib.sql | 181 ++++++++++++------ .../upgrade/YYYY.schema.bib-auth-browse.sql | 180 +++++++++++------ Open-ILS/src/templates/opac/browse.tt2 | 62 +++--- Open-ILS/src/templates/opac/css/style.css.tt2 | 5 +- .../templates/opac/parts/qtype_selector.tt2 | 14 +- 6 files changed, 338 insertions(+), 266 deletions(-) diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm index cc9d73c41f..81644e63e3 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Browse.pm @@ -15,6 +15,7 @@ use OpenSRF::Utils::SettingsClient; use Digest::MD5 qw/md5_hex/; use Apache2::Const -compile => qw/OK/; use MARC::Record; +use List::Util qw/first/; #use Data::Dumper; #$Data::Dumper::Indent = 0; @@ -53,12 +54,7 @@ sub prepare_browse_parameters { no warnings 'uninitialized'; - # XXX TODO add config.global_flag rows for browse limit-limit and - # browse offset-limit? - - my $limit = int($self->cgi->param('blimit') || 10); - my $offset = int($self->cgi->param('boffset') || 0); - my $force_backward = scalar($self->cgi->param('bback')); + # XXX TODO add config.global_flag rows for browse limit-limit ? my @params = ( scalar($self->cgi->param('qtype')), @@ -68,59 +64,36 @@ sub prepare_browse_parameters { $self->ctx->{copy_location_group}, $self->ctx->{is_staff} ? 't' : 'f', scalar($self->cgi->param('bpivot')), - $force_backward ? 't' : 'f' + int( + $self->cgi->param('blimit') || + $self->ctx->{opac_hits_per_page} || 10 + ) ); - # We do need $limit, $offset, and $force_backward as part of the - # cache key, but we also need to keep them separate from other - # parameters for purposes of paging link generation. return ( - "oils_browse_" . md5_hex( - OpenSRF::Utils::JSON->perl2JSON( - [@params, $limit, $offset, $force_backward] - ) - ), - $limit, $offset, $force_backward, @params + "oils_browse_" . md5_hex(OpenSRF::Utils::JSON->perl2JSON(\@params)), + @params ); } -# Break out any Public General Notes (field 680) for display and -# hyperlinking. These are sometimes (erroneously?) called "scope notes." -# I say erroneously, tentatively, because LoC doesn't seem to document -# a "scope notes" field for authority records, while it does so for -# classification records, which are something else. But I am not a -# librarian. +# Break out any Public General Notes (field 680) for display. These are +# sometimes (erroneously?) called "scope notes." I say erroneously, +# tentatively, because LoC doesn't seem to document a "scope notes" +# field for authority records, while it does so for classification +# records, which are something else. But I am not a librarian. sub extract_public_general_notes { my ($self, $record, $row) = @_; - my @notes; - foreach my $note ($record->field('680')) { - my @note; - my $last_heading; - - foreach my $subfield ($note->subfields) { - my ($code, $value) = @$subfield; - - if ($code eq 'i') { - push @note, $value; - } elsif ($code eq '5') { - if ($last_heading) { - my $org = $self->ctx->{get_aou_by_shortname}->($value); - $last_heading->{org_id} = $org->id if $org; - } - push @note, { institution => $value }; - } elsif ($code eq 'a') { - $last_heading = { - heading => $value, bterm => search_normalize($value) - }; - push @note, $last_heading; - } - } - - push @notes, \@note if @note; - } - - $row->{notes} = \@notes; + # Make a list of strings, each string being a concatentation of any + # subfields 'i', '5', or 'a' from one field 680, in order of appearance. + $row->{notes} = [ + map { + join( + " ", + map { $_->[1] } grep { $_->[0] =~ /[i5a]/ } $_->subfields + ) + } $record->field('680') + ]; } sub find_authority_headings_and_notes { @@ -273,15 +246,10 @@ sub flesh_browse_results { } sub load_browse_impl { - my ($self, $limit, $offset, $force_backward, @params) = @_; - - my $inner_limit = ($offset >= 0 and not $force_backward) ? - $limit + 1 : $limit; + my ($self, @params) = @_; my $results = $self->editor->json_query({ - from => [ - "metabib.browse", (@params, $inner_limit, $offset) - ] + from => [ "metabib.browse", @params ] }); if (not $results) { # DB error, not empty result set. @@ -303,50 +271,23 @@ sub load_browse_impl { return $results; } -# $results can be modified by this function. This would be simpler -# but for the moving pivot concept that helps us avoid paging with -# large offsets (slow). +# Find paging information, put it into $self->ctx, and return "real" +# rows from $results, excluding those that contain only paging +# information. sub infer_browse_paging { - my ($self, $results, $limit, $offset, $force_backward) = @_; - - # (All these comments assume a default limit of 10). For typical - # not-backwards requests not at the end of the result set, we - # should have an eleventh result that tells us what's next. - while (scalar @$results > $limit) { - $self->ctx->{forward_pivot} = (pop @$results)->{browse_entry}; - $self->ctx->{more_forward} = 1; - } - - # If we're going backwards by pivot id, we don't have an eleventh - # result to tell us we can page forward, but we can assume we can - # go forward because duh, we followed a link backward to get here. - if ($force_backward and $self->cgi->param('bpivot')) { - $self->ctx->{forward_pivot} = scalar($self->cgi->param('bpivot')); - $self->ctx->{more_forward} = 1; - } - - # The pivot that the user can use for going backwards is the first - # of the result set. - if (@$results) { - $self->ctx->{back_pivot} = $results->[0]->{browse_entry}; - } - - # The result of these tests relate to basic limit/offset paging. + my ($self, $results) = @_; - # This comparison for setting more_forward does not fold into - # those for setting more_back. - if ($offset < 0 || $force_backward) { - $self->ctx->{more_forward} = 1; + foreach (@$results) { + if ($_->{pivot_point}) { + if ($_->{row_number} < 0) { # sic + $self->ctx->{forward_pivot} = $_->{pivot_point}; + } else { + $self->ctx->{back_pivot} = $_->{pivot_point}; + } + } } - if ($offset > 0 or (!$force_backward and $self->cgi->param('bpivot'))) { - $self->ctx->{more_back} = 1; - } elsif (scalar @$results < $limit) { - $self->ctx->{more_back} = 0; - } elsif (not($self->cgi->param('bterm') eq '0' and - not defined $self->cgi->param('bpivot'))) { # paging links - $self->ctx->{more_back} = 1; - } + return [ grep { not defined $_->{pivot_point} } @$results ]; } sub leading_article_test { @@ -370,6 +311,8 @@ sub leading_article_test { if ($map->{$qtype}) { if ($bterm =~ qr/$map->{$qtype}/i) { $self->ctx->{browse_leading_article_warning} = 1; + ($self->ctx->{browse_leading_article_alternative} = $bterm) =~ + s/$map->{$qtype}//; } } }; @@ -383,8 +326,17 @@ sub load_browse { _init_browse_cache(); - $self->ctx->{more_forward} = 0; - $self->ctx->{more_back} = 0; + # If there's a user logged in, flesh extended user info so we can get + # her opac.hits_per_page setting, if any. + if ($self->ctx->{user}) { + $self->prepare_extended_user_info('settings'); + if (my $setting = first { $_->name eq 'opac.hits_per_page' } + @{$self->ctx->{user}->settings}) { + + $self->ctx->{opac_hits_per_page} = + int(OpenSRF::Utils::JSON->JSON2perl($setting->value)); + } + } if ($self->cgi->param('qtype') and defined $self->cgi->param('bterm')) { @@ -393,24 +345,18 @@ sub load_browse { $self->cgi->param('bterm') ); - my ($cache_key, $limit, $offset, $force_backward, @params) = - $self->prepare_browse_parameters; + my ($cache_key, @params) = $self->prepare_browse_parameters; my $results = $browse_cache->get_cache($cache_key); if (not $results) { - $results = $self->load_browse_impl( - $limit, $offset, $force_backward, @params - ); + $results = $self->load_browse_impl(@params); if ($results) { $browse_cache->put_cache($cache_key, $results, $browse_timeout); } } if ($results) { - $self->infer_browse_paging( - $results, $limit, $offset, $force_backward - ); - $self->ctx->{browse_results} = $results; + $self->ctx->{browse_results} = $self->infer_browse_paging($results); } # We don't need an else clause to send the user a 5XX error or diff --git a/Open-ILS/src/sql/Pg/030.schema.metabib.sql b/Open-ILS/src/sql/Pg/030.schema.metabib.sql index ae74fbad46..2014655cbc 100644 --- a/Open-ILS/src/sql/Pg/030.schema.metabib.sql +++ b/Open-ILS/src/sql/Pg/030.schema.metabib.sql @@ -1781,9 +1781,10 @@ CREATE TYPE metabib.flat_browse_entry_appearance AS ( authorities TEXT, sources INT, -- visible ones, that is row_number INT, -- internal use, sort of - accurate BOOL -- Count in sources field is accurate? Not + accurate BOOL, -- Count in sources field is accurate? Not -- if we had more than a browse superpage -- of records to look at. + pivot_point BIGINT ); @@ -1807,42 +1808,55 @@ END; $p$ LANGUAGE PLPGSQL; CREATE OR REPLACE FUNCTION metabib.staged_browse( - core_query TEXT, + query TEXT, context_org INT, context_locations INT[], staff BOOL, browse_superpage_size INT, + count_up_from_zero BOOL, -- if false, count down from -1 result_limit INT, - use_offset INT + next_pivot_pos INT ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ DECLARE - core_cursor REFCURSOR; - core_record RECORD; + curs REFCURSOR; + rec RECORD; qpfts_query TEXT; result_row metabib.flat_browse_entry_appearance%ROWTYPE; results_skipped INT := 0; - results_returned INT := 0; + row_counter INT := 0; + row_number INT; slice_start INT; slice_end INT; full_end INT; superpage_of_records BIGINT[]; superpage_size INT; BEGIN - OPEN core_cursor FOR EXECUTE core_query; + IF count_up_from_zero THEN + row_number := 0; + ELSE + row_number := -1; + END IF; + + OPEN curs FOR EXECUTE query; LOOP - FETCH core_cursor INTO core_record; - EXIT WHEN NOT FOUND; + FETCH curs INTO rec; + IF NOT FOUND THEN + IF result_row.pivot_point IS NOT NULL THEN + RETURN NEXT result_row; + END IF; + RETURN; + END IF; result_row.sources := 0; - full_end := ARRAY_LENGTH(core_record.records, 1); + full_end := ARRAY_LENGTH(rec.records, 1); superpage_size := COALESCE(browse_superpage_size, full_end); slice_start := 1; slice_end := superpage_size; WHILE result_row.sources = 0 AND slice_start <= full_end LOOP - superpage_of_records := core_record.records[slice_start:slice_end]; + superpage_of_records := rec.records[slice_start:slice_end]; qpfts_query := 'SELECT NULL::BIGINT AS id, ARRAY[r] AS records, ' || '1::INT AS rel FROM (SELECT UNNEST(' || @@ -1870,30 +1884,57 @@ BEGIN browse_superpage_size >= full_end; IF result_row.sources > 0 THEN - IF results_skipped < use_offset THEN - results_skipped := results_skipped + 1; - CONTINUE; - END IF; + -- We've got a browse entry with visible holdings. Yay. + - result_row.browse_entry := core_record.id; - result_row.authorities := core_record.authorities; - result_row.fields := core_record.fields; - result_row.value := core_record.value; + -- The function that calls this function needs row_number in order + -- to correctly order results from two different runs of this + -- functions. + result_row.row_number := row_number; - -- This is needed so our caller can flip it and reverse it. - result_row.row_number := results_returned; + -- Now, if row_counter is still less than limit, return a row. If + -- not, but it is less than next_pivot_pos, continue on without + -- returning actual result rows until we find + -- that next pivot, and return it. - RETURN NEXT result_row; + IF row_counter < result_limit THEN + result_row.browse_entry := rec.id; + result_row.authorities := rec.authorities; + result_row.fields := rec.fields; + result_row.value := rec.value; - results_returned := results_returned + 1; + RETURN NEXT result_row; + ELSE + result_row.browse_entry := NULL; + result_row.authorities := NULL; + result_row.fields := NULL; + result_row.value := NULL; + result_row.sources := NULL; + result_row.accurate := NULL; + result_row.pivot_point := rec.id; + + IF row_counter >= next_pivot_pos THEN + RETURN NEXT result_row; + RETURN; + END IF; + END IF; - EXIT WHEN results_returned >= result_limit; + IF count_up_from_zero THEN + row_number := row_number + 1; + ELSE + row_number := row_number - 1; + END IF; + + -- row_counter is different from row_number. + -- It simply counts up from zero so that we know when + -- we've reached our limit. + row_counter := row_counter + 1; END IF; END LOOP; END; $p$ LANGUAGE PLPGSQL; --- This is optimized to be fast for values of result_offset near zero. + CREATE OR REPLACE FUNCTION metabib.browse( search_field INT[], browse_term TEXT, @@ -1901,20 +1942,23 @@ CREATE OR REPLACE FUNCTION metabib.browse( context_loc_group INT DEFAULT NULL, staff BOOL DEFAULT FALSE, pivot_id BIGINT DEFAULT NULL, - force_backward BOOL DEFAULT FALSE, - result_limit INT DEFAULT 10, - result_offset INT DEFAULT 0 -- Can be negative! + result_limit INT DEFAULT 10 ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ DECLARE core_query TEXT; - whole_query TEXT; + back_query TEXT; + forward_query TEXT; pivot_sort_value TEXT; pivot_sort_fallback TEXT; context_locations INT[]; - use_offset INT; browse_superpage_size INT; results_skipped INT := 0; + back_limit INT; + back_to_pivot INT; + forward_limit INT; + forward_to_pivot INT; BEGIN + -- First, find the pivot if we were given a browse term but not a pivot. IF pivot_id IS NULL THEN pivot_id := metabib.browse_pivot(search_field, browse_term); END IF; @@ -1922,20 +1966,43 @@ BEGIN SELECT INTO pivot_sort_value, pivot_sort_fallback sort_value, value FROM metabib.browse_entry WHERE id = pivot_id; + -- Bail if we couldn't find a pivot. IF pivot_sort_value IS NULL THEN RETURN; END IF; + -- Transform the context_loc_group argument (if any) (logc at the + -- TPAC layer) into a form we'll be able to use. IF context_loc_group IS NOT NULL THEN SELECT INTO context_locations ARRAY_AGG(location) FROM asset.copy_location_group_map WHERE lgroup = context_loc_group; END IF; + -- Get the configured size of browse superpages. SELECT INTO browse_superpage_size value -- NULL ok FROM config.global_flag WHERE enabled AND name = 'opac.browse.holdings_visibility_test_limit'; + -- First we're going to search backward from the pivot, then we're going + -- to search forward. In each direction, we need two limits. At the + -- lesser of the two limits, we delineate the edge of the result set + -- we're going to return. At the greater of the two limits, we find the + -- pivot value that would represent an offset from the current pivot + -- at a distance of one "page" in either direction, where a "page" is a + -- result set of the size specified in the "result_limit" argument. + -- + -- The two limits in each direction make four derived values in total, + -- and we calculate them now. + back_limit := CEIL(result_limit::FLOAT / 2); + back_to_pivot := result_limit; + forward_limit := result_limit / 2; + forward_to_pivot := result_limit - 1; + + -- This is the meat of the SQL query that finds browse entries. We'll + -- pass this to a function which uses it with a cursor, so that individual + -- rows may be fetched in a loop until some condition is satisfied, without + -- waiting for a result set of fixed size to be collected all at once. core_query := ' SELECT mbe.id, @@ -1957,30 +2024,29 @@ BEGIN ) WHERE '; - -- PostgreSQL is not magic. We can't actually pass a negative offset. - IF result_offset >= 0 AND NOT force_backward THEN - use_offset := result_offset; - core_query := core_query || - ' mbe.sort_value >= ' || quote_literal(pivot_sort_value) || - ' GROUP BY 1,2,3 ORDER BY mbe.sort_value, mbe.value '; - - RETURN QUERY SELECT * FROM metabib.staged_browse( - core_query, context_org, context_locations, - staff, browse_superpage_size, result_limit, use_offset - ); - ELSE - -- Part 1 of 2 to deliver what the user wants with a negative offset: - core_query := core_query || - ' mbe.sort_value < ' || quote_literal(pivot_sort_value) || - ' GROUP BY 1,2,3 ORDER BY mbe.sort_value DESC, mbe.value DESC '; - - -- Part 2 of 2 to deliver what the user wants with a negative offset: - RETURN QUERY SELECT * FROM (SELECT * FROM metabib.staged_browse( - core_query, context_org, context_locations, - staff, browse_superpage_size, result_limit, use_offset - )) sb ORDER BY row_number DESC; + -- This is the variant of the query for browsing backward. + back_query := core_query || + ' mbe.sort_value <= ' || quote_literal(pivot_sort_value) || + ' GROUP BY 1,2,3 ORDER BY mbe.sort_value DESC, mbe.value DESC '; + + -- This variant browses forward. + forward_query := core_query || + ' mbe.sort_value > ' || quote_literal(pivot_sort_value) || + ' GROUP BY 1,2,3 ORDER BY mbe.sort_value, mbe.value '; + + -- We now call the function which applies a cursor to the provided + -- queries, stopping at the appropriate limits and also giving us + -- the next page's pivot. + RETURN QUERY + SELECT * FROM metabib.staged_browse( + back_query, context_org, context_locations, + staff, browse_superpage_size, TRUE, back_limit, back_to_pivot + ) UNION + SELECT * FROM metabib.staged_browse( + forward_query, context_org, context_locations, + staff, browse_superpage_size, FALSE, forward_limit, forward_to_pivot + ) ORDER BY row_number DESC; - END IF; END; $p$ LANGUAGE PLPGSQL; @@ -1991,9 +2057,7 @@ CREATE OR REPLACE FUNCTION metabib.browse( context_loc_group INT DEFAULT NULL, staff BOOL DEFAULT FALSE, pivot_id BIGINT DEFAULT NULL, - force_backward BOOL DEFAULT FALSE, - result_limit INT DEFAULT 10, - result_offset INT DEFAULT 0 -- Can be negative, implying backward! + result_limit INT DEFAULT 10 ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ BEGIN RETURN QUERY SELECT * FROM metabib.browse( @@ -2004,12 +2068,9 @@ BEGIN context_loc_group, staff, pivot_id, - force_backward, - result_limit, - result_offset + result_limit ); END; $p$ LANGUAGE PLPGSQL; - COMMIT; diff --git a/Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql b/Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql index e441641053..2077f9c9da 100644 --- a/Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql +++ b/Open-ILS/src/sql/Pg/upgrade/YYYY.schema.bib-auth-browse.sql @@ -7148,9 +7148,10 @@ CREATE TYPE metabib.flat_browse_entry_appearance AS ( authorities TEXT, sources INT, -- visible ones, that is row_number INT, -- internal use, sort of - accurate BOOL -- Count in sources field is accurate? Not + accurate BOOL, -- Count in sources field is accurate? Not -- if we had more than a browse superpage -- of records to look at. + pivot_point BIGINT ); @@ -7174,42 +7175,55 @@ END; $p$ LANGUAGE PLPGSQL; CREATE OR REPLACE FUNCTION metabib.staged_browse( - core_query TEXT, + query TEXT, context_org INT, context_locations INT[], staff BOOL, browse_superpage_size INT, + count_up_from_zero BOOL, -- if false, count down from -1 result_limit INT, - use_offset INT + next_pivot_pos INT ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ DECLARE - core_cursor REFCURSOR; - core_record RECORD; + curs REFCURSOR; + rec RECORD; qpfts_query TEXT; result_row metabib.flat_browse_entry_appearance%ROWTYPE; results_skipped INT := 0; - results_returned INT := 0; + row_counter INT := 0; + row_number INT; slice_start INT; slice_end INT; full_end INT; superpage_of_records BIGINT[]; superpage_size INT; BEGIN - OPEN core_cursor FOR EXECUTE core_query; + IF count_up_from_zero THEN + row_number := 0; + ELSE + row_number := -1; + END IF; + + OPEN curs FOR EXECUTE query; LOOP - FETCH core_cursor INTO core_record; - EXIT WHEN NOT FOUND; + FETCH curs INTO rec; + IF NOT FOUND THEN + IF result_row.pivot_point IS NOT NULL THEN + RETURN NEXT result_row; + END IF; + RETURN; + END IF; result_row.sources := 0; - full_end := ARRAY_LENGTH(core_record.records, 1); + full_end := ARRAY_LENGTH(rec.records, 1); superpage_size := COALESCE(browse_superpage_size, full_end); slice_start := 1; slice_end := superpage_size; WHILE result_row.sources = 0 AND slice_start <= full_end LOOP - superpage_of_records := core_record.records[slice_start:slice_end]; + superpage_of_records := rec.records[slice_start:slice_end]; qpfts_query := 'SELECT NULL::BIGINT AS id, ARRAY[r] AS records, ' || '1::INT AS rel FROM (SELECT UNNEST(' || @@ -7237,30 +7251,57 @@ BEGIN browse_superpage_size >= full_end; IF result_row.sources > 0 THEN - IF results_skipped < use_offset THEN - results_skipped := results_skipped + 1; - CONTINUE; - END IF; + -- We've got a browse entry with visible holdings. Yay. - result_row.browse_entry := core_record.id; - result_row.authorities := core_record.authorities; - result_row.fields := core_record.fields; - result_row.value := core_record.value; - -- This is needed so our caller can flip it and reverse it. - result_row.row_number := results_returned; + -- The function that calls this function needs row_number in order + -- to correctly order results from two different runs of this + -- functions. + result_row.row_number := row_number; - RETURN NEXT result_row; + -- Now, if row_counter is still less than limit, return a row. If + -- not, but it is less than next_pivot_pos, continue on without + -- returning actual result rows until we find + -- that next pivot, and return it. - results_returned := results_returned + 1; + IF row_counter < result_limit THEN + result_row.browse_entry := rec.id; + result_row.authorities := rec.authorities; + result_row.fields := rec.fields; + result_row.value := rec.value; - EXIT WHEN results_returned >= result_limit; + RETURN NEXT result_row; + ELSE + result_row.browse_entry := NULL; + result_row.authorities := NULL; + result_row.fields := NULL; + result_row.value := NULL; + result_row.sources := NULL; + result_row.accurate := NULL; + result_row.pivot_point := rec.id; + + IF row_counter >= next_pivot_pos THEN + RETURN NEXT result_row; + RETURN; + END IF; + END IF; + + IF count_up_from_zero THEN + row_number := row_number + 1; + ELSE + row_number := row_number - 1; + END IF; + + -- row_counter is different from row_number. + -- It simply counts up from zero so that we know when + -- we've reached our limit. + row_counter := row_counter + 1; END IF; END LOOP; END; $p$ LANGUAGE PLPGSQL; --- This is optimized to be fast for values of result_offset near zero. + CREATE OR REPLACE FUNCTION metabib.browse( search_field INT[], browse_term TEXT, @@ -7268,20 +7309,23 @@ CREATE OR REPLACE FUNCTION metabib.browse( context_loc_group INT DEFAULT NULL, staff BOOL DEFAULT FALSE, pivot_id BIGINT DEFAULT NULL, - force_backward BOOL DEFAULT FALSE, - result_limit INT DEFAULT 10, - result_offset INT DEFAULT 0 -- Can be negative! + result_limit INT DEFAULT 10 ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ DECLARE core_query TEXT; - whole_query TEXT; + back_query TEXT; + forward_query TEXT; pivot_sort_value TEXT; pivot_sort_fallback TEXT; context_locations INT[]; - use_offset INT; browse_superpage_size INT; results_skipped INT := 0; + back_limit INT; + back_to_pivot INT; + forward_limit INT; + forward_to_pivot INT; BEGIN + -- First, find the pivot if we were given a browse term but not a pivot. IF pivot_id IS NULL THEN pivot_id := metabib.browse_pivot(search_field, browse_term); END IF; @@ -7289,20 +7333,43 @@ BEGIN SELECT INTO pivot_sort_value, pivot_sort_fallback sort_value, value FROM metabib.browse_entry WHERE id = pivot_id; + -- Bail if we couldn't find a pivot. IF pivot_sort_value IS NULL THEN RETURN; END IF; + -- Transform the context_loc_group argument (if any) (logc at the + -- TPAC layer) into a form we'll be able to use. IF context_loc_group IS NOT NULL THEN SELECT INTO context_locations ARRAY_AGG(location) FROM asset.copy_location_group_map WHERE lgroup = context_loc_group; END IF; + -- Get the configured size of browse superpages. SELECT INTO browse_superpage_size value -- NULL ok FROM config.global_flag WHERE enabled AND name = 'opac.browse.holdings_visibility_test_limit'; + -- First we're going to search backward from the pivot, then we're going + -- to search forward. In each direction, we need two limits. At the + -- lesser of the two limits, we delineate the edge of the result set + -- we're going to return. At the greater of the two limits, we find the + -- pivot value that would represent an offset from the current pivot + -- at a distance of one "page" in either direction, where a "page" is a + -- result set of the size specified in the "result_limit" argument. + -- + -- The two limits in each direction make four derived values in total, + -- and we calculate them now. + back_limit := CEIL(result_limit::FLOAT / 2); + back_to_pivot := result_limit; + forward_limit := result_limit / 2; + forward_to_pivot := result_limit - 1; + + -- This is the meat of the SQL query that finds browse entries. We'll + -- pass this to a function which uses it with a cursor, so that individual + -- rows may be fetched in a loop until some condition is satisfied, without + -- waiting for a result set of fixed size to be collected all at once. core_query := ' SELECT mbe.id, @@ -7324,30 +7391,29 @@ BEGIN ) WHERE '; - -- PostgreSQL is not magic. We can't actually pass a negative offset. - IF result_offset >= 0 AND NOT force_backward THEN - use_offset := result_offset; - core_query := core_query || - ' mbe.sort_value >= ' || quote_literal(pivot_sort_value) || - ' GROUP BY 1,2,3 ORDER BY mbe.sort_value, mbe.value '; - - RETURN QUERY SELECT * FROM metabib.staged_browse( - core_query, context_org, context_locations, - staff, browse_superpage_size, result_limit, use_offset - ); - ELSE - -- Part 1 of 2 to deliver what the user wants with a negative offset: - core_query := core_query || - ' mbe.sort_value < ' || quote_literal(pivot_sort_value) || - ' GROUP BY 1,2,3 ORDER BY mbe.sort_value DESC, mbe.value DESC '; - - -- Part 2 of 2 to deliver what the user wants with a negative offset: - RETURN QUERY SELECT * FROM (SELECT * FROM metabib.staged_browse( - core_query, context_org, context_locations, - staff, browse_superpage_size, result_limit, use_offset - )) sb ORDER BY row_number DESC; + -- This is the variant of the query for browsing backward. + back_query := core_query || + ' mbe.sort_value <= ' || quote_literal(pivot_sort_value) || + ' GROUP BY 1,2,3 ORDER BY mbe.sort_value DESC, mbe.value DESC '; + + -- This variant browses forward. + forward_query := core_query || + ' mbe.sort_value > ' || quote_literal(pivot_sort_value) || + ' GROUP BY 1,2,3 ORDER BY mbe.sort_value, mbe.value '; + + -- We now call the function which applies a cursor to the provided + -- queries, stopping at the appropriate limits and also giving us + -- the next page's pivot. + RETURN QUERY + SELECT * FROM metabib.staged_browse( + back_query, context_org, context_locations, + staff, browse_superpage_size, TRUE, back_limit, back_to_pivot + ) UNION + SELECT * FROM metabib.staged_browse( + forward_query, context_org, context_locations, + staff, browse_superpage_size, FALSE, forward_limit, forward_to_pivot + ) ORDER BY row_number DESC; - END IF; END; $p$ LANGUAGE PLPGSQL; @@ -7358,9 +7424,7 @@ CREATE OR REPLACE FUNCTION metabib.browse( context_loc_group INT DEFAULT NULL, staff BOOL DEFAULT FALSE, pivot_id BIGINT DEFAULT NULL, - force_backward BOOL DEFAULT FALSE, - result_limit INT DEFAULT 10, - result_offset INT DEFAULT 0 -- Can be negative, implying backward! + result_limit INT DEFAULT 10 ) RETURNS SETOF metabib.flat_browse_entry_appearance AS $p$ BEGIN RETURN QUERY SELECT * FROM metabib.browse( @@ -7371,9 +7435,7 @@ BEGIN context_loc_group, staff, pivot_id, - force_backward, - result_limit, - result_offset + result_limit ); END; $p$ LANGUAGE PLPGSQL; diff --git a/Open-ILS/src/templates/opac/browse.tt2 b/Open-ILS/src/templates/opac/browse.tt2 index 35a9d5a8b4..44062b6ace 100644 --- a/Open-ILS/src/templates/opac/browse.tt2 +++ b/Open-ILS/src/templates/opac/browse.tt2 @@ -7,10 +7,9 @@ INCLUDE "opac/parts/topnav.tt2"; ctx.page_title = l("Browse the Catalog"); - blimit = CGI.param('blimit') || 10; - boffset = CGI.param('boffset') || 0; + blimit = CGI.param('blimit') || ctx.opac_hits_per_page || 10; - depart_list = ['blimit', 'bterm', 'boffset', 'bpivot', 'bback']; + depart_list = ['blimit', 'bterm', 'bpivot']; %]
@@ -27,46 +26,49 @@
-
+ [% control_qtype = INCLUDE "opac/parts/qtype_selector.tt2" - id="browse-search-class" browse_only=1 %] + id="browse-search-class" browse_only=1 plural=1 %] [% control_bterm = BLOCK %][% END %] [% control_locg = INCLUDE build_org_selector id='browse-context' show_loc_groups=1 arialabel=l('Select holding library') %] - [% l('Browse by [_1] for [_2] held under [_3]', control_qtype, control_bterm, control_locg) %] + [% l('Browse for [_1] starting with [_2] in [_3]', control_qtype, control_bterm, control_locg) %] +
[% BLOCK browse_pager %]
- [% IF ctx.more_back %] - ← [%l ('Back') %] + [% IF ctx.back_pivot %] + ← [%l ('Back') %] [% END %] [% IF browse.english_pager; # XXX how to apply i18n here? current_qtype = CGI.param('qtype') || 'title' %] - 0-9 + 0-9 [% FOR letter IN ['A'..'Z'] %] - [% letter %] + [% letter %] [% END %] [% END %] - [% IF ctx.more_forward %] - [%l ('Forward') %] → + [% IF ctx.forward_pivot %] + [%l ('Next') %] → [% END %] + +
[% END %] - [% PROCESS browse_pager %] + [% PROCESS browse_pager id=0 %]
[% IF ctx.browse_error %] @@ -78,13 +80,22 @@ [% ELSE %] [% IF ctx.browse_leading_article_warning %]
- [% l("Your browse term seems to begin with an article. You might get better results by omitting the article.") %] + [% l("Your browse term seems to begin with an article (a, an, the). You might get better results by omitting the article.") %] + [% IF ctx.browse_leading_article_alternative %] +

+ [% alternative_link = BLOCK %] + [% ctx.browse_leading_article_alternative | html %] + [%- END; # alternative_link BLOCK + l("Did you mean [_1]?", alternative_link); + END # IF %] +

[% END %] -
    + +
      [% FOR result IN ctx.browse_results %]
    1. - +
@@ -150,22 +161,7 @@ [% l("Note:") %] - [% FOR piece IN note; - IF piece.heading; - mkurl_args = {bterm => piece.bterm}; - IF piece.org_id; - mkurl_args.locg = piece.org_id; - END; - %] - [% piece.heading | html %] - [% ELSIF piece.institution %] - - [% piece.institution | html %] - - [% ELSE %] - [% piece | html %] - [% END; - END %] + [% FOR piece IN note; piece | html; END %]
[% END; diff --git a/Open-ILS/src/templates/opac/css/style.css.tt2 b/Open-ILS/src/templates/opac/css/style.css.tt2 index 3efc864ce3..422fdd61dc 100644 --- a/Open-ILS/src/templates/opac/css/style.css.tt2 +++ b/Open-ILS/src/templates/opac/css/style.css.tt2 @@ -1543,11 +1543,14 @@ a.preflib_change { .browse-result-sources, .browse-result-authority-bib-links { margin-left: 1em; } +.browse-result-best-match { + font-weight: bold; +} .browse-pager { margin: 2ex 0; } .browse-result-list { - list-style-type: square; + padding-bottom: 0.5ex; } .browse-shortcuts { font-size: 120%; diff --git a/Open-ILS/src/templates/opac/parts/qtype_selector.tt2 b/Open-ILS/src/templates/opac/parts/qtype_selector.tt2 index 30edbc6e9f..6986fc12d9 100644 --- a/Open-ILS/src/templates/opac/parts/qtype_selector.tt2 +++ b/Open-ILS/src/templates/opac/parts/qtype_selector.tt2 @@ -1,10 +1,10 @@ [% query_types = [ {value => "keyword", label => l("Keyword")}, - {value => "title", label => l("Title"), browse => 1}, + {value => "title", label => l("Title"), plural_label => l("Titles"), browse => 1}, {value => "jtitle", label => l("Journal Title")}, - {value => "author", label => l("Author"), browse => 1}, - {value => "subject", label => l("Subject"), browse => 1}, - {value => "series", label => l("Series"), browse => 1}, + {value => "author", label => l("Author"), plural_label => l("Authors"), browse => 1}, + {value => "subject", label => l("Subject"), plural_label => l("Subjects"), browse => 1}, + {value => "series", label => l("Series"), plural_label => l("Series"), browse => 1}, {value => "id|bibcn", label => l("Bib Call Number")} ] %] -- 2.43.2