From 559e88d9e6839f01363de655a3d429bc03ea7fd5 Mon Sep 17 00:00:00 2001 From: Tom Bloor Date: Tue, 13 Jun 2017 23:15:10 +0100 Subject: [PATCH] Fix issue with leaderboard position due to wrong calculation of trend --- lib/Pear/LocalLoop/Controller/Api/Stats.pm | 11 +- .../LocalLoop/Schema/Result/Leaderboard.pm | 149 ++++++------------ .../Schema/Result/LeaderboardValue.pm | 4 + t/api/stats_leaderboards.t | 20 +-- t/schema/leaderboard_trend.t | 18 +-- t/schema/resultset_leaderboard.t | 76 ++++----- 6 files changed, 115 insertions(+), 163 deletions(-) diff --git a/lib/Pear/LocalLoop/Controller/Api/Stats.pm b/lib/Pear/LocalLoop/Controller/Api/Stats.pm index 357562b..f0da881 100644 --- a/lib/Pear/LocalLoop/Controller/Api/Stats.pm +++ b/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; - - # Dont leak user id's - map { delete $_->{user_id} } @leaderboard_array; + my $current_user_position = $today_values->find({ user_id => $c->stash->{api_user}->id }); 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, }); } diff --git a/lib/Pear/LocalLoop/Schema/Result/Leaderboard.pm b/lib/Pear/LocalLoop/Schema/Result/Leaderboard.pm index 1105806..e296d3f 100644 --- a/lib/Pear/LocalLoop/Schema/Result/Leaderboard.pm +++ b/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', { diff --git a/lib/Pear/LocalLoop/Schema/Result/LeaderboardValue.pm b/lib/Pear/LocalLoop/Schema/Result/LeaderboardValue.pm index 4beaed4..00833f4 100644 --- a/lib/Pear/LocalLoop/Schema/Result/LeaderboardValue.pm +++ b/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 ], diff --git a/t/api/stats_leaderboards.t b/t/api/stats_leaderboards.t index 792c24d..11f366e 100644 --- a/t/api/stats_leaderboards.t +++ b/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 User3', value => 5, trend => 0 }, - { display_name => 'Test User2', value => 3, trend => 1 }, - { display_name => 'Test User1', value => 1, trend => -1}, + { display_name => 'Test User4', value => 9, position => 1, trend => -1 }, + { display_name => 'Test User3', value => 5, position => 2, trend => 0 }, + { display_name => 'Test User2', value => 3, position => 3, 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 User2', value => 1, trend => 0 }, - { display_name => 'Test User3', value => 1, trend => 0 }, - { display_name => 'Test User4', value => 1, trend => 0 }, + { display_name => 'Test User1', value => 1, position => 1, trend => 0 }, + { display_name => 'Test User2', value => 1, position => 2, trend => 0 }, + { display_name => 'Test User3', value => 1, position => 3, trend => 0 }, + { display_name => 'Test User4', value => 1, position => 4, trend => 0 }, ], - 0 + 1 ); done_testing; diff --git a/t/schema/leaderboard_trend.t b/t/schema/leaderboard_trend.t index 63fff33..593b0c2 100644 --- a/t/schema/leaderboard_trend.t +++ b/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 => 3, value => 5, trend => 0 }, - { user_id => 2, value => 3, trend => 1 }, - { user_id => 1, value => 1, trend => -1}, + { user_id => 4, value => 9, trend => -1, position => 1}, + { user_id => 3, value => 5, trend => 0, position => 2}, + { user_id => 2, value => 3, trend => -1, position => 3}, + { 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 => 2, value => 1, trend => 0 }, - { user_id => 3, value => 1, trend => 0 }, - { user_id => 4, value => 1, trend => 0 }, + { user_id => 1, value => 1, trend => 0, position => 1 }, + { user_id => 2, value => 1, trend => 0, position => 2 }, + { user_id => 3, value => 1, trend => 0, position => 3 }, + { user_id => 4, value => 1, trend => 0, position => 4 }, ] ); diff --git a/t/schema/resultset_leaderboard.t b/t/schema/resultset_leaderboard.t index d257442..77f8c4f 100644 --- a/t/schema/resultset_leaderboard.t +++ b/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 => 3, value => 85 }, - { user_id => 2, value => 75 }, - { user_id => 1, value => 65 }, + { user_id => 4, value => 95, position => 1 }, + { user_id => 3, value => 85, position => 2 }, + { user_id => 2, value => 75, position => 3 }, + { user_id => 1, value => 65, position => 4 }, ] ); @@ -157,10 +157,10 @@ test_leaderboard( 'daily_count', $now, [ - { user_id => 1, value => 10 }, - { user_id => 2, value => 10 }, - { user_id => 3, value => 10 }, - { user_id => 4, value => 10 }, + { user_id => 1, value => 10, position => 1 }, + { user_id => 2, value => 10, position => 2 }, + { user_id => 3, value => 10, position => 3 }, + { 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 => 3, value => 185 }, - { user_id => 2, value => 175 }, - { user_id => 1, value => 165 }, + { user_id => 4, value => 195, position => 1 }, + { user_id => 3, value => 185, position => 2 }, + { user_id => 2, value => 175, position => 3 }, + { 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 => 2, value => 10 }, - { user_id => 3, value => 10 }, - { user_id => 4, value => 10 }, + { user_id => 1, value => 10, position => 1 }, + { user_id => 2, value => 10, position => 2 }, + { user_id => 3, value => 10, position => 3 }, + { 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 => 3, value => 470 }, - { user_id => 2, value => 450 }, - { user_id => 1, value => 430 }, + { user_id => 4, value => 490, position => 1 }, + { user_id => 3, value => 470, position => 2 }, + { user_id => 2, value => 450, position => 3 }, + { 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 => 2, value => 20 }, - { user_id => 3, value => 20 }, - { user_id => 4, value => 20 }, + { user_id => 1, value => 20, position => 1 }, + { user_id => 2, value => 20, position => 2 }, + { user_id => 3, value => 20, position => 3 }, + { user_id => 4, value => 20, position => 4 }, ] ); @@ -217,10 +217,10 @@ test_leaderboard( 'all_time_total', $now, [ - { user_id => 4, value => 980 }, - { user_id => 3, value => 940 }, - { user_id => 2, value => 900 }, - { user_id => 1, value => 860 }, + { user_id => 4, value => 980, position => 1 }, + { user_id => 3, value => 940, position => 2 }, + { user_id => 2, value => 900, position => 3 }, + { user_id => 1, value => 860, position => 4 }, ] ); @@ -229,10 +229,10 @@ test_leaderboard( 'all_time_count', $now, [ - { user_id => 1, value => 40 }, - { user_id => 2, value => 40 }, - { user_id => 3, value => 40 }, - { user_id => 4, value => 40 }, + { user_id => 1, value => 40, position => 1 }, + { user_id => 2, value => 40, position => 2 }, + { user_id => 3, value => 40, position => 3 }, + { 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 => 3, value => 85 }, - { user_id => 2, value => 75 }, - { user_id => 1, value => 65 }, + { user_id => 4, value => 95, position => 1 }, + { user_id => 3, value => 85, position => 2 }, + { user_id => 2, value => 75, position => 3 }, + { user_id => 1, value => 65, position => 4 }, ]; is_deeply [ $today_values->all ], $expected, 'array as expected';