From 8e81f922ea40312bf54000c843439c9c8251ac77 Mon Sep 17 00:00:00 2001
From: Azareal <azareal@users.noreply.github.com>
Date: Tue, 31 Jul 2018 14:02:09 +1000
Subject: [PATCH] Added some new test cases to catch more things and to see if
 the statistics on Code Climate change. Added more TODO ideas for changes.
 Removed an unused field from LanguagePack.

---
 common/counters/common.go | 10 +++++-----
 common/extend.go          |  4 +++-
 common/phrases.go         | 11 +++--------
 misc_test.go              | 21 +++++++++++++++++++++
 routes/panel/analytics.go |  1 +
 5 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/common/counters/common.go b/common/counters/common.go
index 7b331191..d54408a1 100644
--- a/common/counters/common.go
+++ b/common/counters/common.go
@@ -2,11 +2,6 @@ package counters
 
 import "sync"
 
-type RWMutexCounterBucket struct {
-	counter int
-	sync.RWMutex
-}
-
 // TODO: Make a neater API for this
 var routeMapEnum map[string]int
 var reverseRouteMapEnum map[int]string
@@ -40,3 +35,8 @@ func SetOSMapEnum(osme map[string]int) {
 func SetReverseOSMapEnum(rosme map[int]string) {
 	reverseOSMapEnum = rosme
 }
+
+type RWMutexCounterBucket struct {
+	counter int
+	sync.RWMutex
+}
diff --git a/common/extend.go b/common/extend.go
index c99f62dd..7a43d665 100644
--- a/common/extend.go
+++ b/common/extend.go
@@ -1,7 +1,7 @@
 /*
 *
 * Gosora Plugin System
-* Copyright Azareal 2016 - 2018
+* Copyright Azareal 2016 - 2019
 *
  */
 package common
@@ -410,6 +410,8 @@ func (plugin *Plugin) RemoveHook(name string, handler interface{}) {
 	delete(plugin.Hooks, name)
 }
 
+// TODO: Add a HasHook method to complete the AddHook, RemoveHook, etc. set?
+
 var PluginsInited = false
 
 func InitPlugins() {
diff --git a/common/phrases.go b/common/phrases.go
index 280b5eb2..bb248ba0 100644
--- a/common/phrases.go
+++ b/common/phrases.go
@@ -35,8 +35,8 @@ type LevelPhrases struct {
 
 // ! For the sake of thread safety, you must never modify a *LanguagePack directly, but to create a copy of it and overwrite the entry in the sync.Map
 type LanguagePack struct {
-	Name                string
-	Phrases             map[string]string // Should we use a sync map or a struct for these? It would be nice, if we could keep all the phrases consistent.
+	Name string
+	// Should we use a sync map or a struct for these? It would be nice, if we could keep all the phrases consistent.
 	Levels              LevelPhrases
 	GlobalPerms         map[string]string
 	LocalPerms          map[string]string
@@ -140,11 +140,7 @@ func SaveLangPack(langPack *LanguagePack) error {
 	return nil
 }
 
-func GetPhrase(name string) (string, bool) {
-	res, ok := currentLangPack.Load().(*LanguagePack).Phrases[name]
-	return res, ok
-}
-
+// TODO: Merge these two maps?
 func GetGlobalPermPhrase(name string) string {
 	res, ok := currentLangPack.Load().(*LanguagePack).GlobalPerms[name]
 	if !ok {
@@ -152,7 +148,6 @@ func GetGlobalPermPhrase(name string) string {
 	}
 	return res
 }
-
 func GetLocalPermPhrase(name string) string {
 	res, ok := currentLangPack.Load().(*LanguagePack).LocalPerms[name]
 	if !ok {
diff --git a/misc_test.go b/misc_test.go
index cf69a86f..a7893b7b 100644
--- a/misc_test.go
+++ b/misc_test.go
@@ -957,6 +957,27 @@ func TestPluginManager(t *testing.T) {
 	expectNilErr(t, plugin2.SetActive(false))
 	plugin.Deactivate()
 	expectNilErr(t, plugin.SetActive(false))
+
+	// Hook tests
+	expect(t, common.RunSshook("haha", "ho") == "ho", "Sshook shouldn't have anything bound to it yet")
+	var handle = func(in string) (out string) {
+		return in + "hi"
+	}
+	plugin.AddHook("haha", handle)
+	expect(t, common.RunSshook("haha", "ho") == "hohi", "Sshook didn't give hohi")
+	plugin.RemoveHook("haha", handle)
+	expect(t, common.RunSshook("haha", "ho") == "ho", "Sshook shouldn't have anything bound to it anymore")
+
+	// TODO: Add tests for more hook types
+}
+
+func TestPhrases(t *testing.T) {
+	expect(t, common.GetGlobalPermPhrase("BanUsers") == "Can ban users", "Not the expected phrase")
+	expect(t, common.GetGlobalPermPhrase("NoSuchPerm") == "{lang.perms[NoSuchPerm]}", "Not the expected phrase")
+	expect(t, common.GetLocalPermPhrase("ViewTopic") == "Can view topics", "Not the expected phrase")
+	expect(t, common.GetLocalPermPhrase("NoSuchPerm") == "{lang.perms[NoSuchPerm]}", "Not the expected phrase")
+
+	// TODO: Cover the other phrase types, also try switching between languages to see if anything strange happens
 }
 
 func TestSlugs(t *testing.T) {
diff --git a/routes/panel/analytics.go b/routes/panel/analytics.go
index 77c64ad6..b66e1e89 100644
--- a/routes/panel/analytics.go
+++ b/routes/panel/analytics.go
@@ -132,6 +132,7 @@ func AnalyticsViews(w http.ResponseWriter, r *http.Request, user common.User) co
 	revLabelList, labelList, viewMap := analyticsTimeRangeToLabelList(timeRange)
 
 	common.DebugLog("in panel.AnalyticsViews")
+	// TODO: Add some sort of analytics store / iterator?
 	acc := qgen.Builder.Accumulator()
 	rows, err := acc.Select("viewchunks").Columns("count, createdAt").Where("route = ''").DateCutoff("createdAt", timeRange.Quantity, timeRange.Unit).Query()
 	if err != nil && err != sql.ErrNoRows {