From f928b4c84e960bec4ee3273e70d1192ad979d7ed Mon Sep 17 00:00:00 2001
From: Kegan Dougal <kegan@matrix.org>
Date: Thu, 11 Aug 2016 15:11:45 +0100
Subject: [PATCH] Review comments

---
 .../matrix-org/go-neb/database/db.go          |  1 +
 .../matrix-org/go-neb/database/schema.go      |  2 +-
 .../matrix-org/go-neb/realms/jira/jira.go     |  3 +-
 .../matrix-org/go-neb/services/jira/jira.go   | 29 ++++++++++++++-----
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/github.com/matrix-org/go-neb/database/db.go b/src/github.com/matrix-org/go-neb/database/db.go
index c07e73b..eca84a9 100644
--- a/src/github.com/matrix-org/go-neb/database/db.go
+++ b/src/github.com/matrix-org/go-neb/database/db.go
@@ -185,6 +185,7 @@ func (d *ServiceDB) LoadAuthRealm(realmID string) (realm types.AuthRealm, err er
 }
 
 // LoadAuthRealmsByType loads all auth realms with the given type from the database.
+// The realms are ordered based on their realm ID.
 // Returns an empty list if there are no realms with that type.
 func (d *ServiceDB) LoadAuthRealmsByType(realmType string) (realms []types.AuthRealm, err error) {
 	err = runTransaction(d.db, func(txn *sql.Tx) error {
diff --git a/src/github.com/matrix-org/go-neb/database/schema.go b/src/github.com/matrix-org/go-neb/database/schema.go
index bd423d6..c2046d9 100644
--- a/src/github.com/matrix-org/go-neb/database/schema.go
+++ b/src/github.com/matrix-org/go-neb/database/schema.go
@@ -263,7 +263,7 @@ func selectRealmTxn(txn *sql.Tx, realmID string) (types.AuthRealm, error) {
 }
 
 const selectRealmsByTypeSQL = `
-SELECT realm_id, realm_json FROM auth_realms WHERE realm_type = $1
+SELECT realm_id, realm_json FROM auth_realms WHERE realm_type = $1 ORDER BY realm_id
 `
 
 func selectRealmsByTypeTxn(txn *sql.Tx, realmType string) (realms []types.AuthRealm, err error) {
diff --git a/src/github.com/matrix-org/go-neb/realms/jira/jira.go b/src/github.com/matrix-org/go-neb/realms/jira/jira.go
index 22832ca..142e09d 100644
--- a/src/github.com/matrix-org/go-neb/realms/jira/jira.go
+++ b/src/github.com/matrix-org/go-neb/realms/jira/jira.go
@@ -262,8 +262,7 @@ func (r *JIRARealm) JIRAClient(userID string, allowUnauth bool) (*jira.Client, e
 	if jsession.AccessSecret == "" || jsession.AccessToken == "" {
 		if allowUnauth {
 			// make an unauthenticated client
-			cli, err = jira.NewClient(nil, r.JIRAEndpoint)
-			return cli, err
+			return jira.NewClient(nil, r.JIRAEndpoint)
 		}
 		return nil, errors.New("No authenticated session found for " + userID)
 	}
diff --git a/src/github.com/matrix-org/go-neb/services/jira/jira.go b/src/github.com/matrix-org/go-neb/services/jira/jira.go
index bd6b75f..a9a7100 100644
--- a/src/github.com/matrix-org/go-neb/services/jira/jira.go
+++ b/src/github.com/matrix-org/go-neb/services/jira/jira.go
@@ -81,9 +81,17 @@ func (s *jiraService) Plugin(roomID string) plugin.Plugin {
 						},
 					}
 					cli, err := r.JIRAClient(userID, false)
+					if err != nil {
+						return nil, err
+					}
 					i, res, err := cli.Issue.Create(&iss)
 					if err != nil {
-						log.WithError(err).Print("Failed to create issue")
+						log.WithFields(log.Fields{
+							log.ErrorKey: err,
+							"user_id":    userID,
+							"project":    pkey,
+							"realm_id":   r.ID(),
+						}).Print("Failed to create issue")
 						return nil, errors.New("Failed to create issue")
 					}
 					if res.StatusCode < 200 || res.StatusCode >= 300 {
@@ -123,34 +131,39 @@ func (s *jiraService) projectToRealm(userID, pkey string) (*realms.JIRARealm, er
 	}
 	// typecast and move ones which the user has authed with to the front of the queue
 	var queue []*realms.JIRARealm
+	var unauthRealms []*realms.JIRARealm
 	for _, r := range knownRealms {
 		jrealm, ok := r.(*realms.JIRARealm)
 		if !ok {
 			logger.WithField("realm_id", r.ID()).Print(
-				"Failed to type-cast 'jira' type realm into JIRARealm")
+				"Failed to type-cast 'jira' type realm into JIRARealm",
+			)
 			continue
 		}
 
 		_, err := database.GetServiceDB().LoadAuthSessionByUser(r.ID(), userID)
 		if err != nil {
 			if err == sql.ErrNoRows {
-				// not authed with this JIRA realm; add to back of queue.
-				queue = append(queue, jrealm)
+				unauthRealms = append(unauthRealms, jrealm)
 			} else {
 				logger.WithError(err).WithField("realm_id", r.ID()).Print(
-					"Failed to load auth sessions for user")
+					"Failed to load auth sessions for user",
+				)
 			}
 			continue // this may not have been the match anyway so don't give up!
 		}
-		// authed with this JIRA realm; add to front of queue
-		queue = append([]*realms.JIRARealm{jrealm}, queue...)
+		queue = append(queue, jrealm)
 	}
 
+	// push unauthed realms to the back
+	queue = append(queue, unauthRealms...)
+
 	for _, jr := range queue {
 		exists, err := jr.ProjectKeyExists(userID, pkey)
 		if err != nil {
 			logger.WithError(err).WithField("realm_id", jr.ID()).Print(
-				"Failed to check if project key exists on this realm.")
+				"Failed to check if project key exists on this realm.",
+			)
 			continue // may not have been found anyway so keep searching!
 		}
 		if exists {