Improve sources

- Add proper error handling,
- Use flash[:alert] and flash[:notice],
- Use `resource` instead of `resources`,

app/controllers/projects/mattermost_controller.rb → app/controllers/projects/mattermosts_controller.rb

-class Projects::MattermostController < Projects::ApplicationController
+class Projects::MattermostsController < Projects::ApplicationController
+  include TriggersHelper
+  include ActionView::Helpers::AssetUrlHelper
+
   layout 'project_settings'
+
   before_action :authorize_admin_project!
   before_action :service
   before_action :teams, only: [:new]
@@ -8,32 +12,30 @@ class Projects::MattermostController < Projects::ApplicationController
   end
 
   def create
-    message = @service.configure(current_user, configure_params)
-    notice = message.is_a?(String) ? message : 'This service is now configured'
-
-    redirect_to(
-      new_namespace_project_mattermost_path(@project.namespace, @project),
-      notice: notice
-    )
-  rescue NoSessionError
-    redirect_to(
-      new_namespace_project_mattermost_path(@project.namespace, @project),
-      alert: 'No session could be set up, is Mattermost configured with Single Sign on?'
-    )
+    @service.configure!(current_user, configure_params)
+
+    flash[:notice] = 'This service is now configured'
+    redirect_to edit_namespace_project_service_path(@project.namespace, @project, service)
+  rescue => e
+    flash[:alert] = e.message
+    redirect_to new_namespace_project_mattermost_path(@project.namespace, @project)
   end
 
   private
 
   def configure_params
-    params.permit(:trigger, :team_id).
-      merge(url: service_trigger_url(@service), icon_url: asset_url('gitlab_logo.png'))
+    params.require(:mattermost).permit(:trigger, :team_id).merge(
+      url: service_trigger_url(@service),
+      icon_url: asset_url('gitlab_logo.png'))
   end
 
-  def service
-    @service ||= @project.find_or_initialize_service('mattermost_slash_commands')
+  def teams
+    @teams ||= @service.list_teams(current_user)
+  rescue => e
+    flash[:alert] = e.message
   end
 
-  def teams
-    @teams = @service.list_teams(current_user)
+  def service
+    @service ||= @project.find_or_initialize_service('mattermost_slash_commands')
   end
 end

app/helpers/projects_helper.rb

@@ -149,12 +149,11 @@ module ProjectsHelper
   end
 
   def mattermost_teams_options(teams)
-    teams_options = teams.map do |team|
-      return nil unless team['display_name'] && team['id']
-      [team['display_name'], team['id']]
+    teams_options = teams.map do |id, options|
+      return nil unless id && options['display_name']
+      [options['display_name'], id]
     end.compact
-    teams_options.unshift(['Select team...', '0']) unless teams_options.one?
-    teams_options
+    teams_options.unshift(['Select team...', '0'])
   end
 
   private

app/models/project_services/mattermost_slash_commands_service.rb

@@ -25,29 +25,18 @@ class MattermostSlashCommandsService < ChatService
     ]
   end
 
-  def configure(current_user, params)
-    result = Mattermost::Session.new(current_user).with_session do |session|
-      Mattermost::Command.create(session, params[:team_id], command)
+  def configure!(current_user, params)
+    token = Mattermost::Session.new(current_user).with_session do |session|
+      Mattermost::Command.create(session, command(params))
     end
 
-    if result.has_key?('message')
-      result['message']
-    else
-      update!(token: result['token'], active: true)
-    end
+    update!(active: true, token: token)
   end
 
-  def list_teams(current_user)
-    begin
-      response = Mattermost::Session.new(current_user).with_session do |session|
+  def list_teams(user)
+    Mattermost::Session.new(user).with_session do |session|
       Mattermost::Team.all(session)
     end
-
-      # We ignore the error message as we can't display it
-      response.has_key?('message') ? [] : response
-    rescue Mattermost::NoSessionError
-      []
-    end
   end
 
   def trigger(params)
@@ -64,21 +53,17 @@ class MattermostSlashCommandsService < ChatService
 
   private
 
-  def command(trigger:, url:, icon_url:)
+  def command(params)
     pretty_project_name = project.name_with_namespace
 
