From eaa673c4e465aacf05075adb53ee3497f86b940f Mon Sep 17 00:00:00 2001 From: Romain Le Jeune Date: Tue, 4 Jul 2023 09:59:43 +0000 Subject: [PATCH] fix(GODT-2708): fix dimensions event format + handling of ReportClicked event. --- internal/bridge/config_status.go | 2 +- internal/configstatus/config_status.go | 26 +++++++++++++++++-- internal/configstatus/configuration_abort.go | 17 +++++++----- .../configstatus/configuration_abort_test.go | 12 ++++----- .../configstatus/configuration_progress.go | 2 +- .../configuration_progress_test.go | 2 +- .../configstatus/configuration_recovery.go | 17 ++++++------ .../configuration_recovery_test.go | 12 ++++----- .../configstatus/configuration_success.go | 15 ++++++----- .../configuration_success_test.go | 12 ++++----- internal/user/config_status.go | 6 +++-- 11 files changed, 76 insertions(+), 47 deletions(-) diff --git a/internal/bridge/config_status.go b/internal/bridge/config_status.go index 05b801e0..755e1627 100644 --- a/internal/bridge/config_status.go +++ b/internal/bridge/config_status.go @@ -24,7 +24,7 @@ import ( func (bridge *Bridge) ReportBugClicked() { safe.Lock(func() { for _, user := range bridge.users { - user.ReportBugSent() + user.ReportBugClicked() } }, bridge.usersLock) } diff --git a/internal/configstatus/config_status.go b/internal/configstatus/config_status.go index 6f590936..429a10a6 100644 --- a/internal/configstatus/config_status.go +++ b/internal/configstatus/config_status.go @@ -21,6 +21,7 @@ import ( "encoding/json" "fmt" "os" + "strconv" "time" "github.com/ProtonMail/proton-bridge/v3/internal/safe" @@ -74,8 +75,9 @@ func (status *ConfigurationStatus) Save() error { if err != nil { return err } - - err = json.NewEncoder(f).Encode(status.Data) + enc := json.NewEncoder(f) + enc.SetIndent("", " ") + err = enc.Encode(status.Data) if err := f.Close(); err != nil { logrus.WithError(err).Error("Error while closing configstatus file.") } @@ -197,3 +199,23 @@ func (data *ConfigurationStatusData) hasLinkClicked(pos uint) bool { val := data.DataV1.ClickedLink & (1 << pos) return val > 0 } + +func (data *ConfigurationStatusData) clickedLinkToString() string { + var str = "" + var first = true + for i := 0; i < 64; i++ { + if data.hasLinkClicked(uint(i)) { + if !first { + str += "," + } else { + first = false + str += "[" + } + str += strconv.Itoa(i) + } + } + if str != "" { + str += "]" + } + return str +} diff --git a/internal/configstatus/configuration_abort.go b/internal/configstatus/configuration_abort.go index 29e5a147..36ad8f5c 100644 --- a/internal/configstatus/configuration_abort.go +++ b/internal/configstatus/configuration_abort.go @@ -17,16 +17,19 @@ package configstatus -import "time" +import ( + "strconv" + "time" +) type ConfigAbortValues struct { Duration int `json:"duration"` } type ConfigAbortDimensions struct { - ReportClick interface{} `json:"report_click"` - ReportSent interface{} `json:"report_sent"` - ClickedLink uint64 `json:"clicked_link"` + ReportClick string `json:"report_click"` + ReportSent string `json:"report_sent"` + ClickedLink string `json:"clicked_link"` } type ConfigAbortData struct { @@ -46,9 +49,9 @@ func (*ConfigAbortBuilder) New(data *ConfigurationStatusData) ConfigAbortData { Duration: int(time.Since(data.DataV1.PendingSince).Minutes()), }, Dimensions: ConfigSuccessDimensions{ - ReportClick: data.DataV1.ReportClick, - ReportSent: data.DataV1.ReportSent, - ClickedLink: data.DataV1.ClickedLink, + ReportClick: strconv.FormatBool(data.DataV1.ReportClick), + ReportSent: strconv.FormatBool(data.DataV1.ReportSent), + ClickedLink: data.clickedLinkToString(), }, } } diff --git a/internal/configstatus/configuration_abort_test.go b/internal/configstatus/configuration_abort_test.go index e076952a..c9851b2a 100644 --- a/internal/configstatus/configuration_abort_test.go +++ b/internal/configstatus/configuration_abort_test.go @@ -38,9 +38,9 @@ func TestConfigurationAbort_default(t *testing.T) { require.Equal(t, "bridge.any.configuration", req.MeasurementGroup) require.Equal(t, "bridge_config_abort", req.Event) require.Equal(t, 0, req.Values.Duration) - require.Equal(t, false, req.Dimensions.ReportClick) - require.Equal(t, false, req.Dimensions.ReportSent) - require.Equal(t, uint64(0), req.Dimensions.ClickedLink) + require.Equal(t, "false", req.Dimensions.ReportClick) + require.Equal(t, "false", req.Dimensions.ReportSent) + require.Equal(t, "", req.Dimensions.ClickedLink) } func TestConfigurationAbort_fed(t *testing.T) { @@ -69,7 +69,7 @@ func TestConfigurationAbort_fed(t *testing.T) { require.Equal(t, "bridge.any.configuration", req.MeasurementGroup) require.Equal(t, "bridge_config_abort", req.Event) require.Equal(t, 10, req.Values.Duration) - require.Equal(t, true, req.Dimensions.ReportClick) - require.Equal(t, false, req.Dimensions.ReportSent) - require.Equal(t, uint64(42), req.Dimensions.ClickedLink) + require.Equal(t, "true", req.Dimensions.ReportClick) + require.Equal(t, "false", req.Dimensions.ReportSent) + require.Equal(t, "[1,3,5]", req.Dimensions.ClickedLink) } diff --git a/internal/configstatus/configuration_progress.go b/internal/configstatus/configuration_progress.go index 8046e689..d25a6019 100644 --- a/internal/configstatus/configuration_progress.go +++ b/internal/configstatus/configuration_progress.go @@ -46,7 +46,7 @@ func (*ConfigProgressBuilder) New(data *ConfigurationStatusData) ConfigProgressD func numberOfDay(now, prev time.Time) int { if now.IsZero() || prev.IsZero() { - return 0 + return 1 } if now.Year() > prev.Year() { if now.YearDay() > prev.YearDay() { diff --git a/internal/configstatus/configuration_progress_test.go b/internal/configstatus/configuration_progress_test.go index ff0cd88c..e772af01 100644 --- a/internal/configstatus/configuration_progress_test.go +++ b/internal/configstatus/configuration_progress_test.go @@ -38,7 +38,7 @@ func TestConfigurationProgress_default(t *testing.T) { require.Equal(t, "bridge.any.configuration", req.MeasurementGroup) require.Equal(t, "bridge_config_progress", req.Event) require.Equal(t, 0, req.Values.NbDay) - require.Equal(t, 0, req.Values.NbDaySinceLast) + require.Equal(t, 1, req.Values.NbDaySinceLast) } func TestConfigurationProgress_fed(t *testing.T) { diff --git a/internal/configstatus/configuration_recovery.go b/internal/configstatus/configuration_recovery.go index dae2b96e..850a7dfc 100644 --- a/internal/configstatus/configuration_recovery.go +++ b/internal/configstatus/configuration_recovery.go @@ -18,6 +18,7 @@ package configstatus import ( + "strconv" "time" ) @@ -26,11 +27,11 @@ type ConfigRecoveryValues struct { } type ConfigRecoveryDimensions struct { - Autoconf string `json:"autoconf"` - ReportClick interface{} `json:"report_click"` - ReportSent interface{} `json:"report_sent"` - ClickedLink uint64 `json:"clicked_link"` - FailureDetails string `json:"failure_details"` + Autoconf string `json:"autoconf"` + ReportClick string `json:"report_click"` + ReportSent string `json:"report_sent"` + ClickedLink string `json:"clicked_link"` + FailureDetails string `json:"failure_details"` } type ConfigRecoveryData struct { @@ -51,9 +52,9 @@ func (*ConfigRecoveryBuilder) New(data *ConfigurationStatusData) ConfigRecoveryD }, Dimensions: ConfigRecoveryDimensions{ Autoconf: data.DataV1.Autoconf, - ReportClick: data.DataV1.ReportClick, - ReportSent: data.DataV1.ReportSent, - ClickedLink: data.DataV1.ClickedLink, + ReportClick: strconv.FormatBool(data.DataV1.ReportClick), + ReportSent: strconv.FormatBool(data.DataV1.ReportSent), + ClickedLink: data.clickedLinkToString(), FailureDetails: data.DataV1.FailureDetails, }, } diff --git a/internal/configstatus/configuration_recovery_test.go b/internal/configstatus/configuration_recovery_test.go index 722e5245..c05ac32a 100644 --- a/internal/configstatus/configuration_recovery_test.go +++ b/internal/configstatus/configuration_recovery_test.go @@ -39,9 +39,9 @@ func TestConfigurationRecovery_default(t *testing.T) { require.Equal(t, "bridge_config_recovery", req.Event) require.Equal(t, 0, req.Values.Duration) require.Equal(t, "", req.Dimensions.Autoconf) - require.Equal(t, false, req.Dimensions.ReportClick) - require.Equal(t, false, req.Dimensions.ReportSent) - require.Equal(t, uint64(0), req.Dimensions.ClickedLink) + require.Equal(t, "false", req.Dimensions.ReportClick) + require.Equal(t, "false", req.Dimensions.ReportSent) + require.Equal(t, "", req.Dimensions.ClickedLink) require.Equal(t, "", req.Dimensions.FailureDetails) } @@ -72,8 +72,8 @@ func TestConfigurationRecovery_fed(t *testing.T) { require.Equal(t, "bridge_config_recovery", req.Event) require.Equal(t, 10, req.Values.Duration) require.Equal(t, "Mr TBird", req.Dimensions.Autoconf) - require.Equal(t, true, req.Dimensions.ReportClick) - require.Equal(t, false, req.Dimensions.ReportSent) - require.Equal(t, uint64(42), req.Dimensions.ClickedLink) + require.Equal(t, "true", req.Dimensions.ReportClick) + require.Equal(t, "false", req.Dimensions.ReportSent) + require.Equal(t, "[1,3,5]", req.Dimensions.ClickedLink) require.Equal(t, "Not an error", req.Dimensions.FailureDetails) } diff --git a/internal/configstatus/configuration_success.go b/internal/configstatus/configuration_success.go index 2089a16e..84c421e1 100644 --- a/internal/configstatus/configuration_success.go +++ b/internal/configstatus/configuration_success.go @@ -18,6 +18,7 @@ package configstatus import ( + "strconv" "time" ) @@ -26,10 +27,10 @@ type ConfigSuccessValues struct { } type ConfigSuccessDimensions struct { - Autoconf string `json:"autoconf"` - ReportClick interface{} `json:"report_click"` - ReportSent interface{} `json:"report_sent"` - ClickedLink uint64 `json:"clicked_link"` + Autoconf string `json:"autoconf"` + ReportClick string `json:"report_click"` + ReportSent string `json:"report_sent"` + ClickedLink string `json:"clicked_link"` } type ConfigSuccessData struct { @@ -50,9 +51,9 @@ func (*ConfigSuccessBuilder) New(data *ConfigurationStatusData) ConfigSuccessDat }, Dimensions: ConfigSuccessDimensions{ Autoconf: data.DataV1.Autoconf, - ReportClick: data.DataV1.ReportClick, - ReportSent: data.DataV1.ReportSent, - ClickedLink: data.DataV1.ClickedLink, + ReportClick: strconv.FormatBool(data.DataV1.ReportClick), + ReportSent: strconv.FormatBool(data.DataV1.ReportSent), + ClickedLink: data.clickedLinkToString(), }, } } diff --git a/internal/configstatus/configuration_success_test.go b/internal/configstatus/configuration_success_test.go index 83b73f24..9f98990f 100644 --- a/internal/configstatus/configuration_success_test.go +++ b/internal/configstatus/configuration_success_test.go @@ -39,9 +39,9 @@ func TestConfigurationSuccess_default(t *testing.T) { require.Equal(t, "bridge_config_success", req.Event) require.Equal(t, 0, req.Values.Duration) require.Equal(t, "", req.Dimensions.Autoconf) - require.Equal(t, false, req.Dimensions.ReportClick) - require.Equal(t, false, req.Dimensions.ReportSent) - require.Equal(t, uint64(0), req.Dimensions.ClickedLink) + require.Equal(t, "false", req.Dimensions.ReportClick) + require.Equal(t, "false", req.Dimensions.ReportSent) + require.Equal(t, "", req.Dimensions.ClickedLink) } func TestConfigurationSuccess_fed(t *testing.T) { @@ -71,7 +71,7 @@ func TestConfigurationSuccess_fed(t *testing.T) { require.Equal(t, "bridge_config_success", req.Event) require.Equal(t, 10, req.Values.Duration) require.Equal(t, "Mr TBird", req.Dimensions.Autoconf) - require.Equal(t, true, req.Dimensions.ReportClick) - require.Equal(t, false, req.Dimensions.ReportSent) - require.Equal(t, uint64(42), req.Dimensions.ClickedLink) + require.Equal(t, "true", req.Dimensions.ReportClick) + require.Equal(t, "false", req.Dimensions.ReportSent) + require.Equal(t, "[1,3,5]", req.Dimensions.ClickedLink) } diff --git a/internal/user/config_status.go b/internal/user/config_status.go index b973989b..8a93c8f3 100644 --- a/internal/user/config_status.go +++ b/internal/user/config_status.go @@ -124,10 +124,12 @@ func (user *User) SendConfigStatusProgress() { if !user.configStatus.IsPending() { return } - var builder configstatus.ConfigProgressBuilder progress := builder.New(user.configStatus.Data) - if progress.Values.NbDaySinceLast == 0 || progress.Values.NbDay == 0 { + if progress.Values.NbDay == 0 { + return + } + if progress.Values.NbDaySinceLast == 0 { return }