Skip to content

Commit

Permalink
Adds pagination for custom props & fixes an issue with errant (none) …
Browse files Browse the repository at this point in the history 
…return values (#1382)

* Adds pagination for custom props, fixes errant (none) being returned in the middle of a dataset

* Formatting

* Fixes errant (none) value correctly

* Changelog

* Adds tests
  • Loading branch information
@Vigasaurus
Vigasaurus authored Oct 14, 2021
1 parent 688e195 commit 7e9d83d
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ All notable changes to this project will be documented in this file.
- Ability to invite users to sites with different roles plausible/analytics#1122
- Option to configure a custom name for the script file
- Add Conversion Rate to Top Sources, Top Pages Devices, Countries when filtered by a goal plausible/analytics#1299
- Add ability to view more than 100 custom goal properties plausible/analytics#1353

### Fixed
- Fix weekly report time range plausible/analytics#951
Expand Down
34 changes: 28 additions & 6 deletions assets/js/dashboard/stats/conversions/prop-breakdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ export default class PropertyBreakdown extends React.Component {
loading: true,
propKey: propKey,
viewport: DEFAULT_WIDTH,
breakdown: [],
page: 1,
moreResultsAvailable: false
}

this.handleResize = this.handleResize.bind(this);
Expand Down Expand Up @@ -66,11 +69,19 @@ export default class PropertyBreakdown extends React.Component {

fetchPropBreakdown() {
if (this.props.query.filters['goal']) {
api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/property/${encodeURIComponent(this.state.propKey)}`, this.props.query)
.then((res) => this.setState({loading: false, breakdown: res}))
api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/property/${encodeURIComponent(this.state.propKey)}`, this.props.query, {limit: 100, page: this.state.page})
.then((res) => this.setState((state) => ({
loading: false,
breakdown: state.breakdown.concat(res),
moreResultsAvailable: res.length === 100
})))
}
}

loadMore() {
this.setState({loading: true, page: this.state.page + 1}, this.fetchPropBreakdown.bind(this))
}

renderUrl(value) {
if (isValidHttpUrl(value.name)) {
return (
Expand Down Expand Up @@ -131,17 +142,27 @@ export default class PropertyBreakdown extends React.Component {

changePropKey(newKey) {
storage.setItem(this.storageKey, newKey)
this.setState({propKey: newKey, loading: true}, this.fetchPropBreakdown)
this.setState({propKey: newKey, loading: true, breakdown: [], page: 1, moreResultsAvailable: false}, this.fetchPropBreakdown)
}

renderBody() {
renderLoading() {
if (this.state.loading) {
return <div className="px-4 py-2"><div className="loading sm mx-auto"><div></div></div></div>
} else {
return this.state.breakdown.map((propValue) => this.renderPropValue(propValue))
} else if (this.state.moreResultsAvailable) {
return (
<div className="w-full text-center my-4">
<button onClick={this.loadMore.bind(this)} type="button" className="button">
Load more
</button>
</div>
)
}
}

renderBody() {
return this.state.breakdown.map((propValue) => this.renderPropValue(propValue))
}

renderPill(key) {
const isActive = this.state.propKey === key

Expand All @@ -162,6 +183,7 @@ export default class PropertyBreakdown extends React.Component {
</ul>
</div>
{ this.renderBody() }
{ this.renderLoading()}
</div>
)
}
Expand Down
15 changes: 9 additions & 6 deletions lib/plausible/stats/breakdown.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,15 @@ defmodule Plausible.Stats.Breakdown do
end

def breakdown(site, query, "event:props:" <> custom_prop, metrics, pagination) do
{limit, _} = pagination

none_result =
if !Enum.any?(query.filters, fn {key, _} -> String.starts_with?(key, "event:props") end) do
none_filters = Map.put(query.filters, "event:props:" <> custom_prop, {:is, "(none)"})
none_query = %Query{query | filters: none_filters}

{limit, page} = pagination
offset = (page - 1) * limit

from(e in base_event_query(site, none_query),
order_by: [desc: fragment("uniq(?)", e.user_id)],
limit: ^limit,
offset: ^offset,
select: %{},
select_merge: %{^custom_prop => "(none)"},
having: fragment("uniq(?)", e.user_id) > 0
Expand All @@ -82,7 +79,13 @@ defmodule Plausible.Stats.Breakdown do
end

results = breakdown_events(site, query, "event:props:" <> custom_prop, metrics, pagination)
zip_results(none_result, results, custom_prop, metrics)
zipped = zip_results(none_result, results, custom_prop, metrics)

if Enum.find_index(zipped, fn value -> value[custom_prop] == "(none)" end) == limit do
Enum.slice(zipped, 0..(limit - 1))
else
zipped
end
end

def breakdown(site, query, "event:page", metrics, pagination) do
Expand Down
3 changes: 2 additions & 1 deletion lib/plausible_web/controllers/api/stats_controller.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ defmodule PlausibleWeb.Api.StatsController do
def prop_breakdown(conn, params) do
site = conn.assigns[:site]
query = Query.from(site.timezone, params) |> Filters.add_prefix()
pagination = parse_pagination(params)

total_q = Query.remove_goal(query)

Expand All @@ -539,7 +540,7 @@ defmodule PlausibleWeb.Api.StatsController do
prop_name = "event:props:" <> params["prop_name"]

props =
Stats.breakdown(site, query, prop_name, ["visitors", "events"], {100, 1})
Stats.breakdown(site, query, prop_name, ["visitors", "events"], pagination)
|> transform_keys(%{
params["prop_name"] => "name",
"visitors" => "count",
Expand Down
169 changes: 169 additions & 0 deletions test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,175 @@ defmodule PlausibleWeb.Api.ExternalStatsController.BreakdownTest do
]
}
end

test "breakdown by custom event property, with (none)", %{conn: conn, site: site} do
populate_stats([
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["16"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:00:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["16"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:00:00]
),
build(:event,
name: "Purchase",
domain: site.domain,
timestamp: ~N[2021-01-01 00:25:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["14"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:25:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["14"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:26:00]
)
])

conn =
get(conn, "/api/v1/stats/breakdown", %{
"site_id" => site.domain,
"period" => "day",
"date" => "2021-01-01",
"property" => "event:props:cost",
"filters" => "event:name==Purchase"
})

assert json_response(conn, 200) == %{
"results" => [
%{"cost" => "16", "visitors" => 2},
%{"cost" => "14", "visitors" => 2},
%{"cost" => "(none)", "visitors" => 1}
]
}
end

test "breakdown by custom event property, limited", %{conn: conn, site: site} do
populate_stats([
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["16"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:00:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["16"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:00:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["18"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:25:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["14"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:25:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["14"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:26:00]
)
])

conn =
get(conn, "/api/v1/stats/breakdown", %{
"site_id" => site.domain,
"period" => "day",
"date" => "2021-01-01",
"property" => "event:props:cost",
"filters" => "event:name==Purchase",
"limit" => 2
})

assert json_response(conn, 200) == %{
"results" => [
%{"cost" => "14", "visitors" => 2},
%{"cost" => "16", "visitors" => 2}
]
}
end

test "breakdown by custom event property, paginated", %{conn: conn, site: site} do
populate_stats([
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["16"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:00:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["16"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:00:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["18"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:25:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["14"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:25:00]
),
build(:event,
name: "Purchase",
"meta.key": ["cost"],
"meta.value": ["14"],
domain: site.domain,
timestamp: ~N[2021-01-01 00:26:00]
)
])

conn =
get(conn, "/api/v1/stats/breakdown", %{
"site_id" => site.domain,
"period" => "day",
"date" => "2021-01-01",
"property" => "event:props:cost",
"filters" => "event:name==Purchase",
"limit" => 2,
"page" => 2
})

assert json_response(conn, 200) == %{
"results" => [
%{"cost" => "18", "visitors" => 1}
]
}
end
end

describe "filtering" do
Expand Down

0 comments on commit 7e9d83d

Please sign in to comment.