-    {
+    params.merge(
       auto_complete: true,
       auto_complete_desc: "Perform common operations on: #{pretty_project_name}",
       auto_complete_hint: '[help]',
       description: "Perform common operations on: #{pretty_project_name}",
       display_name: "GitLab  / #{pretty_project_name}",
       method: 'P',
-      user_name: 'GitLab',
-      trigger: trigger,
-      url: url,
-      icon_url: icon_url
-    }
+      user_name: 'GitLab')
   end
 
   def find_chat_user(params)

app/views/projects/mattermost/_team_selection.html.haml → app/views/projects/mattermosts/_team_selection.html.haml

 %p
   This service will be installed on the Mattermost instance at
-  %strong= Gitlab.config.mattermost.host
+  %strong= link_to Gitlab.config.mattermost.host, Gitlab.config.mattermost.host
 %hr
-= form_for(:create, method: :post, url: configure_namespace_project_mattermost_index_path(@project.namespace, @project)) do |f|
+= form_for(:mattermost, method: :post, url: namespace_project_mattermost_path(@project.namespace, @project)) do |f|
   %h4 Team
   %p Select or create the team where the slash commands will be used in
-  - options = mattermost_teams_options(@teams)
-  = f.select(:team_id, options, {}, { class: 'form-control', selected: "#{options.first[1] if options.count.one?}", disabled: options.count.one? })
+  - selected_id = @teams.keys.first if @teams.one?
+  = f.select(:team_id, mattermost_teams_options(@teams), {}, { class: 'form-control', selected: "#{selected_id}", disabled: @teams.one? })
   .help-block
-    - if options.count.one?
+    - if @teams.one?
       This is the only team where you are an administrator.
     - else
       The list shows teams where you are administrator

app/views/projects/services/mattermost_slash_commands/_installation_info.html.haml

@@ -5,13 +5,10 @@
   .row
     %strong.col-sm-3.text-right Mattermost
     = link_to pretty_url(Gitlab.config.mattermost.host), Gitlab.config.mattermost.host, class: 'col-sm-9', target: '__blank'
+  - unless @service.activated?
     .row
       %strong.col-sm-3.text-right Installation
       .col-sm-9
-      - if @service.activated?
-        To edit or uninstall this service, press
-        %strong Edit in Mattermost
-      - else
           To install this service, press
           %strong Add to Mattermost
           and follow the instructions
@@ -19,4 +16,4 @@
       .col-sm-9.col-sm-offset-3
         = link_to new_namespace_project_mattermost_path(@project.namespace, @project), class: 'btn btn-lg' do
           = custom_icon('mattermost_logo', size: 15)
-        = @service.activated? ? 'Edit in Mattermost' : 'Add to Mattermost'
+          = 'Add to Mattermost'

config/routes/project.rb

@@ -76,7 +76,7 @@ constraints(ProjectUrlConstrainer.new) do
         end
       end
 
-      resources :mattermost, only: [:new, :create]
+      resource :mattermost, only: [:new, :create]
 
       resources :deploy_keys, constraints: { id: /\d+/ }, only: [:index, :new, :create] do
         member do

lib/mattermost/command.rb

 module Mattermost
   class Command
-    def self.create(session, team_id, command)
-      response = session.post("/api/v3/teams/#{team_id}/commands/create", body: command.to_json).parsed_response
+    def self.create(session, params)
+      response = session.post("/api/v3/teams/#{params[:team_id]}/commands/create",
+        body: params.to_json)
 
-      if response.has_key?('message')
-        response
+      if response.success?
+        response.parsed_response['token']
+      elsif response.parsed_response.try(:has_key?, 'message')
+        raise response.parsed_response['message']
       else
-        response['token']
+        raise 'Failed to create a new command'
       end
     end
   end

lib/mattermost/session.rb

 module Mattermost
-  class NoSessionError < StandardError; end
+  class NoSessionError < StandardError
+    def message
+      'No session could be set up, is Mattermost configured with Single Sign on?'
+    end
+  end
+
   # This class' prime objective is to obtain a session token on a Mattermost
   # instance with SSO configured where this GitLab instance is the provider.
   #

lib/mattermost/team.rb

 module Mattermost
   class Team
     def self.all(session)
-      session.get('/api/v3/teams/all').parsed_response
+      response = session.get('/api/v3/teams/all')
+
+      if response.success?
+        response.parsed_response
+      elsif response.parsed_response.try(:has_key?, 'message')
+        raise response.parsed_response['message']
+      else
+        raise 'Failed to list teams'
+      end
     end
   end
 end