Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove recording of browsers are "mobile" #137

Merged
merged 1 commit into from Jan 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 6 additions & 10 deletions cron/browser_stat.go
Expand Up @@ -23,8 +23,6 @@ import (
// 1 | 2019-12-17 | Chrome | 38 | 13 | t
// 1 | 2019-12-17 | Chrome | 77 | 2 | f
// 1 | 2019-12-17 | Opera | 9 | 1 | f
//
// TODO: mobile counts are inaccurate as it's not grouped by that.
func updateBrowserStats(ctx context.Context, hits []goatcounter.Hit) error {
return zdb.TX(ctx, func(ctx context.Context, tx zdb.DB) error {
// Group by day + browser.
Expand All @@ -37,7 +35,7 @@ func updateBrowserStats(ctx context.Context, hits []goatcounter.Hit) error {
}
grouped := map[string]gt{}
for _, h := range hits {
browser, version, mobile := getBrowser(h.Browser)
browser, version := getBrowser(h.Browser)
if browser == "" {
continue
}
Expand All @@ -49,7 +47,6 @@ func updateBrowserStats(ctx context.Context, hits []goatcounter.Hit) error {
v.day = day
v.browser = browser
v.version = version
v.mobile = mobile
var err error
v.count, err = existingBrowserStats(ctx, tx, h.Site, day, v.browser, v.version)
if err != nil {
Expand All @@ -63,9 +60,9 @@ func updateBrowserStats(ctx context.Context, hits []goatcounter.Hit) error {

siteID := goatcounter.MustGetSite(ctx).ID
ins := bulk.NewInsert(ctx, tx,
"browser_stats", []string{"site", "day", "browser", "version", "count", "mobile"})
"browser_stats", []string{"site", "day", "browser", "version", "count"})
for _, v := range grouped {
ins.Values(siteID, v.day, v.browser, v.version, v.count, v.mobile)
ins.Values(siteID, v.day, v.browser, v.version, v.count)
}
return ins.Finish()
})
Expand Down Expand Up @@ -96,13 +93,13 @@ func existingBrowserStats(
return c, nil
}

func getBrowser(uaHeader string) (string, string, bool) {
func getBrowser(uaHeader string) (string, string) {
ua := user_agent.New(uaHeader)
browser, version := ua.Browser()

// A lot of this is wrong, so just skip for now.
if browser == "Android" {
return "", "", false
return "", ""
}

if browser == "Chromium" {
Expand Down Expand Up @@ -134,6 +131,5 @@ func getBrowser(uaHeader string) (string, string, bool) {
}
}

//mobile := ua.Mobile()
return browser, version, false
return browser, version
}
14 changes: 7 additions & 7 deletions cron/browser_stat_test.go
Expand Up @@ -30,13 +30,13 @@ func TestBrowserStats(t *testing.T) {
}

var stats goatcounter.Stats
total, totalMobile, err := stats.ListBrowsers(ctx, now, now)
total, err := stats.ListBrowsers(ctx, now, now)
if err != nil {
t.Fatal(err)
}

want := `4 -> 0 -> [{Firefox false 3} {Chrome false 1}]`
out := fmt.Sprintf("%d -> %d -> %v", total, totalMobile, stats)
want := `4 -> [{Firefox 3} {Chrome 1}]`
out := fmt.Sprintf("%d -> %v", total, stats)
if want != out {
t.Errorf("\nwant: %s\nout: %s", want, out)
}
Expand All @@ -52,13 +52,13 @@ func TestBrowserStats(t *testing.T) {
}

stats = goatcounter.Stats{}
total, totalMobile, err = stats.ListBrowsers(ctx, now, now)
total, err = stats.ListBrowsers(ctx, now, now)
if err != nil {
t.Fatal(err)
}

want = `7 -> 0 -> [{Firefox false 6} {Chrome false 1}]`
out = fmt.Sprintf("%d -> %d -> %v", total, totalMobile, stats)
want = `7 -> [{Firefox 6} {Chrome 1}]`
out = fmt.Sprintf("%d -> %v", total, stats)
if want != out {
t.Errorf("\nwant: %s\nout: %s", want, out)
}
Expand All @@ -70,7 +70,7 @@ func TestBrowserStats(t *testing.T) {
t.Fatal(err)
}

want = `6 -> [{Firefox 69.0 false 4} {Firefox 68.0 false 1} {Firefox 70.0 false 1}]`
want = `6 -> [{Firefox 69.0 4} {Firefox 68.0 1} {Firefox 70.0 1}]`
out = fmt.Sprintf("%d -> %v", total, stats)
if want != out {
t.Errorf("\nwant: %s\nout: %s", want, out)
Expand Down
4 changes: 2 additions & 2 deletions cron/location_stat_test.go
Expand Up @@ -34,7 +34,7 @@ func TestLocationStats(t *testing.T) {
t.Fatal(err)
}

want := `3 -> [{Indonesia false 2} {Ethiopia false 1}]`
want := `3 -> [{Indonesia 2} {Ethiopia 1}]`
out := fmt.Sprintf("%d -> %v", total, stats)
if want != out {
t.Errorf("\nwant: %s\nout: %s", want, out)
Expand All @@ -60,7 +60,7 @@ func TestLocationStats(t *testing.T) {
t.Fatal(err)
}

want = `10 -> [{Ethiopia false 5} {Indonesia false 4} {New Zealand false 1}]`
want = `10 -> [{Ethiopia 5} {Indonesia 4} {New Zealand 1}]`
out = fmt.Sprintf("%d -> %v", total, stats)
if want != out {
t.Errorf("\nwant: %s\nout: %s", want, out)
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/pgsql/2020-01-24-1-rm-mobile.sql
@@ -0,0 +1,5 @@
begin;
alter table browser_stats drop column mobile;
insert into version values ('2020-01-24-1-rm-mobile');
commit;

19 changes: 19 additions & 0 deletions db/migrate/sqlite/2020-01-24-1-rm-mobile.sql
@@ -0,0 +1,19 @@
begin;
create table browser_stats2 (
site integer not null check(site > 0),

day date not null check(day = strftime('%Y-%m-%d', day)),
browser varchar not null,
version varchar not null,
count int not null,

foreign key (site) references sites(id) on delete restrict on update restrict
);

insert into browser_stats2 select site, day, browser, version, count from browser_stats;
drop table browser_stats;
alter table browser_stats2 rename to browser_stats;

insert into version values ('2020-01-24-1-rm-mobile');
commit;

4 changes: 1 addition & 3 deletions handlers/backend.go
Expand Up @@ -293,7 +293,7 @@ func (h backend) index(w http.ResponseWriter, r *http.Request) error {
l = l.Since("pages.List")

var browsers goatcounter.Stats
totalBrowsers, totalMobile, err := browsers.ListBrowsers(r.Context(), start, end)
totalBrowsers, err := browsers.ListBrowsers(r.Context(), start, end)
if err != nil {
return err
}
Expand Down Expand Up @@ -345,7 +345,6 @@ func (h backend) index(w http.ResponseWriter, r *http.Request) error {
TotalHitsDisplay int
Browsers goatcounter.Stats
TotalBrowsers int
TotalMobile string
SubSites []string
SizeStat goatcounter.Stats
TotalSize int
Expand All @@ -354,7 +353,6 @@ func (h backend) index(w http.ResponseWriter, r *http.Request) error {
ShowMoreLocations bool
}{newGlobals(w, r), sr, r.URL.Query().Get("hl-period"), start, end, filter,
pages, refs, moreRefs, total, totalDisplay, browsers, totalBrowsers,
fmt.Sprintf("%.1f", float32(totalMobile)/float32(totalBrowsers)*100),
subs, sizeStat, totalSize, locStat, totalLoc, showMoreLoc})
l = l.Since("zhttp.Template")
l.FieldsSince().Print("")
Expand Down
48 changes: 15 additions & 33 deletions hit.go
Expand Up @@ -528,45 +528,28 @@ func (h *HitStats) ListPathsLike(ctx context.Context, path string) error {
}

type Stats []struct {
Name string
Mobile bool
Count int
Name string
Count int
}

// List all browser statistics for the given time period.
func (h *Stats) ListBrowsers(ctx context.Context, start, end time.Time) (int, int, error) {
func (h *Stats) ListBrowsers(ctx context.Context, start, end time.Time) (int, error) {
err := zdb.MustGet(ctx).SelectContext(ctx, h, `
select browser as name, sum(count) as count from browser_stats
where site=$1 and day >= $2 and day <= $3
group by browser
group by browser
order by count desc
`, MustGetSite(ctx).ID, start.Format("2006-01-02"), end.Format("2006-01-02"))
if err != nil {
return 0, 0, errors.Wrap(err, "Stats.ListBrowsers browsers")
return 0, errors.Wrap(err, "Stats.ListBrowsers browsers")
}

var total int
for _, b := range *h {
total += b.Count
}

// List number of mobile browsers.
// TODO: inaccurate and not shown in UI at the moment.
//var m *int
//err = zdb.MustGet(ctx).GetContext(ctx, &m, `
// select sum(count) from browser_stats
// where site=$1 and day >= $2 and day <= $3 and mobile=true
//`, MustGetSite(ctx).ID, start.Format("2006-01-02"), end.Format("2006-01-02"))
//if err != nil {
// return 0, 0, errors.Wrap(err, "Stats.ListBrowsers mobile")
//}

mobile := 0
//if m != nil {
// mobile = *m
//}

return total, mobile, nil
return total, nil
}

// ListBrowser lists all the versions for one browser.
Expand Down Expand Up @@ -632,12 +615,12 @@ func (h *Stats) ListSizes(ctx context.Context, start, end time.Time) (int, error
// TODO: group a bit; ideally I'd like to make a line chart in the future,
// in which case this should no longer be needed.
ns := Stats{
{sizePhones, false, 0},
{sizeLargePhones, false, 0},
{sizeTablets, false, 0},
{sizeDesktop, false, 0},
{sizeDesktopHD, false, 0},
{sizeUnknown, false, 0},
{sizePhones, 0},
{sizeLargePhones, 0},
{sizeTablets, 0},
{sizeDesktop, 0},
{sizeDesktopHD, 0},
{sizeUnknown, 0},
}

hh := *h
Expand Down Expand Up @@ -720,10 +703,9 @@ func (h *Stats) ListSize(ctx context.Context, name string, start, end time.Time)
for size, count := range grouped {
total += count
ns = append(ns, struct {
Name string
Mobile bool
Count int
}{size, false, count})
Name string
Count int
}{size, count})
}
sort.Slice(ns, func(i int, j int) bool { return ns[i].Count > ns[j].Count })
*h = ns
Expand Down
30 changes: 27 additions & 3 deletions pack/pack.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions tpl/backend.gohtml
Expand Up @@ -81,9 +81,7 @@
<em>Nothing to display</em>
{{else}}
<div class="chart-hbar" data-detail="/sizes">{{hbar_chart .Context .SizeStat .TotalSize 0 0.1 true}}</div>
<p><small>The screen sizes are an indication and influenced by DPI and zoom levels.
{{/*Approximately {{.TotalMobile}}% advertised the usage of a mobile browser.*/}}
</small></p>
<p><small>The screen sizes are an indication and influenced by DPI and zoom levels.</small></p>
{{end}}
</div>
<div class="location-chart">
Expand Down