From 3d656ae95be0c577084a0324f5240e261ec035da Mon Sep 17 00:00:00 2001 From: Vini Brasil Date: Thu, 13 Apr 2023 10:01:54 -0300 Subject: [PATCH] Match day of the week feature for comparisons (#2822) * Add support for `match_day_of_week?` back-end option * Add match day of week input to comparison input * Default match day of the week to true --- assets/js/dashboard/api.js | 1 + assets/js/dashboard/comparison-input.js | 39 +++++++++- assets/js/dashboard/query.js | 5 +- lib/plausible/stats/comparisons.ex | 71 ++++++++++++++++++- .../controllers/api/stats_controller.ex | 17 +++-- test/plausible/stats/comparisons_test.exs | 48 +++++++++++++ 6 files changed, 171 insertions(+), 10 deletions(-) diff --git a/assets/js/dashboard/api.js b/assets/js/dashboard/api.js index b67ee17d8..165ca0838 100644 --- a/assets/js/dashboard/api.js +++ b/assets/js/dashboard/api.js @@ -50,6 +50,7 @@ export function serializeQuery(query, extraQuery=[]) { queryObj.comparison = query.comparison queryObj.compare_from = query.compare_from ? formatISO(query.compare_from) : undefined queryObj.compare_to = query.compare_to ? formatISO(query.compare_to) : undefined + queryObj.match_day_of_week = query.match_day_of_week } Object.assign(queryObj, ...extraQuery) diff --git a/assets/js/dashboard/comparison-input.js b/assets/js/dashboard/comparison-input.js index dac7ff2a2..0a099d39e 100644 --- a/assets/js/dashboard/comparison-input.js +++ b/assets/js/dashboard/comparison-input.js @@ -19,6 +19,10 @@ const DEFAULT_COMPARISON_MODE = 'previous_period' export const COMPARISON_DISABLED_PERIODS = ['realtime', 'all'] +export const getStoredMatchDayOfWeek = function(domain) { + return storage.getItem(`comparison_match_day_of_week__${domain}`) || 'true' +} + export const getStoredComparisonMode = function(domain) { const mode = storage.getItem(`comparison_mode__${domain}`) if (Object.keys(COMPARISON_MODES).includes(mode)) { @@ -53,7 +57,7 @@ export const toggleComparisons = function(history, query, site) { } } -function DropdownItem({ label, value, isCurrentlySelected, updateMode, setUiMode }) { +function ComparisonModeOption({ label, value, isCurrentlySelected, updateMode, setUiMode }) { const click = () => { if (value == "custom") { setUiMode("datepicker") @@ -80,6 +84,33 @@ function DropdownItem({ label, value, isCurrentlySelected, updateMode, setUiMode ) } +function MatchDayOfWeekInput({ history, query, site }) { + const click = (matchDayOfWeek) => { + storage.setItem(`comparison_match_day_of_week__${site.domain}`, matchDayOfWeek.toString()) + navigateToQuery(history, query, { match_day_of_week: matchDayOfWeek.toString() }) + } + + const buttonClass = (hover, selected) => + classNames("px-4 py-2 w-full text-left font-medium text-sm dark:text-white cursor-pointer", { + "bg-gray-100 text-gray-900 dark:bg-gray-900 dark:text-gray-100": hover, + "font-bold": selected, + }) + + return <> + click(true)}> + {({ active }) => ( + + )} + + + click(false)}> + {({ active }) => ( + + )} + + +} + const ComparisonInput = function({ site, query, history }) { if (!site.flags.comparisons) return null if (COMPARISON_DISABLED_PERIODS.includes(query.period)) return null @@ -137,7 +168,11 @@ const ComparisonInput = function({ site, query, history }) { leaveFrom="transform opacity-100 scale-100" leaveTo="transform opacity-0 scale-95"> - { Object.keys(COMPARISON_MODES).map((key) => DropdownItem({ label: COMPARISON_MODES[key], value: key, isCurrentlySelected: key == query.comparison, updateMode, setUiMode })) } + { Object.keys(COMPARISON_MODES).map((key) => ComparisonModeOption({ label: COMPARISON_MODES[key], value: key, isCurrentlySelected: key == query.comparison, updateMode, setUiMode })) } + { query.comparison !== "custom" && +
+ + } diff --git a/assets/js/dashboard/query.js b/assets/js/dashboard/query.js index cfdcab3b9..3aaec0a3d 100644 --- a/assets/js/dashboard/query.js +++ b/assets/js/dashboard/query.js @@ -2,7 +2,7 @@ import React from 'react' import { Link, withRouter } from 'react-router-dom' import {nowForSite} from './util/date' import * as storage from './util/storage' -import { COMPARISON_DISABLED_PERIODS, getStoredComparisonMode, isComparisonEnabled } from './comparison-input' +import { COMPARISON_DISABLED_PERIODS, getStoredComparisonMode, isComparisonEnabled,getStoredMatchDayOfWeek } from './comparison-input' import dayjs from 'dayjs'; import utc from 'dayjs/plugin/utc'; @@ -27,6 +27,8 @@ export function parseQuery(querystring, site) { let comparison = q.get('comparison') || getStoredComparisonMode(site.domain) if (COMPARISON_DISABLED_PERIODS.includes(period) || !isComparisonEnabled(comparison)) comparison = null + let matchDayOfWeek = q.get('match_day_of_week') || getStoredMatchDayOfWeek(site.domain) + return { period, comparison, @@ -35,6 +37,7 @@ export function parseQuery(querystring, site) { date: q.get('date') ? dayjs.utc(q.get('date')) : nowForSite(site), from: q.get('from') ? dayjs.utc(q.get('from')) : undefined, to: q.get('to') ? dayjs.utc(q.get('to')) : undefined, + match_day_of_week: matchDayOfWeek == 'true', with_imported: q.get('with_imported') ? q.get('with_imported') === 'true' : true, filters: { 'goal': q.get('goal'), diff --git a/lib/plausible/stats/comparisons.ex b/lib/plausible/stats/comparisons.ex index e3d009a0b..a81753c72 100644 --- a/lib/plausible/stats/comparisons.ex +++ b/lib/plausible/stats/comparisons.ex @@ -44,6 +44,15 @@ defmodule Plausible.Stats.Comparisons do * `:to` - a ISO-8601 date string used when mode is `"custom"`. Must be after `from`. + * `:match_day_of_week?` - determines whether the comparison query should be + adjusted to match the day of the week of the source query. When this option + is set to true, the comparison query is shifted to start on the same day of + the week as the source query, rather than on the exact same date. For + example, if the source query starts on Sunday, January 1st, 2023 and the + `year_over_year` comparison query is configured to `match_day_of_week?`, + it will be shifted to start on Sunday, January 2nd, 2022 instead of + January 1st. Defaults to false. + """ def compare(%Plausible.Site{} = site, %Stats.Query{} = source_query, mode, opts \\ []) do if valid_mode?(source_query, mode) do @@ -61,7 +70,13 @@ defmodule Plausible.Stats.Comparisons do end_date = earliest(source_query.date_range.last, now) |> Date.add(-365) range = Date.range(start_date, end_date) - {:ok, %Stats.Query{source_query | date_range: range}} + + comparison_query = + source_query + |> Map.put(:date_range, range) + |> maybe_match_day_of_week(source_query, opts) + + {:ok, comparison_query} end defp do_compare(source_query, "previous_period", opts) do @@ -74,7 +89,13 @@ defmodule Plausible.Stats.Comparisons do new_last = Date.add(last, diff_in_days) range = Date.range(new_first, new_last) - {:ok, %Stats.Query{source_query | date_range: range}} + + comparison_query = + source_query + |> Map.put(:date_range, range) + |> maybe_match_day_of_week(source_query, opts) + + {:ok, comparison_query} end defp do_compare(source_query, "custom", opts) do @@ -91,6 +112,52 @@ defmodule Plausible.Stats.Comparisons do if Date.compare(a, b) in [:eq, :lt], do: a, else: b end + defp maybe_match_day_of_week(comparison_query, source_query, opts) do + if Keyword.get(opts, :match_day_of_week?, false) do + day_to_match = Date.day_of_week(source_query.date_range.first) + + new_first = + shift_to_nearest( + day_to_match, + comparison_query.date_range.first, + source_query.date_range.first + ) + + days_shifted = Date.diff(new_first, comparison_query.date_range.first) + new_last = Date.add(comparison_query.date_range.last, days_shifted) + + new_range = Date.range(new_first, new_last) + %Stats.Query{comparison_query | date_range: new_range} + else + comparison_query + end + end + + defp shift_to_nearest(day_of_week, date, reject) do + if Date.day_of_week(date) == day_of_week do + date + else + [next_occurring(day_of_week, date), previous_occurring(day_of_week, date)] + |> Enum.sort_by(&Date.diff(date, &1)) + |> Enum.reject(&(&1 == reject)) + |> List.first() + end + end + + defp next_occurring(day_of_week, date) do + days_to_add = day_of_week - Date.day_of_week(date) + days_to_add = if days_to_add > 0, do: days_to_add, else: days_to_add + 7 + + Date.add(date, days_to_add) + end + + defp previous_occurring(day_of_week, date) do + days_to_subtract = Date.day_of_week(date) - day_of_week + days_to_subtract = if days_to_subtract > 0, do: days_to_subtract, else: days_to_subtract + 7 + + Date.add(date, -days_to_subtract) + end + @spec valid_mode?(Stats.Query.t(), mode()) :: boolean() @doc """ Returns whether the source query and the selected mode support comparisons. diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index 6ad0c0b88..30850f024 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -111,11 +111,10 @@ defmodule PlausibleWeb.Api.StatsController do timeseries_result = Stats.timeseries(site, timeseries_query, [selected_metric]) + comparison_opts = parse_comparison_opts(params) + comparison_result = - case Comparisons.compare(site, query, params["comparison"], - from: params["compare_from"], - to: params["compare_to"] - ) do + case Comparisons.compare(site, query, params["comparison"], comparison_opts) do {:ok, comparison_query} -> Stats.timeseries(site, comparison_query, [selected_metric]) {:error, :not_supported} -> nil end @@ -193,7 +192,7 @@ defmodule PlausibleWeb.Api.StatsController do query = Query.from(site, params) |> Filters.add_prefix() comparison_mode = params["comparison"] || "previous_period" - comparison_opts = [from: params["compare_from"], to: params["compare_to"]] + comparison_opts = parse_comparison_opts(params) comparison_query = case Stats.Comparisons.compare(site, query, comparison_mode, comparison_opts) do @@ -1316,4 +1315,12 @@ defmodule PlausibleWeb.Api.StatsController do |> put_status(400) |> json(%{error: message}) end + + defp parse_comparison_opts(params) do + [ + from: params["compare_from"], + to: params["compare_to"], + match_day_of_week?: params["match_day_of_week"] == "true" + ] + end end diff --git a/test/plausible/stats/comparisons_test.exs b/test/plausible/stats/comparisons_test.exs index bbf474819..657c1894f 100644 --- a/test/plausible/stats/comparisons_test.exs +++ b/test/plausible/stats/comparisons_test.exs @@ -24,6 +24,18 @@ defmodule Plausible.Stats.ComparisonsTest do assert comparison.date_range.first == ~D[2023-02-28] assert comparison.date_range.last == ~D[2023-02-28] end + + test "matches the day of the week when nearest day is original query start date and mode is previous_period" do + site = build(:site) + query = Query.from(site, %{"period" => "month", "date" => "2023-03-02"}) + now = ~N[2023-03-02 14:00:00] + + {:ok, comparison} = + Comparisons.compare(site, query, "previous_period", now: now, match_day_of_week?: true) + + assert comparison.date_range.first == ~D[2023-02-22] + assert comparison.date_range.last == ~D[2023-02-23] + end end describe "with period set to previous month" do @@ -59,6 +71,30 @@ defmodule Plausible.Stats.ComparisonsTest do assert comparison.date_range.first == ~D[2019-02-01] assert comparison.date_range.last == ~D[2019-03-01] end + + test "matches the day of the week when mode is previous_period keeping the same day" do + site = build(:site) + query = Query.from(site, %{"period" => "month", "date" => "2023-02-01"}) + now = ~N[2023-03-01 14:00:00] + + {:ok, comparison} = + Comparisons.compare(site, query, "previous_period", now: now, match_day_of_week?: true) + + assert comparison.date_range.first == ~D[2023-01-04] + assert comparison.date_range.last == ~D[2023-01-31] + end + + test "matches the day of the week when mode is previous_period" do + site = build(:site) + query = Query.from(site, %{"period" => "month", "date" => "2023-01-01"}) + now = ~N[2023-03-01 14:00:00] + + {:ok, comparison} = + Comparisons.compare(site, query, "previous_period", now: now, match_day_of_week?: true) + + assert comparison.date_range.first == ~D[2022-12-04] + assert comparison.date_range.last == ~D[2023-01-03] + end end describe "with period set to year to date" do @@ -83,6 +119,18 @@ defmodule Plausible.Stats.ComparisonsTest do assert comparison.date_range.first == ~D[2022-01-01] assert comparison.date_range.last == ~D[2022-03-01] end + + test "matches the day of the week when mode is year_over_year" do + site = build(:site) + query = Query.from(site, %{"period" => "year", "date" => "2023-03-01"}) + now = ~N[2023-03-01 14:00:00] + + {:ok, comparison} = + Comparisons.compare(site, query, "year_over_year", now: now, match_day_of_week?: true) + + assert comparison.date_range.first == ~D[2022-01-02] + assert comparison.date_range.last == ~D[2022-03-02] + end end describe "with period set to previous year" do