Skip to content

Commit

Permalink
Remove recording of browsers are "mobile" (#137)
Browse files Browse the repository at this point in the history
This didn't work well anyway, and now that we gave screen size stats
it's not really needed any more.

Fixes #136, the database after creating from scratch has the mobile
column as integer, whereas my production one has boolean. Not sure how
that happened, but not really worth fixing (so just remove).
  • Loading branch information
arp242 committed Jan 24, 2020
1 parent d267c9f commit 1e814a6
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 61 deletions.
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

0 comments on commit 1e814a6

Please sign in to comment.