[Rt-commit] rt branch, 4.4/dashboard-charts, created. rt-4.4.0-111-gf75aa33

Shawn Moore shawn at bestpractical.com
Wed May 11 13:43:50 EDT 2016


The branch, 4.4/dashboard-charts has been created
        at  f75aa33c4f6be5724525931b40159595f162bc5a (commit)

- Log -----------------------------------------------------------------
commit 2544758ca70912b236d98777f0b3549beb7a166d
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Mon May 9 22:44:05 2016 +0000

    Make ChartStyle a scalar
    
        I can see no reason why this is treated as an array reference
        in /Search/Chart.html; it's treated as a scalar everywhere else,
        including in the UI.

diff --git a/share/html/Search/Chart.html b/share/html/Search/Chart.html
index 9dc0d78..57cd0ec 100644
--- a/share/html/Search/Chart.html
+++ b/share/html/Search/Chart.html
@@ -49,7 +49,7 @@
 my $default_value = {
     Query => 'id > 0',
     GroupBy => ['Status'],
-    ChartStyle => ['bar+table+sql'],
+    ChartStyle => 'bar+table+sql',
     ChartFunction => ['COUNT'],
 };
     
@@ -174,8 +174,8 @@ $m->callback( ARGSRef => \%ARGS, QueryArgsRef => \%query );
 </&>
 
 <&| /Widgets/TitleBox, title => loc('Picture'), class => "chart-picture" &>
-<input name="ChartStyle" type="hidden" value="<% $query{ChartStyle}[0] %>" />
-<label><% loc('Style') %>: <& Elements/SelectChartType, Default => $query{ChartStyle}[0] =~ /^(pie|bar|table)\b/ ? $1 : undef &></label>
+<input name="ChartStyle" type="hidden" value="<% $query{ChartStyle} %>" />
+<label><% loc('Style') %>: <& Elements/SelectChartType, Default => $query{ChartStyle} =~ /^(pie|bar|table)\b/ ? $1 : undef &></label>
 <span class="width">
 <label><% loc("Width") %>: <input type="text" name="Width" value="<% $query{'Width'} || q{} %>"> <% loc("px") %></label>
 </span>
@@ -184,10 +184,10 @@ $m->callback( ARGSRef => \%ARGS, QueryArgsRef => \%query );
   <label><% loc("Height") %>: <input type="text" name="Height" value="<% $query{'Height'} || q{} %>"> <% loc("px") %></label>
 </span>
 <div class="include-table">
-    <input type="checkbox" name="ChartStyleIncludeTable" <% $query{ChartStyle}[0] =~ /\btable\b/ ? 'checked="checked"' : '' |n %>> <% loc('Include data table') %>
+    <input type="checkbox" name="ChartStyleIncludeTable" <% $query{ChartStyle} =~ /\btable\b/ ? 'checked="checked"' : '' |n %>> <% loc('Include data table') %>
 </div>
 <div class="include-sql">
-    <input type="checkbox" name="ChartStyleIncludeSQL" <% $query{ChartStyle}[0] =~ /\bsql\b/ ? 'checked="checked"' : '' |n %>> <% loc('Include TicketSQL query') %>
+    <input type="checkbox" name="ChartStyleIncludeSQL" <% $query{ChartStyle} =~ /\bsql\b/ ? 'checked="checked"' : '' |n %>> <% loc('Include TicketSQL query') %>
 </div>
 </&>
 <script type="text/javascript">

commit 2da708cb86b3ebea47b172c718fa4ef939c18c10
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Tue May 10 20:53:53 2016 +0000

    Add test demonstrating not-fully-initialized saved charts
    
        Reported on issues ticket 31557.

diff --git a/t/web/saved_search_chart.t b/t/web/saved_search_chart.t
index 3737b51..e964a8f 100644
--- a/t/web/saved_search_chart.t
+++ b/t/web/saved_search_chart.t
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 
-use RT::Test no_plan => 1;
+use RT::Test tests => undef;
 my ( $url, $m ) = RT::Test->started_ok;
 use RT::Attribute;
 
@@ -151,3 +151,43 @@ is(
 );
 
 page_chart_link_has($m, $saved_search_ids[1]);
