Fix issue with leaderboard position due to wrong calculation of trend

lib/Pear/LocalLoop/Controller/Api/Stats.pm

@@ -69,12 +69,12 @@ sub post_leaderboards {
   my $today_values = $today_board->values->search(
     {},
     {
-      order_by => { -desc => 'me.value' },
+      order_by => { -asc => 'me.position' },
       columns => [
         qw/
           me.value
           me.trend
-          me.user_id
+          me.position
         /,
         { display_name => 'customer.display_name' },
       ],
@@ -85,15 +85,12 @@ sub post_leaderboards {
 
   my @leaderboard_array = $today_values->all;
 
-  my $current_user_index = first { $leaderboard_array[$_]->{user_id} == $c->stash->{api_user}->id } 0..$#leaderboard_array;
+  my $current_user_position = $today_values->find({ user_id => $c->stash->{api_user}->id });
-
-  # Dont leak user id's
-  map { delete $_->{user_id} } @leaderboard_array;
 
   return $c->render( json => {
     success => Mojo::JSON->true,
     leaderboard => [ @leaderboard_array ],
-    user_position => $current_user_index,
+    user_position => defined $current_user_position ? $current_user_position->{position} : 0,
   });
 }
 

lib/Pear/LocalLoop/Schema/Result/Leaderboard.pm

@@ -72,6 +72,48 @@ sub _get_customer_rs {
   });
 }
 
+sub _set_position_and_trend {
+  my ( $self, @leaderboard ) = @_;
+
+  # Sort numerically descending
+  @leaderboard = sort { $b->{value} <=> $a->{value} } @leaderboard;
+
+  my $position = 0;
+
+  my $previous_board = $self->get_latest;
+
+  if ( defined $previous_board ) {
+    $previous_board = $previous_board->values;
+  }
+
+  for my $lb_val ( @leaderboard ) {
+    $position++;
+    $lb_val->{position} = $position;
+
+    my $previous_value;
+
+    if ( defined $previous_board ) {
+      $previous_value = $previous_board->find({ user_id => $lb_val->{user_id} });
+    }
+
+    my $trend;
+
+    if ( ! defined $previous_value ) {
+      $trend = 0;
+    } elsif ( $previous_value->position > $position ) {
+      $trend = -1;
+    } elsif ( $previous_value->position < $position ) {
+      $trend = 1;
+    } else {
+      $trend = 0;
+    }
+
+    $lb_val->{trend} = $trend;
+  }
+
+  return @leaderboard;
+}
+
 sub _create_total_set {
   my ( $self, $start, $end ) = @_;
 
@@ -79,42 +121,19 @@ sub _create_total_set {
 
   my @leaderboard;
 
-  my $previous_board = $self->get_latest;
-
-  if ( defined $previous_board ) {
-    $previous_board = $previous_board->values;
-  }
-  
   while ( my $user_result = $user_rs->next ) {
     my $transaction_rs = $user_result->transactions->search_between( $start, $end );
 
     my $transaction_sum = $transaction_rs->get_column('value')->sum;
 
-    my $previous_value;
-
-    if ( defined $previous_board ) {
-      $previous_value = $previous_board->find({ user_id => $user_result->id });
-    }
-
-    my $trend;
-
-    if ( ! defined $previous_value ) {
-      $trend = 0;
-    } elsif ( $previous_value->value > $transaction_sum ) {
-      $trend = -1;
-    } elsif ( $previous_value->value < $transaction_sum ) {
-      $trend = 1;
-    } else {
-      $trend = 0;
-    }
-
     push @leaderboard, {
       user_id => $user_result->id,
       value => $transaction_sum || 0,
-      trend => $trend,
     };
   }
 
+  @leaderboard = $self->_set_position_and_trend(@leaderboard);
+
   $self->create_related(
     'sets',
     {
@@ -133,42 +152,19 @@ sub _create_count_set {
 
   my @leaderboard;
 
-  my $previous_board = $self->get_latest;
-
-  if ( defined $previous_board ) {
-    $previous_board = $previous_board->values;
-  }
-
   while ( my $user_result = $user_rs->next ) {
     my $transaction_rs = $user_result->transactions->search_between( $start, $end );
 
     my $transaction_count = $transaction_rs->count;
 
-    my $previous_value;
-
-    if ( defined $previous_board ) {
-      $previous_value = $previous_board->find({ user_id => $user_result->id });
-    }
-
-    my $trend;
-
-    if ( ! defined $previous_value ) {
-      $trend = 0;
-    } elsif ( $previous_value->value > $transaction_count ) {
-      $trend = -1;
-    } elsif ( $previous_value->value < $transaction_count ) {
-      $trend = 1;
-    } else {
-      $trend = 0;
-    }
-
     push @leaderboard, {
       user_id => $user_result->id,
       value => $transaction_count || 0,
-      trend => $trend,
     };
   }
 
+  @leaderboard = $self->_set_position_and_trend(@leaderboard);
+
   $self->create_related(
     'sets',
     {
@@ -187,41 +183,19 @@ sub _create_total_all_time {
  
   my @leaderboard;
 
-  my $previous_board = $self->get_latest;
-
-  if ( defined $previous_board ) {
-    $previous_board = $previous_board->values;
-  }
-
   while ( my $user_result = $user_rs->next ) {
     my $transaction_rs = $user_result->transactions;
 
     my $transaction_sum = $transaction_rs->get_column('value')->sum;
 
-    my $previous_value;
-
-    if ( defined $previous_board ) {
-      $previous_value = $previous_board->find({ user_id => $user_result->id });
-    }
-
-    my $trend;
-
-    if ( ! defined $previous_value ) {
-      $trend = 0;
-    } elsif ( $previous_value->value > $transaction_sum ) {
-      $trend = -1;
-    } elsif ( $previous_value->value < $transaction_sum ) {
-      $trend = 1;
-    } else {
-      $trend = 0;
-    }
-
     push @leaderboard, {
       user_id => $user_result->id,
       value => $transaction_sum || 0,
     };
   }
 
+  @leaderboard = $self->_set_position_and_trend(@leaderboard);
+
   $self->create_related(
     'sets',
     {
@@ -240,42 +214,19 @@ sub _create_count_all_time {
 
   my @leaderboard;
 
-  my $previous_board = $self->get_latest;
-
-  if ( defined $previous_board ) {
-    $previous_board = $previous_board->values;
-  }
-
   while ( my $user_result = $user_rs->next ) {
     my $transaction_rs = $user_result->transactions;
 
     my $transaction_count = $transaction_rs->count;
 
-    my $previous_value;
-
-    if ( defined $previous_board ) {
-      $previous_value = $previous_board->find({ user_id => $user_result->id });
-    }
-
-    my $trend;
-
-    if ( ! defined $previous_value ) {
-      $trend = 0;
-    } elsif ( $previous_value->value > $transaction_count ) {
-      $trend = -1;
-    } elsif ( $previous_value->value < $transaction_count ) {
-      $trend = 1;
-    } else {
-      $trend = 0;
-    }
-
     push @leaderboard, {
       user_id => $user_result->id,
       value => $transaction_count || 0,
-      trend => $trend,
     };
   }
 
+  @leaderboard = $self->_set_position_and_trend(@leaderboard);
+
   $self->create_related(
     'sets',
     {

lib/Pear/LocalLoop/Schema/Result/LeaderboardValue.pm

@@ -23,6 +23,10 @@ __PACKAGE__->add_columns(
     is_foreign_key => 1,
     is_nullable => 0,
   },
+  "position" => {
+    data_type => "integer",
+    is_nullable => 0,
+  },
   "value" => {
     data_type => "decimal",
     size => [ 16, 2 ],

t/api/stats_leaderboards.t

@@ -174,12 +174,12 @@ test_leaderboard(
   'daily_total',
   $now,
   [
-    { display_name => 'Test User4', value => 9, trend => 1 },
+    { display_name => 'Test User4', value => 9, position => 1, trend => -1 },
-    { display_name => 'Test User3', value => 5, trend => 0 },
+    { display_name => 'Test User3', value => 5, position => 2, trend => 0 },
-    { display_name => 'Test User2', value => 3, trend => 1 },
+    { display_name => 'Test User2', value => 3, position => 3, trend => -1 },
-    { display_name => 'Test User1', value => 1, trend => -1},
+    { display_name => 'Test User1', value => 1, position => 4, trend => 1},
   ],
-  3
+  4
 );
 
 test_leaderboard(
@@ -187,12 +187,12 @@ test_leaderboard(
   'daily_count',
   $now,
   [
-    { display_name => 'Test User1', value => 1, trend => 0 },
+    { display_name => 'Test User1', value => 1, position => 1, trend => 0 },
-    { display_name => 'Test User2', value => 1, trend => 0 },
+    { display_name => 'Test User2', value => 1, position => 2, trend => 0 },
-    { display_name => 'Test User3', value => 1, trend => 0 },
+    { display_name => 'Test User3', value => 1, position => 3, trend => 0 },
-    { display_name => 'Test User4', value => 1, trend => 0 },
+    { display_name => 'Test User4', value => 1, position => 4, trend => 0 },
   ],
-  0
+  1
 );
 
 done_testing;

t/schema/leaderboard_trend.t

@@ -161,7 +161,7 @@ sub test_leaderboard {
       {},
       {
         order_by => { -desc => 'value' },
-        columns => [ qw/ user_id value trend / ],
+        columns => [ qw/ user_id value trend position / ],
       },
     );
     $today_values->result_class( 'DBIx::Class::ResultClass::HashRefInflator' );
@@ -177,10 +177,10 @@ test_leaderboard(
   'daily_total',
   $now,
   [
-    { user_id => 4, value => 9, trend => 1 },
+    { user_id => 4, value => 9, trend => -1, position => 1},
-    { user_id => 3, value => 5, trend => 0 },
+    { user_id => 3, value => 5, trend => 0,  position => 2},
-    { user_id => 2, value => 3, trend => 1 },
+    { user_id => 2, value => 3, trend => -1, position => 3},
-    { user_id => 1, value => 1, trend => -1},
+    { user_id => 1, value => 1, trend => 1,  position => 4},
   ]
 );
 
@@ -189,10 +189,10 @@ test_leaderboard(
   'daily_count',
   $now,
   [
-    { user_id => 1, value => 1, trend => 0 },
+    { user_id => 1, value => 1, trend => 0, position => 1 },
-    { user_id => 2, value => 1, trend => 0 },
+    { user_id => 2, value => 1, trend => 0, position => 2 },
-    { user_id => 3, value => 1, trend => 0 },
+    { user_id => 3, value => 1, trend => 0, position => 3 },
-    { user_id => 4, value => 1, trend => 0 },
+    { user_id => 4, value => 1, trend => 0, position => 4 },
   ]
 );
 

t/schema/resultset_leaderboard.t

@@ -131,7 +131,7 @@ sub test_leaderboard {
       {},
       {
         order_by => { -desc => 'value' },
-        columns => [ qw/ user_id value / ],
+        columns => [ qw/ user_id value position / ],
       },
     );
     $today_values->result_class( 'DBIx::Class::ResultClass::HashRefInflator' );
@@ -145,10 +145,10 @@ test_leaderboard(
   'daily_total',
   $now,
   [
-    { user_id => 4, value => 95 },
+    { user_id => 4, value => 95, position => 1 },
-    { user_id => 3, value => 85 },
+    { user_id => 3, value => 85, position => 2 },
-    { user_id => 2, value => 75 },
+    { user_id => 2, value => 75, position => 3 },
-    { user_id => 1, value => 65 },
+    { user_id => 1, value => 65, position => 4 },
   ]
 );
 
@@ -157,10 +157,10 @@ test_leaderboard(
   'daily_count',
   $now,
   [
-    { user_id => 1, value => 10 },
+    { user_id => 1, value => 10, position => 1 },
-    { user_id => 2, value => 10 },
+    { user_id => 2, value => 10, position => 2 },
-    { user_id => 3, value => 10 },
+    { user_id => 3, value => 10, position => 3 },
-    { user_id => 4, value => 10 },
+    { user_id => 4, value => 10, position => 4 },
   ]
 );
 
@@ -169,10 +169,10 @@ test_leaderboard(
   'weekly_total',
   $now->clone->subtract( days => 7 ),
   [
-    { user_id => 4, value => 195 },
+    { user_id => 4, value => 195, position => 1 },
-    { user_id => 3, value => 185 },
+    { user_id => 3, value => 185, position => 2 },
-    { user_id => 2, value => 175 },
+    { user_id => 2, value => 175, position => 3 },
-    { user_id => 1, value => 165 },
+    { user_id => 1, value => 165, position => 4 },
   ]
 );
 
@@ -181,10 +181,10 @@ test_leaderboard(
   'weekly_count',
   $now->clone->subtract( days => 7 ),
   [
-    { user_id => 1, value => 10 },
+    { user_id => 1, value => 10, position => 1 },
-    { user_id => 2, value => 10 },
+    { user_id => 2, value => 10, position => 2 },
-    { user_id => 3, value => 10 },
+    { user_id => 3, value => 10, position => 3 },
-    { user_id => 4, value => 10 },
+    { user_id => 4, value => 10, position => 4 },
   ]
 );
 
@@ -193,10 +193,10 @@ test_leaderboard(
   'monthly_total',
   $now->clone->subtract( months => 1 ),
   [
-    { user_id => 4, value => 490 },
+    { user_id => 4, value => 490, position => 1 },
-    { user_id => 3, value => 470 },
+    { user_id => 3, value => 470, position => 2 },
-    { user_id => 2, value => 450 },
+    { user_id => 2, value => 450, position => 3 },
-    { user_id => 1, value => 430 },
+    { user_id => 1, value => 430, position => 4 },
   ]
 );
 
@@ -205,10 +205,10 @@ test_leaderboard(
   'monthly_count',
   $now->clone->subtract( months => 1 ),
   [
-    { user_id => 1, value => 20 },
+    { user_id => 1, value => 20, position => 1 },
-    { user_id => 2, value => 20 },
+    { user_id => 2, value => 20, position => 2 },
-    { user_id => 3, value => 20 },
+    { user_id => 3, value => 20, position => 3 },
-    { user_id => 4, value => 20 },
+    { user_id => 4, value => 20, position => 4 },
   ]
 );
 
@@ -217,10 +217,10 @@ test_leaderboard(
   'all_time_total',
   $now,
   [
-    { user_id => 4, value => 980 },
+    { user_id => 4, value => 980, position => 1 },
-    { user_id => 3, value => 940 },
+    { user_id => 3, value => 940, position => 2 },
-    { user_id => 2, value => 900 },
+    { user_id => 2, value => 900, position => 3 },
-    { user_id => 1, value => 860 },
+    { user_id => 1, value => 860, position => 4 },
   ]
 );
 
@@ -229,10 +229,10 @@ test_leaderboard(
   'all_time_count',
   $now,
   [
-    { user_id => 1, value => 40 },
+    { user_id => 1, value => 40, position => 1 },
-    { user_id => 2, value => 40 },
+    { user_id => 2, value => 40, position => 2 },
-    { user_id => 3, value => 40 },
+    { user_id => 3, value => 40, position => 3 },
-    { user_id => 4, value => 40 },
+    { user_id => 4, value => 40, position => 4 },
   ]
 );
 
@@ -250,16 +250,16 @@ subtest 'get_latest' => sub {
     {},
     {
       order_by => { -desc => 'value' },
-      columns => [ qw/ user_id value / ],
+      columns => [ qw/ user_id value position / ],
     },
   );
   $today_values->result_class( 'DBIx::Class::ResultClass::HashRefInflator' );
 
   my $expected = [
-    { user_id => 4, value => 95 },
+    { user_id => 4, value => 95, position => 1 },
-    { user_id => 3, value => 85 },
+    { user_id => 3, value => 85, position => 2 },
-    { user_id => 2, value => 75 },
+    { user_id => 2, value => 75, position => 3 },
-    { user_id => 1, value => 65 },
+    { user_id => 1, value => 65, position => 4 },
   ];
 
   is_deeply [ $today_values->all ], $expected, 'array as expected';