Imported disabled graph notice

CHANGELOG.md

@@ -20,6 +20,7 @@ All notable changes to this project will be documented in this file.
 - Testing framework for `/assets`
 
 ### Removed
+- Stop returning imported stats in timeseries queries for intervals shorter than `day`
 - Deprecate `ECTO_IPV6` and `ECTO_CH_IPV6` env vars in CE plausible/analytics#4245
 - Remove support for importing data from no longer available Universal Analytics
 - Soft-deprecate `DATABASE_SOCKET_DIR` plausible/analytics#4202

assets/js/dashboard/stats/graph/visitor-graph.js

@@ -14,6 +14,7 @@ import { isComparisonEnabled } from '../../query-time-periods';
 import LineGraphWithRouter from './line-graph';
 import { useQueryContext } from '../../query-context';
 import { useSiteContext } from '../../site-context';
+import { ExclamationCircleIcon } from '@heroicons/react/24/outline'
 
 function fetchTopStats(site, query) {
   const q = { ...query }
@@ -135,6 +136,22 @@ export default function VisitorGraph({ updateImportedDataInView }) {
     }
   }
 
+  function importedSwitchVisible() {
+    return !!topStatData?.with_imported_switch && topStatData?.with_imported_switch.visible
+  }
+
+  function renderImportedIntervalUnsupportedWarning() {
+    const unsupportedInterval = ['hour', 'minute'].includes(getCurrentInterval(site, query))
+    const showingImported = importedSwitchVisible() && query.with_imported === true
+
+    return (
+      <FadeIn show={showingImported && unsupportedInterval} className="h-6 mr-1">
+        <span tooltip={"Inteval is too short to graph imported data"}>
+          <ExclamationCircleIcon className="w-6 h-6 text-gray-700 dark:text-gray-300" />
+        </span>
+      </FadeIn>
+    )
+  }
 
   return (
     <div className={"relative w-full mt-2 bg-white rounded shadow-xl dark:bg-gray-825"}>
@@ -150,9 +167,10 @@ export default function VisitorGraph({ updateImportedDataInView }) {
         <div className="relative px-2">
           {graphRefreshing && renderLoader()}
           <div className="absolute right-4 -top-8 py-1 flex items-center">
+            {renderImportedIntervalUnsupportedWarning()}
             {!isRealtime && <StatsExport />}
             <SamplingNotice samplePercent={topStatData} />
-            {!!topStatData?.with_imported_switch && topStatData?.with_imported_switch.visible &&
+            {importedSwitchVisible() &&
               <WithImportedSwitch
                 tooltipMessage={topStatData.with_imported_switch.tooltip_msg}
                 disabled={!topStatData.with_imported_switch.togglable}

lib/plausible/stats/query.ex

@@ -148,7 +148,7 @@ defmodule Plausible.Stats.Query do
         {:error, :unsupported_query}
 
       "time:minute" in query.dimensions or "time:hour" in query.dimensions ->
-        {:error, :unsupported_query}
+        {:error, :unsupported_interval}
 
       Date.after?(query.date_range.first, query.latest_import_end_date) ->
         {:error, :out_of_range}

lib/plausible/stats/query_result.ex

@@ -68,6 +68,8 @@ defmodule Plausible.Stats.QueryResult do
   @imports_unsupported_query_warning "Imported stats are not included in the results because query parameters are not supported. " <>
                                        "For more information, see: https://plausible.io/docs/stats-api#filtering-imported-stats"
 
+  @imports_unsupported_interval_warning "Imported stats are not included because the time dimension (i.e. the interval) is too short."
+
   defp meta(query) do
     %{
       imports_included: if(query.include.imports, do: query.include_imported, else: nil),
@@ -76,6 +78,7 @@ defmodule Plausible.Stats.QueryResult do
       imports_warning:
         case query.skip_imported_reason do
           :unsupported_query -> @imports_unsupported_query_warning
+          :unsupported_interval -> @imports_unsupported_interval_warning
           _ -> nil
         end,
       time_labels:

lib/plausible/stats/timeseries.ex

@@ -38,11 +38,17 @@ defmodule Plausible.Stats.Timeseries do
 
     q = SQL.QueryBuilder.build(query_with_metrics, site)
 
-    q
-    |> ClickhouseRepo.all(query: query)
-    |> QueryResult.from(site, query_with_metrics)
-    |> build_timeseries_result(query_with_metrics, currency)
-    |> transform_keys(%{group_conversion_rate: :conversion_rate})
+    query_result =
+      q
+      |> ClickhouseRepo.all(query: query)
+      |> QueryResult.from(site, query_with_metrics)
+
+    timeseries_result =
+      query_result
+      |> build_timeseries_result(query_with_metrics, currency)
+      |> transform_keys(%{group_conversion_rate: :conversion_rate})
+
+    {timeseries_result, query_result.meta}
   end
 
   defp time_dimension(query), do: Map.fetch!(@time_dimension, query.interval)

lib/plausible_web/controllers/api/external_stats_controller.ex

@@ -273,8 +273,13 @@ defmodule PlausibleWeb.Api.ExternalStatsController do
          :ok <- validate_filters(site, query.filters),
          {:ok, metrics} <- parse_and_validate_metrics(params, query),
          :ok <- ensure_custom_props_access(site, query) do
-      graph = Plausible.Stats.timeseries(site, query, metrics)
-      payload = maybe_add_warning(%{results: graph}, query)
+      {results, meta} = Plausible.Stats.timeseries(site, query, metrics)
+
+      payload =
+        case meta[:imports_warning] do
+          nil -> %{results: results}
+          warning -> %{results: results, warning: warning}
+        end
 
       json(conn, payload)
     else

lib/plausible_web/controllers/api/stats_controller.ex

@@ -107,17 +107,17 @@ defmodule PlausibleWeb.Api.StatsController do
          params <- realtime_period_to_30m(params),
          query = Query.from(site, params, debug_metadata(conn)),
          {:ok, metric} <- parse_and_validate_graph_metric(params, query) do
-      timeseries_result = Stats.timeseries(site, query, [metric])
+      {timeseries_result, _meta} = Stats.timeseries(site, query, [metric])
 
       comparison_opts = parse_comparison_opts(params)
 
-      {comparison_query, comparison_result} =
+      comparison_result =
         case Comparisons.compare(site, query, params["comparison"], comparison_opts) do
           {:ok, comparison_query} ->
-            {comparison_query, Stats.timeseries(site, comparison_query, [metric])}
+            Stats.timeseries(site, comparison_query, [metric]) |> elem(0)
 
           {:error, :not_supported} ->
-            {nil, nil}
+            nil
         end
 
       labels = label_timeseries(timeseries_result, comparison_result)
@@ -131,8 +131,6 @@ defmodule PlausibleWeb.Api.StatsController do
         comparison_plot: comparison_result && plot_timeseries(comparison_result, metric),
         comparison_labels: comparison_result && label_timeseries(comparison_result, nil),
         present_index: present_index,
-        includes_imported: includes_imported?(query, comparison_query),
-        imports_exist: site.complete_import_ids != [],
         full_intervals: full_intervals
       })
     else

lib/plausible_web/controllers/stats_controller.ex

@@ -173,6 +173,7 @@ defmodule PlausibleWeb.StatsController do
     prepend_column_headers = fn data -> [column_headers | data] end
 
     Plausible.Stats.timeseries(site, query, metrics)
+    |> elem(0)
     |> Enum.map(map_bucket_to_row)
     |> prepend_column_headers.()
     |> NimbleCSV.RFC4180.dump_to_iodata()

test/plausible_web/controllers/api/external_stats_controller/query_imported_test.exs

@@ -583,10 +583,10 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryImportedTest do
              ]
 
       refute json_response(conn, 200)["meta"]["imports_included"]
-      assert json_response(conn, 200)["meta"]["imports_skip_reason"] == "unsupported_query"
+      assert json_response(conn, 200)["meta"]["imports_skip_reason"] == "unsupported_interval"
 
       assert json_response(conn, 200)["meta"]["imports_warning"] =~
-               "Imported stats are not included in the results because query parameters are not supported."
+               "Imported stats are not included because the time dimension (i.e. the interval) is too short."
     end
 
     test "adds a warning when query params are not supported for imported data", %{

test/plausible_web/controllers/api/external_stats_controller/timeseries_test.exs

@@ -1611,6 +1611,34 @@ defmodule PlausibleWeb.Api.ExternalStatsController.TimeseriesTest do
                "Imported stats are not included in the results because query parameters are not supported."
     end
 
+    test "is not included for a day period and an appropriate warning is returned", %{
+      conn: conn,
+      site: site
+    } do
+      site_import = insert(:site_import, site: site)
+
+      populate_stats(site, site_import.id, [
+        build(:imported_visitors, visitors: 1, date: ~D[2021-01-01])
+      ])
+
+      conn =
+        conn
+        |> get("/api/v1/stats/timeseries", %{
+          "site_id" => site.domain,
+          "period" => "day",
+          "metrics" => "visitors",
+          "date" => "2021-01-01",
+          "with_imported" => "true"
+        })
+
+      assert %{"results" => results, "warning" => warning} = json_response(conn, 200)
+
+      Enum.each(results, &assert(&1["visitors"] == 0))
+
+      assert warning ==
+               "Imported stats are not included because the time dimension (i.e. the interval) is too short."
+    end
+
     test "does not add a warning when there are no site imports", %{conn: conn, site: site} do
       populate_stats(site, [
         build(:pageview,

test/plausible_web/controllers/api/stats_controller/imported_test.exs

@@ -80,8 +80,7 @@ defmodule PlausibleWeb.Api.StatsController.ImportedTest do
             "/api/stats/#{site.domain}/main-graph?period=month&date=2021-01-01&with_imported=true"
           )
 
-        assert %{"plot" => plot, "imports_exist" => true} =
-                 json_response(conn, 200)
+        assert %{"plot" => plot} = json_response(conn, 200)
 
         assert Enum.count(plot) == 31
         assert List.first(plot) == 2
@@ -129,8 +128,7 @@ defmodule PlausibleWeb.Api.StatsController.ImportedTest do
             "/api/stats/#{site.domain}/main-graph?period=month&date=2021-01-01&with_imported=true&interval=week"
           )
 
-        assert %{"plot" => plot, "imports_exist" => true} =
-                 json_response(conn, 200)
+        assert %{"plot" => plot} = json_response(conn, 200)
 
         assert Enum.count(plot) == 5
         assert List.first(plot) == 2

test/plausible_web/controllers/api/stats_controller/main_graph_test.exs

@@ -54,10 +54,10 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
         )
 
       zeroes = List.duplicate(0, 30)
-      assert %{"plot" => ^zeroes, "includes_imported" => false} = json_response(conn, 200)
+      assert %{"plot" => ^zeroes} = json_response(conn, 200)
     end
 
-    test "displays visitors for a day with imported data (not included for hourly)", %{
+    test "imported data is not included for hourly interval", %{
       conn: conn,
       site: site
     } do
@@ -74,8 +74,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
           "/api/stats/#{site.domain}/main-graph?period=day&date=2021-01-01&with_imported=true"
         )
 
-      assert %{"plot" => plot, "imports_exist" => true, "includes_imported" => true} =
-               json_response(conn, 200)
+      assert %{"plot" => plot} = json_response(conn, 200)
 
       assert plot == [1] ++ List.duplicate(0, 23)
     end
@@ -141,8 +140,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
           "/api/stats/#{site.domain}/main-graph?period=month&date=2021-01-01&with_imported=true"
         )
 
-      assert %{"plot" => plot, "imports_exist" => true, "includes_imported" => true} =
-               json_response(conn, 200)
+      assert %{"plot" => plot} = json_response(conn, 200)
 
       assert Enum.count(plot) == 31
       assert List.first(plot) == 2
@@ -162,8 +160,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
           "/api/stats/#{site.domain}/main-graph?period=month&date=2021-01-01&with_imported=true"
         )
 
-      assert %{"plot" => plot, "imports_exist" => true, "includes_imported" => true} =
-               json_response(conn, 200)
+      assert %{"plot" => plot} = json_response(conn, 200)
 
       assert Enum.count(plot) == 31
       assert List.first(plot) == 1
@@ -1199,12 +1196,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
           "/api/stats/#{site.domain}/main-graph?period=year&date=2021-01-01&with_imported=true&comparison=year_over_year&interval=month"
         )
 