+
+diag "saving a chart without changing its config shows up on dashboards (I#31557)";
+{
+    $m->get_ok( $url . "/Search/Chart.html?Query=" . 'id!=-1' );
+    $m->submit_form(
+        form_name => 'SaveSearch',
+        fields    => {
+            SavedSearchDescription => 'chart without updates',
+            SavedSearchOwner       => $owner,
+        },
+        button => 'SavedSearchSave',
+    );
+
+    $m->form_name('SaveSearch');
+    @saved_search_ids =
+        $m->current_form->find_input('SavedSearchLoad')->possible_values;
+    shift @saved_search_ids; # first value is blank
+    my $chart_without_updates_id = $saved_search_ids[2];
+    ok($chart_without_updates_id, 'got a saved chart id');
+
+    my ($privacy, $user_id, $search_id) = $chart_without_updates_id =~ /^(RT::User-(\d+))-SavedSearch-(\d+)$/;
+    my $user = RT::User->new(RT->SystemUser);
+    $user->Load($user_id);
+    is($user->Name, 'root', 'loaded user');
+    my $currentuser = RT::CurrentUser->new($user);
+
+    my $search = RT::SavedSearch->new($currentuser);
+    $search->Load($privacy, $search_id);
+    is($search->Name, 'chart without updates', 'loaded search');
+    is($search->GetParameter('ChartStyle'), 'bar+table+sql', 'chart correctly initialized with default ChartStyle');
+    is($search->GetParameter('Height'), undef, 'no height by default');
+    is($search->GetParameter('Width'), undef, 'no width by default');
+    is($search->GetParameter('Query'), 'id!=-1', 'chart correctly initialized with Query');
+    is($search->GetParameter('SearchType'), 'Chart', 'chart correctly initialized with SearchType');
+    is_deeply($search->GetParameter('GroupBy'), ['Status'], 'chart correctly initialized with default GroupBy');
+    is_deeply($search->GetParameter('ChartFunction'), ['COUNT'], 'chart correctly initialized with default ChartFunction');
+}
+
+undef $m;
+done_testing;

commit f75aa33c4f6be5724525931b40159595f162bc5a
Author: Shawn M Moore <shawn at bestpractical.com>
Date:   Mon May 9 22:55:23 2016 +0000

    Fix charts sometimes not showing up on dashboards
    
        The problem was charts that had not been updated using the "Update
        Chart" button were incompletely written into Saved Charts: their
        default values (ChartStyle=bar+table+sql, etc) were not saved
        into the saved chart. This is ordinarily not a problem for saved
        charts because those defaults would be set up when you loaded a
        saved chart using the normal UI. However, dashboards don't have the
        same initialization order, so the $ChartStyle would be empty, causing
        no rendering to happen.
    
        This regression was exarcerbated by
        aedc560e55bea689e083214f33912abb59ac0617 which introduces a
        conditional on the uninitialized $ChartStyle. Previously,
        <img src="…/Search/Chart"> was included unconditionally, which
        worked because /Search/Chart itself provides a default $ChartStyle.
    
        This commit addresses the issue by unconditionally saving the
        default values (e.g. ChartStyle=bar+table+sql) into the Saved Chart
        record, fixing all codepaths including dashboards.
    
    Fixes: I#31557

diff --git a/share/html/Search/Chart.html b/share/html/Search/Chart.html
index 57cd0ec..284e4b3 100644
--- a/share/html/Search/Chart.html
+++ b/share/html/Search/Chart.html
@@ -63,7 +63,11 @@ my $saved_search = $m->comp( '/Widgets/SavedSearch:new',
     SearchFields => [@search_fields],
 );
 
-my @actions = $m->comp( '/Widgets/SavedSearch:process', args => \%ARGS, self => $saved_search );
+my @actions = $m->comp( '/Widgets/SavedSearch:process',
+    args     => \%ARGS,
+    defaults => $default_value,
+    self     => $saved_search,
+);
 
 my %query;
 
diff --git a/share/html/Widgets/SavedSearch b/share/html/Widgets/SavedSearch
index d029203..022ec2d 100644
--- a/share/html/Widgets/SavedSearch
+++ b/share/html/Widgets/SavedSearch
@@ -115,6 +115,9 @@ if ( $args->{SavedSearchSave} ) {
     }
     else {
         # new saved search
+
+        $SearchParams->{$_} //= $defaults->{$_} for @{$self->{SearchFields}};
+
         my $saved_search = RT::SavedSearch->new( $session{'CurrentUser'} );
         my ( $ok, $search_msg ) = $saved_search->Save(
             Privacy      => $args->{'SavedSearchOwner'},
@@ -151,6 +154,7 @@ return @actions;
 <%ARGS>
 $self
 $args
+$defaults => {}
 </%ARGS>
 
 </%method>

-----------------------------------------------------------------------


More information about the rt-commit mailing list