Merge branch 'zj-mattermost-session' into zj-mattermost-slash-config

app/controllers/projects/mattermost_controller.rb

@@ -8,15 +8,15 @@ class Projects::MattermostController < Projects::ApplicationController
   end
 
   def configure
-    @service.configure(host, current_user, params)
+    @service.configure(host, current_user, configure_params)
 
     redirect_to(
-      new_namespace_project_service_path(@project.namespace, @project, @service.to_param),
+      new_namespace_project_mattermost_path(@project.namespace, @project),
       notice: 'This service is now configured.'
     )
-  rescue Mattermost::NoSessionError
+  rescue NoSessionError
     redirect_to(
-      edit_namespace_project_service_path(@project.namespace, @project, @service.to_param),
+      new_namespace_project_mattermost_path(@project.namespace, @project),
       alert: 'No session could be set up, is Mattermost configured with Single Sign on?'
     )
   end
@@ -24,11 +24,11 @@ class Projects::MattermostController < Projects::ApplicationController
   private
 
   def configure_params
-    params.require(:configure_params).permit(:trigger, :team_id)
+    params.permit(:trigger, :team_id).merge(url: service_trigger_url(@service), icon_url: asset_url('gitlab_logo.png'))
   end
 
   def service
-    @service ||= @project.services.find_by(type: 'MattermostSlashCommandsService')
+    @service ||= @project.find_or_initialize_service('mattermost_slash_commands')
   end
 
   def teams
@@ -37,7 +37,7 @@ class Projects::MattermostController < Projects::ApplicationController
         Mattermost::Mattermost.new(Gitlab.config.mattermost.host, current_user).with_session do
           Mattermost::Team.team_admin
         end
-      rescue Mattermost::NoSessionError
+      rescue
         []
       end
   end

app/models/project_services/mattermost_slash_commands_service.rb

@@ -25,15 +25,15 @@ class MattermostSlashCommandsService < ChatService
     ]
   end
 
-  def configure(host, current_user, params)
-    token = Mattermost::Mattermost.new(host, current_user).with_session do
-      Mattermost::Commands.create(params[:team_id],
-                                  trigger: params[:trigger] || @service.project.path,
-                                  url: service_trigger_url(@service),
-                                  icon_url: asset_url('gitlab_logo.png'))
+  def configure(host, current_user, team_id:, trigger:, url:, icon_url:)
+    new_token = Mattermost::Session.new(host, current_user).with_session do
+      Mattermost::Command.create(team_id,
+                                 trigger: trigger || @service.project.path,
+                                 url: url,
+                                 icon_url: icon_url)
     end
 
-    update_attributes(token: token)
+    update!(token: new_token)
   end
 
   def trigger(params)

app/views/projects/mattermost/new.html.haml

 = "hello world"
-= form_for(:create, method: :post, url: configure_namespace_project_mattermost_path(@project.namespace, @project, @service.to_param)) do |f|
+= @teams
+= form_for(:create, method: :post, url: configure_namespace_project_mattermost_path(@project.namespace, @project, )) do |f|
   = "Team ID"
   = f.text_field(:team_id)
   = f.submit 'Configure', class: 'btn btn-save'

lib/mattermost/command.rb

 module Mattermost
   class Command < Session
-    def self.all(team_id)
-      get("/teams/#{team_id}/commands/list_team_commands").parsed_response
-    end
-
-    # params should be a hash, which supplies _at least_:
-    # - trigger => The slash command, no spaces, cannot start with a /
-    # - url => What is the URL to trigger here?
-    # - icon_url => Supply a link to the icon
-    def self.create(team_id, params)
+    def self.create(team_id, trigger: 'gitlab', url:, icon_url:)
       command = {
         auto_complete: true,
         auto_complete_desc: 'List all available commands',
         auto_complete_hint: '[help]',
         description: 'Perform common operations on GitLab',
-        display_name: 'GitLab',
+        display_name: 'GitLab Slash Commands',
         method: 'P',
         user_name: 'GitLab',
-        trigger: 'gitlab',
-      }.merge(params)
+        trigger: trigger,
+        url: url,
+        icon_url: icon_url
+      }
 
       response = post( "/teams/#{team_id}/commands/create", body: command.to_json)
 

lib/mattermost/session.rb

@@ -21,6 +21,8 @@ module Mattermost
 
     def initialize(uri, current_user)
       uri = normalize_uri(uri)
+
+      # Sets the base uri for HTTParty, so we can use paths
       self.class.base_uri(uri)
 
       @current_resource_owner = current_user
@@ -28,12 +30,14 @@ module Mattermost
 
     def with_session
       raise NoSessionError unless create
-      result = yield
-      destroy
 
-      result
-    rescue Errno::ECONNREFUSED
-      raise NoSessionError
+      begin
+        yield
+      rescue Errno::ECONNREFUSED
+        raise NoSessionError
+      ensure
+        destroy
+      end
     end
 
     # Next methods are needed for Doorkeeper

spec/lib/mattermost/session_spec.rb

@@ -5,7 +5,7 @@ describe Mattermost::Session do
 
   subject { described_class.new('http://localhost:8065', user) }
 
-  # Needed for doorman to function
+  # Needed for doorkeeper to function
   it { is_expected.to respond_to(:current_resource_owner) }
   it { is_expected.to respond_to(:request) }
   it { is_expected.to respond_to(:authorization) }