[open-ils-commits] r19144 - trunk/Open-ILS/src/perlmods/OpenILS/Application (dbs)

[svn at svn.open-ils.org]

Author: dbs
Date: 2011-01-10 10:27:37 -0500 (Mon, 10 Jan 2011)
New Revision: 19144

Modified:
   trunk/Open-ILS/src/perlmods/OpenILS/Application/SuperCat.pm
Log:
Simplify authority reference deduplication

Rather than multiplying the number of records to be returned in
the case of a .refs. call, stick with the original number. This
avoids corner cases where paging through the results would skip
some records, and the UI is not overly heavily affected as even
in the case where one record offers 10 matches for see also /
see from references and represents an entire page of results,
the UI will display 11 lines (1 for the 1XX field and 10 for
the 4XX / 5XX fields). The user will be able to page to the next
set of results to continue browsing.

We can also be smarter about some things:
  * Return the original list immediately if not a .refs. call
  * No need to reverse and unshift the list items when
    deduping the .refs. list; just stick with the same
    order of the incoming list


Modified: trunk/Open-ILS/src/perlmods/OpenILS/Application/SuperCat.pm
===================================================================
--- trunk/Open-ILS/src/perlmods/OpenILS/Application/SuperCat.pm	2011-01-10 03:47:37 UTC (rev 19143)
+++ trunk/Open-ILS/src/perlmods/OpenILS/Application/SuperCat.pm	2011-01-10 15:27:37 UTC (rev 19144)
@@ -2,6 +2,7 @@
 use XML::LibXML;
 use XML::LibXSLT;
 use Unicode::Normalize;
+use Data::Dumper;
 
 # ... and this has some handy common methods
 use OpenILS::Application::AppUtils;
@@ -1014,13 +1015,6 @@
             push(@ref_tags, '5' . substr($tagname, 1, 2));
         }
     }
-    # We'll remove dupe record IDs that turn up due to 4xx and 5xx matches,
-    # so leave some room for expansion
-    if ($self->api_name =~ /\.refs\./) {
-        $after_limit *= 2;
-        $before_limit *= 2;
-    }
-
     my @list = ();
 
     if ($page < 0) {
@@ -1057,22 +1051,18 @@
         push @list, map { $_->{record} } @$after;
     }
 
+    # If we're not pulling in see/see also references, just return the raw list
+    if ($self->api_name !~ /\.refs\./) {
+        return \@list;
+    } 
+
+    # Remove dupe record IDs that turn up due to 4xx and 5xx matches
     my @retlist = ();
     my %seen;
-    if ($page < 0) {
-        foreach my $record (reverse @list) {
-            next if exists $seen{$record};
-            unshift @retlist, $record;
-            $seen{$record} = 1;
-            last if scalar(@retlist) == $page_size;
-        }
-    } else {
-        foreach my $record (@list) {
-            next if exists $seen{$record};
-            push @retlist, $record;
-            $seen{$record} = 1;
-            last if scalar(@retlist) == $page_size;
-        }
+    foreach my $record (@list) {
+        next if exists $seen{$record};
+        push @retlist, int($record);
+        $seen{$record} = 1;
     }
 
     return \@retlist;
@@ -1526,11 +1516,6 @@
             push(@ref_tags, '5' . substr($tagname, 1, 2));
         }
     }
-    # We'll remove dupe record IDs that turn up due to 4xx and 5xx matches,
-    # so leave some room for expansion
-    if ($self->api_name =~ /\.refs\./) {
-        $ref_limit *= 2;
-    }
 
     my @list = ();
 
@@ -1571,22 +1556,18 @@
         push @list, map { $_->{record} } @$after;
     }
 
+    # If we're not pulling in see/see also references, just return the raw list
+    if ($self->api_name !~ /\.refs\./) {
+        return \@list;
+    }
+
+    # Remove dupe record IDs that turn up due to 4xx and 5xx matches
     my @retlist = ();
     my %seen;
-    if ($page < 1) {
-        foreach my $record (reverse @list) {
-            next if exists $seen{$record};
-            unshift @retlist, $record;
-            $seen{$record} = 1;
-            last if scalar(@retlist) == $limit;
-        }
-    } else {
-        foreach my $record (@list) {
-            next if exists $seen{$record};
-            push @retlist, $record;
-            $seen{$record} = 1;
-            last if scalar(@retlist) == $limit;
-        }
+    foreach my $record (@list) {
+        next if exists $seen{$record};
+        push @retlist, int($record);
+        $seen{$record} = 1;
     }
 
     return \@retlist;