-      assert %{
-               "plot" => plot,
-               "comparison_plot" => comparison_plot,
-               "imports_exist" => true,
-               "includes_imported" => true
-             } = json_response(conn, 200)
+      assert %{"plot" => plot, "comparison_plot" => comparison_plot} = json_response(conn, 200)
 
       assert 4 == Enum.sum(plot)
       assert 2 == Enum.sum(comparison_plot)
@@ -1245,12 +1237,7 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
           "/api/stats/#{site.domain}/main-graph?period=year&date=2021-01-01&with_imported=false&comparison=year_over_year&interval=month"
         )
 
-      assert %{
-               "plot" => plot,
-               "comparison_plot" => comparison_plot,
-               "imports_exist" => true,
-               "includes_imported" => false
-             } = json_response(conn, 200)
+      assert %{"plot" => plot, "comparison_plot" => comparison_plot} = json_response(conn, 200)
 
       assert 4 == Enum.sum(plot)
       assert 0 == Enum.sum(comparison_plot)
@@ -1275,12 +1262,8 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do
           "/api/stats/#{site.domain}/main-graph?period=7d&date=2021-01-14&comparison=previous_period&metric=conversion_rate&filters=#{filters}"
         )
 
-      assert %{
-               "plot" => this_week_plot,
-               "comparison_plot" => last_week_plot,
-               "imports_exist" => true,
-               "includes_imported" => false
-             } = json_response(conn, 200)
+      assert %{"plot" => this_week_plot, "comparison_plot" => last_week_plot} =
+               json_response(conn, 200)
 
       assert this_week_plot == [50.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0]
       assert last_week_plot == [33.3, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0]