[Bps-public-commit] r19866 - in Net-Google-Code/trunk/lib/Net/Google/Code: Issue

sunnavy at bestpractical.com sunnavy at bestpractical.com
Wed Jun 3 10:54:24 EDT 2009


Author: sunnavy
Date: Wed Jun  3 10:54:23 2009
New Revision: 19866

Modified:
   Net-Google-Code/trunk/lib/Net/Google/Code/Issue/Search.pm
   Net-Google-Code/trunk/lib/Net/Google/Code/Role/Pageable.pm

Log:
updated_after does *not* do what we want, rename it to modified_date, make it only check the Modified column, and add the caveat

Modified: Net-Google-Code/trunk/lib/Net/Google/Code/Issue/Search.pm
==============================================================================
--- Net-Google-Code/trunk/lib/Net/Google/Code/Issue/Search.pm	(original)
+++ Net-Google-Code/trunk/lib/Net/Google/Code/Issue/Search.pm	Wed Jun  3 10:54:23 2009
@@ -37,7 +37,7 @@
         load_after_search => 1,
         can               => 2,
         colspec           => 'ID+Type+Status+Priority+Milestone+Owner+Summary',
-        updated_after     => undef,
+        modified_after     => undef,
         @_
     );
 
@@ -45,18 +45,15 @@
         $args{can} = $CAN_MAP{ $args{can} };
     }
 
-    my $user_sort;
-    if ( $args{updated_after} ) {
+    if ( $args{modified_after} ) {
         $args{colspec} .= '+Modified' unless $args{colspec} =~ /Modified/;
-        $user_sort = $args{sort};
-        $args{sort} = '-id'; # to sort by id reversely
 
-        # convert updated_after to epoch
-        if ( ref $args{updated_after} ) {
-            $args{updated_after} = $args{updated_after}->epoch;
+        # convert modified_after to epoch
+        if ( ref $args{modified_after} ) {
+            $args{modified_after} = $args{modified_after}->epoch;
         }
         else {
-            $args{updated_after} = UnixDate( $args{updated_after}, '%o' );
+            $args{modified_after} = UnixDate( $args{modified_after}, '%o' );
         }
     }
 
@@ -76,36 +73,20 @@
 
     if ( $mech->title =~ /issue\s+(\d+)/i ) {
 
-         get only one ticket
+        # get only one ticket
         my $issue =
           Net::Google::Code::Issue->new( project => $self->project, id => $1, );
-        $issue->load if $args{load_after_search} || $args{updated_after};
-        if ( !$args{updated_after} || $issue->updated->epoch > $args{updated_after} ) {
-            $self->results( [$issue] );
-        }
-        else {
-            $self->results( [] );
-        }
+        $issue->load if $args{load_after_search};
+        $self->results( [$issue] );
     }
     elsif ( $mech->title =~ /issues/i ) {
 
         # get a ticket list
         my @rows = $self->rows(
-            html          => $content,
-            limit         => $args{limit},
-            updated_after => $args{updated_after},
+            html           => $content,
+            limit          => $args{limit},
+            modified_after => $args{modified_after},
         );
-        if ( $user_sort && $user_sort =~ /(-)?(\w+)/ ) {
-            my $reverse = $1 ? 1 : 0;
-            my $col = $2;
-            @rows = sort {
-# if id is not the sort col, we use id as secondary sort col.
-# in this case, no matter the main order, we always sort the id ascendingly
-                $reverse
-                  ? ( $b->{$col} <=> $a->{$col} || $a->{id} <=> $b->{id} )
-                  : ( $a->{$col} <=> $b->{$col} || $a->{id} <=> $b->{id} )
-            } @rows;
-        }
 
         my @issues;
         for my $row (@rows) {
@@ -113,10 +94,8 @@
                 project => $self->project,
                 %$row,
             );
-            $issue->load if $args{load_after_search} || $args{updated_after};
-            if ( !$args{updated_after} || $issue->updated->epoch >= $args{updated_after} ) {
-                push @issues, $issue;
-            }
+            $issue->load if $args{load_after_search};
+            push @issues, $issue;
         }
         $self->results( \@issues );
     }
@@ -143,14 +122,26 @@
 
 =over 4
 
-=item search ( can => 'all', q = 'foo', sort => '-modified' )
+=item search ( can => 'all', q = 'foo', sort => '-modified', limit => 1000 )
 
 do the search, the results is set to $self->results,
   which is an arrayref with Net::Google::Code::Issue as element.
 
-If a "sort" argument is specified, that will be passed to google code's issue list.
+If a "sort" argument is specified, that will be passed to google code's
+issue list.
 Generally, these are composed of "+" or "-" followed by a column name.
 
+limit => Num is to limit the results number.
+
+load_after_search => Bool is to state if we should call $issue->load after
+search
+
+modified_after => DateTime
+CAVEAT: this usually doesn't do what you want, be careful.
+In fact, it just filter issues by comparing the arg and the Modified column
+value, of which the date is not only rough, but also is just about the
+properity change: new comments without prop changes won't affect this value.
+
 return true if search is successful, false on the other hand.
 
 =item project

Modified: Net-Google-Code/trunk/lib/Net/Google/Code/Role/Pageable.pm
==============================================================================
--- Net-Google-Code/trunk/lib/Net/Google/Code/Role/Pageable.pm	(original)
+++ Net-Google-Code/trunk/lib/Net/Google/Code/Role/Pageable.pm	Wed Jun  3 10:54:23 2009
@@ -20,7 +20,7 @@
                 type     => SCALAR | UNDEF,
                 optional => 1,
             },
