From b3de65bcc5627915511182a9ac9857b1fef14853 Mon Sep 17 00:00:00 2001
From: "Z.J. van de Weg" <git@zjvandeweg.nl>
Date: Thu, 26 Jan 2017 09:16:07 +0100
Subject: [PATCH] Update #list_teams to propagate errors

---
 app/controllers/projects/mattermosts_controller.rb        | 5 +----
 .../project_services/mattermost_slash_commands_service.rb | 8 ++++----
 app/views/projects/mattermosts/_no_teams.html.haml        | 5 +++--
 app/views/projects/mattermosts/new.html.haml              | 2 +-
 lib/mattermost/client.rb                                  | 4 ++--
 .../mattermost_slash_commands_service_spec.rb             | 2 +-
 6 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/app/controllers/projects/mattermosts_controller.rb b/app/controllers/projects/mattermosts_controller.rb
index 20161e1cfd8..38f7e6eb5e9 100644
--- a/app/controllers/projects/mattermosts_controller.rb
+++ b/app/controllers/projects/mattermosts_controller.rb
@@ -34,10 +34,7 @@ class Projects::MattermostsController < Projects::ApplicationController
   end
 
   def teams
-    @teams ||= begin
-                 teams, error_message = @service.list_teams(current_user)
-                 error_message ? error_message : teams
-               end
+    @teams, @teams_error_message = @service.list_teams(current_user)
   end
 
   def service
diff --git a/app/models/project_services/mattermost_slash_commands_service.rb b/app/models/project_services/mattermost_slash_commands_service.rb
index 548163f38fa..b0f7a42f9a3 100644
--- a/app/models/project_services/mattermost_slash_commands_service.rb
+++ b/app/models/project_services/mattermost_slash_commands_service.rb
@@ -28,10 +28,10 @@ class MattermostSlashCommandsService < ChatSlashCommandsService
     [false, e.message]
   end
 
-  def list_teams(user)
-    Mattermost::Team.new(user).all
-  rescue Mattermost::Error
-    []
+  def list_teams(current_user)
+    [Mattermost::Team.new(current_user).all, nil]
+  rescue Mattermost::Error => e
+    [[], e.message]
   end
 
   private
diff --git a/app/views/projects/mattermosts/_no_teams.html.haml b/app/views/projects/mattermosts/_no_teams.html.haml
index 1bb3642431e..aac74a25b75 100644
--- a/app/views/projects/mattermosts/_no_teams.html.haml
+++ b/app/views/projects/mattermosts/_no_teams.html.haml
@@ -1,5 +1,6 @@
-= content_for :flash_message do
-  .alert.alert-danger= @teams if @teams.is_a?(String)
+- if @teams_error_message
+  = content_for :flash_message do
+    .alert.alert-danger= @teams_error_message
 
 %p
   You aren’t a member of any team on the Mattermost instance at
diff --git a/app/views/projects/mattermosts/new.html.haml b/app/views/projects/mattermosts/new.html.haml
index 82f596da0fe..96b1d2aee61 100644
--- a/app/views/projects/mattermosts/new.html.haml
+++ b/app/views/projects/mattermosts/new.html.haml
@@ -2,7 +2,7 @@
   .inline.pull-right
     = custom_icon('mattermost_logo', size: 48)
   %h3 Install Mattermost Command
-  - if @teams.is_a?(String) || @teams.empty?
+  - if @teams.empty?
     = render 'no_teams'
   - else
     = render 'team_selection'
diff --git a/lib/mattermost/client.rb b/lib/mattermost/client.rb
index 7552856f37a..ec2903b7ec6 100644
--- a/lib/mattermost/client.rb
+++ b/lib/mattermost/client.rb
@@ -27,12 +27,12 @@ module Mattermost
     end
 
     def json_response(response)
+      json_response = JSON.parse(response.body)
+
       unless response.success?
         raise Mattermost::ClientError.new(json_response['message'] || 'Undefined error')
       end
 
-      json_response = JSON.parse(response.body)
-
       json_response
     rescue JSON::JSONError
       raise Mattermost::ClientError.new('Cannot parse response')
diff --git a/spec/models/project_services/mattermost_slash_commands_service_spec.rb b/spec/models/project_services/mattermost_slash_commands_service_spec.rb
index 57ac63932e6..98f3d420c8a 100644
--- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb
+++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb
@@ -113,7 +113,7 @@ describe MattermostSlashCommandsService, :models do
         end
 
         it 'shows error messages' do
-          expect(subject).to eq([])
+          expect(subject).to eq([[], "Failed to get team list."])
         end
       end
     end