From c2de2873ff919cf8c8275f49c995e73e9e4170cc Mon Sep 17 00:00:00 2001
From: Marcel Schramm <marceloschr@googlemail.com>
Date: Sun, 25 Oct 2020 12:22:52 +0100
Subject: [PATCH] Refactor version checking slightly

---
 app/app.go            | 14 ++-------
 version/check.go      | 66 +++++++++++++++++++++++++------------------
 version/check_test.go |  2 +-
 version/version.go    |  4 +++
 4 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/app/app.go b/app/app.go
index 0bdb7463..f0a419d5 100644
--- a/app/app.go
+++ b/app/app.go
@@ -47,15 +47,7 @@ func RunWithAccount(account string) {
 		configuration.Token = configuration.GetAccountToken(account)
 	}
 
-	updateAvailableChannel := make(chan bool, 1)
-	if configuration.ShowUpdateNotifications {
-		go func() {
-			updateAvailableChannel <- version.IsLocalOutdated(configuration.DontShowUpdateNotificationFor)
-		}()
-	} else {
-		updateAvailableChannel <- false
-	}
-
+	updateAvailableChannel := version.CheckForUpdate(configuration.DontShowUpdateNotificationFor)
 	app.MouseEnabled = configuration.MouseEnabled
 
 	go func() {
@@ -78,9 +70,7 @@ func RunWithAccount(account string) {
 
 		readstate.Load(discord.State)
 
-		isUpdateAvailable := <-updateAvailableChannel
-		close(updateAvailableChannel)
-		if isUpdateAvailable {
+		if isUpdateAvailable := <-updateAvailableChannel; isUpdateAvailable {
 			waitForUpdateDialogChannel := make(chan bool, 1)
 
 			dialog := tview.NewModal()
diff --git a/version/check.go b/version/check.go
index 3af256ef..4b2c4804 100644
--- a/version/check.go
+++ b/version/check.go
@@ -7,44 +7,54 @@ import (
 	"github.com/google/go-github/v29/github"
 )
 
-var latestRemoteVersion *string
-
-// IsLocalOutdated checks the latest release on github and compares the version
-// numbers. Version numbers representing a later date than the local version
-// will cause the call to return true. If an error occurs, we will return
-// false. If dontRemindFor is passed, false will be returned if it equals the
-// current remote version. This is used in order to avoid spamming the user
-// with an update notification on every startup.
-func IsLocalOutdated(dontRemindFor string) bool {
-	if GetLatestRemoteVersion() == "" || GetLatestRemoteVersion() == dontRemindFor {
-		return false
-	}
-
-	return isCurrentOlder(Version, GetLatestRemoteVersion())
+var latestRemoteVersion string
+
+// CheckForUpdate checks whether the Version-string saved at version.Version
+// is different to the name of the latest tagged GitHub release. If it is, and
+// the release name isn't equal to the `donRemindFor`-parameter, we return
+// true, which stands for "update available". This allows the user to manually
+// say "Don't remind me again for this version". The actual return value is
+// supplied via a channel that's closed after one value has been read.
+func CheckForUpdate(dontRemindFor string) chan bool {
+	//Note, this isn't buffered, so that we can safely close the channel
+	//when it has been read by the caller.
+	updateAvailableChannel := make(chan bool)
+	go func() {
+		remoteVersion := GetLatestRemoteVersion()
+		if remoteVersion == "" || remoteVersion == dontRemindFor {
+			//Error retrieving version or user wishes to ignore update.
+			updateAvailableChannel <- false
+		} else {
+			updateAvailableChannel <- isLocalOlderThanRemote(Version, remoteVersion)
+		}
+		close(updateAvailableChannel)
+	}()
+	return updateAvailableChannel
 }
 
-func isCurrentOlder(current, other string) bool {
-	var yearRemote, monthRemote, dayRemote int
-	fmt.Sscanf(other, "%04d-%02d-%02d", &yearRemote, &monthRemote, &dayRemote)
-	var yearLocal, monthLocal, dayLocal int
-	fmt.Sscanf(current, "%04d-%02d-%02d", &yearLocal, &monthLocal, &dayLocal)
+func isLocalOlderThanRemote(local, remote string) bool {
+	yearRemote, monthRemote, dayRemote := parseTag(remote)
+	yearLocal, monthLocal, dayLocal := parseTag(local)
 	return !(yearLocal >= yearRemote && monthLocal >= monthRemote && dayLocal >= dayRemote)
 }
 
+func parseTag(tag string) (year int, month int, day int) {
+	fmt.Sscanf(tag, "%04d-%02d-%02d", &year, &month, &day)
+	return
+}
+
 // GetLatestRemoteVersion queries GitHub for the latest Release-Tag and caches
-// it. This value will never be updated during runtime of cordless.
+// it. This value will never be updated during runtime.
 func GetLatestRemoteVersion() string {
-	if latestRemoteVersion != nil {
-		return *latestRemoteVersion
+	if latestRemoteVersion != "" {
+		return latestRemoteVersion
 	}
 
 	repositoryRelease, _, lookupError := github.NewClient(nil).Repositories.GetLatestRelease(context.Background(), "Bios-Marcel", "cordless")
-	if lookupError == nil {
-		latestRemoteVersion = repositoryRelease.TagName
-	} else {
-		emptyVersion := ""
-		latestRemoteVersion = &emptyVersion
+	if lookupError != nil || repositoryRelease == nil {
+		return ""
 	}
 
-	return *latestRemoteVersion
+	latestRemoteVersion = *repositoryRelease.TagName
+	return latestRemoteVersion
 }
diff --git a/version/check_test.go b/version/check_test.go
index b0ea34e5..ea6663d6 100644
--- a/version/check_test.go
+++ b/version/check_test.go
@@ -58,7 +58,7 @@ func Test_isCurrentOlder(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := isCurrentOlder(tt.args.current, tt.args.other); got != tt.want {
+			if got := isLocalOlderThanRemote(tt.args.current, tt.args.other); got != tt.want {
 				t.Errorf("isCurrentOlder() = %v, want %v", got, tt.want)
 			}
 		})
diff --git a/version/version.go b/version/version.go
index 7a1e2471..d8c8e137 100644
--- a/version/version.go
+++ b/version/version.go
@@ -1,3 +1,7 @@
 package version
 
+// Version reflects the latest release date. For local builds, this number
+// isn't really correct. Therefore new commits also won't lead to an update
+// reminder, unless the latest tag is different to this number. This should
+// not be touched manually.
 var Version = "2020-10-24"