Skip to content

Commit

Permalink
Remove LegacyDashboardFilterParser (#4882)
Browse files Browse the repository at this point in the history
* Remove LegacyDashboardFilterParser usage in stats_controller

* Update tests to avoid legacy dashboard parsing

* Update top stats tests

Note a few behavioral test changes since dashboard doesn't do wildcards anymore

* Update test/plausible_web/controllers/stats_controller_test.exs

* Update sources_test

Removed test was dead functionality

* Update countries_test.exs

* Update test/plausible_web/controllers/api/stats_controller/browsers_test.exs

* Update test/plausible_web/controllers/api/stats_controller/custom_prop_breakdown_test.exs

* Update test/plausible_web/controllers/api/stats_controller/suggestions_test.exs

* Remove dead tests

* Update pages_test

* Update conversions_test.exs

* Update funnels_test.exs

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

* Update test/plausible_web/controllers/api/stats_controller/suggestions_test.exs

* Update test/plausible/stats/query_test.exs

* Remove legacy parsing code

* Changelog

* Inline utils
  • Loading branch information
macobo authored Dec 9, 2024
1 parent 0c8fed9 commit 5b7b543
Show file tree
Hide file tree
Showing 23 changed files with 396 additions and 660 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ All notable changes to this project will be documented in this file.
- Support for `case_sensitive: false` modifiers in Stats API V2 filters for case-insensitive searches.

### Removed

- Internal stats API routes no longer support legacy dashboard filter format.

### Changed

- Details modal search inputs are now case-insensitive.
Expand Down
11 changes: 2 additions & 9 deletions lib/plausible/stats/filters/filters.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ defmodule Plausible.Stats.Filters do
"""

alias Plausible.Stats.Filters.QueryParser
alias Plausible.Stats.Filters.{LegacyDashboardFilterParser, StatsAPIFilterParser}
alias Plausible.Stats.Filters.StatsAPIFilterParser

@visit_props [
:source,
Expand Down Expand Up @@ -52,17 +52,13 @@ defmodule Plausible.Stats.Filters do
Depending on the format and type of the `filters` argument, returns:
* a decoded list, when `filters` is encoded JSON
* a parsed filter list, when `filters` is a filter expression string
* the same list, when `filters` is a map
Returns an empty list when argument type is unexpected (e.g. `nil`).
### Examples:
iex> Filters.parse("{\\"page\\":\\"/blog/**\\"}")
[[:matches_wildcard, "event:page", ["/blog/**"]]]
iex> Filters.parse("visit:browser!=Chrome")
[[:is_not, "visit:browser", ["Chrome"]]]
Expand All @@ -71,15 +67,12 @@ defmodule Plausible.Stats.Filters do
"""
def parse(filters) when is_binary(filters) do
case Jason.decode(filters) do
{:ok, filters} when is_map(filters) or is_list(filters) -> parse(filters)
{:ok, filters} when is_list(filters) -> parse(filters)
{:ok, _} -> []
{:error, err} -> StatsAPIFilterParser.parse_filter_expression(err.data)
end
end

def parse(filters) when is_map(filters),
do: LegacyDashboardFilterParser.parse_and_prefix(filters)

def parse(filters) when is_list(filters) do
{:ok, parsed_filters} = QueryParser.parse_filters(filters)
parsed_filters
Expand Down
86 changes: 0 additions & 86 deletions lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex

This file was deleted.

20 changes: 19 additions & 1 deletion lib/plausible/stats/filters/stats_api_filter_parser.ex
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
defmodule Plausible.Stats.Filters.StatsAPIFilterParser do
@moduledoc false

import Plausible.Stats.Filters.Utils
@non_escaped_pipe_regex ~r/(?<!\\)\|/

@doc """
This function parses the filter expression given as a string.
Expand Down Expand Up @@ -70,4 +70,22 @@ defmodule Plausible.Stats.Filters.StatsAPIFilterParser do

[:is, key, List.wrap(value)]
end

defp list_expression?(expression) do
Regex.match?(@non_escaped_pipe_regex, expression)
end

defp wildcard_expression?(expression) do
String.contains?(expression, "*")
end

defp parse_member_list(raw_value) do
raw_value
|> String.split(@non_escaped_pipe_regex)
|> Enum.map(&remove_escape_chars/1)
end

defp remove_escape_chars(value) do
String.replace(value, "\\|", "|")
end
end
25 changes: 1 addition & 24 deletions lib/plausible/stats/filters/utils.ex
Original file line number Diff line number Diff line change
@@ -1,28 +1,5 @@
defmodule Plausible.Stats.Filters.Utils do
@moduledoc """
Contains utility functions shared between `LegacyDashboardFilterParser`
and `StatsAPIFilterParser`.
"""

@non_escaped_pipe_regex ~r/(?<!\\)\|/

def list_expression?(expression) do
Regex.match?(@non_escaped_pipe_regex, expression)
end

def wildcard_expression?(expression) do
String.contains?(expression, "*")
end

def parse_member_list(raw_value) do
raw_value
|> String.split(@non_escaped_pipe_regex)
|> Enum.map(&remove_escape_chars/1)
end

def remove_escape_chars(value) do
String.replace(value, "\\|", "|")
end
@moduledoc false

def split_goals(goals) do
Enum.split_with(goals, fn goal -> Plausible.Goal.type(goal) == :event end)
Expand Down
5 changes: 1 addition & 4 deletions lib/plausible_web/controllers/api/stats_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ defmodule PlausibleWeb.Api.StatsController do

alias Plausible.Stats
alias Plausible.Stats.{Query, Comparisons, Filters, Time, TableDecider}
alias Plausible.Stats.Filters.LegacyDashboardFilterParser
alias PlausibleWeb.Api.Helpers, as: H

require Logger
Expand Down Expand Up @@ -803,11 +802,9 @@ defmodule PlausibleWeb.Api.StatsController do
site = conn.assigns[:site]
params = Map.put(params, "property", "visit:referrer")

referrer_filter = LegacyDashboardFilterParser.filter_value("visit:source", referrer)

query =
Query.from(site, params, debug_metadata(conn))
|> Query.add_filter(referrer_filter)
|> Query.add_filter([:is, "visit:source", [referrer]])

pagination = parse_pagination(params)

Expand Down
Loading

0 comments on commit 5b7b543

Please sign in to comment.