-            updated_after => {
+            modified_after => {
                 type => SCALAR | OBJECT | UNDEF,
                 optional => 1,
             },
@@ -31,7 +31,7 @@
     my $tree = $args{html};
     $tree = $self->html_tree( html => $tree ) unless blessed $tree;
 
-    my $updated_after = $args{updated_after};
+    my $modified_after = $args{modified_after};
 
     # assuming there's at most 20 columns
     my @titles;
@@ -65,72 +65,50 @@
     my $pagination = $tree->look_down( class => 'pagination' );
     return unless $pagination;
 
-    if ( my ( $start, $end, $total ) =
-        $pagination->as_text =~ /(\d+)\s+-\s+(\d+)\s+of\s+(\d+)/ )
-    {
-        # all the rows in a page
-        my @all_rows = $self->_rows(
-            html         => $tree,
-            titles       => \@titles,
-            label_column => $label_column,
-          );
-        my $found_number = scalar @all_rows;
-        my $max_discarded = 0;
-
+    if ( $pagination->as_text =~ /\d+\s+-\s+\d+\s+of\s+\d+/ ) {
         my $filter = sub {
-            return 1 unless $args{updated_after};
-
+            return 1 unless $args{modified_after};
             my $row = shift;
-
             if ( $row->{modified} ) {
                 my $epoch = UnixDate( $row->{modified}, '%o' );
-                if ( $epoch < $args{updated_after} ) {
-                    $max_discarded = $row->{id} if $max_discarded < $row->{id};
-                    return;
-                }
-                else {
+                if ( $epoch >= $args{modified_after} ) {
                     return 1;
                 }
             }
-            elsif ( $row->{id} < $max_discarded ) {
-   # no modified, means there is no comment, the updated date is the created
-   # since the id is less than the max discarded id, it's safe to discard it too
-                return;
-            }
-            else {
-                return 1;
-            }
+            return;
         };
 
-        push @rows, grep { $filter->($_) } @all_rows;
-
-        $total = $args{limit} if $args{limit} < $total;
-        while ( $found_number < $total ) {
+        # all the rows in a page
+        push @rows, grep { $filter->($_) } $self->_rows(
+            html         => $tree,
+            titles       => \@titles,
+            label_column => $label_column,
+          );
 
-            if ( $self->mech->follow_link( text_regex => qr/Next\s+/ ) ) {
+        while ( scalar @rows < $args{limit} ) {
+            my $next_link = $self->mech->find_link( text_regex => qr/Next\s+/ );
+            if ($next_link) {
+                $self->mech->get( $next_link->url );
                 if ( $self->mech->response->is_success ) {
-                    my @all_rows = $self->_rows(
+                    push @rows, grep { $filter->($_) } $self->_rows(
                         html         => $self->mech->content,
                         titles       => \@titles,
                         label_column => $label_column,
                     );
-                    $found_number += @all_rows;
-
-                    push @rows, grep { $filter->($_) } @all_rows;
                 }
                 else {
                     die "failed to follow 'Next' link";
                 }
             }
             else {
-                warn "didn't find enough rows";
                 last;
             }
         }
     }
 
     if ( scalar @rows > $args{limit} ) {
-        # this happens when limit is less than the 1st page's number 
+        # this happens when limit is less than the 1st page's number, so in
+        # some similar situations 
         return @rows[0 .. $args{limit}-1];
     }
     else {



More information about the Bps-public-commit mailing list