diff options
author | Kamil Trzciński <ayufan@ayufan.eu> | 2017-03-06 12:13:39 +0000 |
---|---|---|
committer | Kamil Trzciński <ayufan@ayufan.eu> | 2017-03-06 12:13:39 +0000 |
commit | 498351cfacfc09b5a33281838f0f366a6b2961df (patch) | |
tree | 6339b8c98c9eb0e2ac34d7c87c74e02b2631434b | |
parent | 21d539855c6a174f89cf34a93ecca9eb912288bc (diff) | |
parent | 3b89ebb81c828dec546e32093aeebdeb8a7e1829 (diff) | |
download | gitlab-ce-498351cfacfc09b5a33281838f0f366a6b2961df.tar.gz |
Merge branch '26384-improve-omnibus-mattermost-command-installation-flow' into 'master'
Resolve "Improve Omnibus-Mattermost Command installation flow"
See merge request !9257
-rw-r--r-- | app/assets/stylesheets/framework/layout.scss | 13 | ||||
-rw-r--r-- | app/helpers/mattermost_helper.rb | 6 | ||||
-rw-r--r-- | app/views/layouts/application.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/mattermosts/_team_selection.html.haml | 13 | ||||
-rw-r--r-- | app/views/projects/mattermosts/new.html.haml | 2 | ||||
-rw-r--r-- | lib/mattermost/team.rb | 2 | ||||
-rw-r--r-- | spec/features/projects/services/mattermost_slash_command_spec.rb | 42 | ||||
-rw-r--r-- | spec/lib/mattermost/team_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/project_services/mattermost_slash_commands_service_spec.rb | 2 |
9 files changed, 57 insertions, 31 deletions
diff --git a/app/assets/stylesheets/framework/layout.scss b/app/assets/stylesheets/framework/layout.scss index 29d55c44699..0a42b17c1f5 100644 --- a/app/assets/stylesheets/framework/layout.scss +++ b/app/assets/stylesheets/framework/layout.scss @@ -8,6 +8,19 @@ body { &.navless { background-color: $white-light !important; } + + &.card-content { + background-color: $gray-darker; + + .content-wrapper { + padding: 0; + + .container-fluid, + .container-limited { + background-color: $gray-darker; + } + } + } } .container { diff --git a/app/helpers/mattermost_helper.rb b/app/helpers/mattermost_helper.rb index 49ac12db832..27ff4051c8d 100644 --- a/app/helpers/mattermost_helper.rb +++ b/app/helpers/mattermost_helper.rb @@ -1,9 +1,7 @@ module MattermostHelper def mattermost_teams_options(teams) - teams_options = teams.map do |id, options| - [options['display_name'] || options['name'], id] + teams.map do |team| + [team['display_name'] || team['name'], team['id']] end - - teams_options.compact.unshift(['Select team...', '0']) end end diff --git a/app/views/layouts/application.html.haml b/app/views/layouts/application.html.haml index 19bd9b6d5c9..36543edc040 100644 --- a/app/views/layouts/application.html.haml +++ b/app/views/layouts/application.html.haml @@ -1,7 +1,7 @@ !!! 5 %html{ lang: "en", class: "#{page_class}" } = render "layouts/head" - %body{ data: { page: body_data_page, project: "#{@project.path if @project}", group: "#{@group.path if @group}" } } + %body{ class: @body_class, data: { page: body_data_page, project: "#{@project.path if @project}", group: "#{@group.path if @group}" } } = Gon::Base.render_data = render "layouts/header/default", title: header_title diff --git a/app/views/projects/mattermosts/_team_selection.html.haml b/app/views/projects/mattermosts/_team_selection.html.haml index a80f9aa4c4a..04bd4e8b683 100644 --- a/app/views/projects/mattermosts/_team_selection.html.haml +++ b/app/views/projects/mattermosts/_team_selection.html.haml @@ -2,16 +2,15 @@ This service will be installed on the Mattermost instance at %strong= link_to Gitlab.config.mattermost.host, Gitlab.config.mattermost.host %hr -= form_for(:mattermost, method: :post, url: namespace_project_mattermost_path(@project.namespace, @project)) do |f| += form_for(:mattermost, method: :post, url: namespace_project_mattermost_path(@project.namespace, @project), html: { class: 'js-requires-input'} ) do |f| %h4 Team %p = @teams.one? ? 'The team' : 'Select the team' where the slash commands will be used in - - selected_id = @teams.one? ? @teams.keys.first : 0 - - options = mattermost_teams_options(@teams) - - options = options_for_select(options, selected_id) - = f.select(:team_id, options, {}, { class: 'form-control', disabled: @teams.one?, selected: selected_id }) - = f.hidden_field(:team_id, value: selected_id) if @teams.one? + - selected_id = @teams.one? ? @teams.first['id'] : nil + - options = options_for_select(mattermost_teams_options(@teams), selected_id) + = f.select(:team_id, options, { include_blank: 'Select team...'}, { class: 'form-control', disabled: @teams.one?, selected: selected_id, required: true }) + = f.hidden_field(:team_id, value: selected_id, required: true) if @teams.one? .help-block - if @teams.one? This is the only available team. @@ -25,7 +24,7 @@ %hr %h4 Command trigger word %p Choose the word that will trigger commands - = f.text_field(:trigger, value: @project.path, class: 'form-control') + = f.text_field(:trigger, value: @project.path, class: 'form-control', required: true) .help-block %p Trigger word must be unique, and can't begin with a slash or contain any spaces. diff --git a/app/views/projects/mattermosts/new.html.haml b/app/views/projects/mattermosts/new.html.haml index 96b1d2aee61..15829a3f143 100644 --- a/app/views/projects/mattermosts/new.html.haml +++ b/app/views/projects/mattermosts/new.html.haml @@ -1,3 +1,5 @@ +- @body_class = 'card-content' + .service-installation .inline.pull-right = custom_icon('mattermost_logo', size: 48) diff --git a/lib/mattermost/team.rb b/lib/mattermost/team.rb index 09dfd082b3a..afc152aa02e 100644 --- a/lib/mattermost/team.rb +++ b/lib/mattermost/team.rb @@ -1,7 +1,7 @@ module Mattermost class Team < Client def all - session_get('/api/v3/teams/all') + session_get('/api/v3/teams/all').values end end end diff --git a/spec/features/projects/services/mattermost_slash_command_spec.rb b/spec/features/projects/services/mattermost_slash_command_spec.rb index f5adb53a2dc..24d22a092d4 100644 --- a/spec/features/projects/services/mattermost_slash_command_spec.rb +++ b/spec/features/projects/services/mattermost_slash_command_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'Setup Mattermost slash commands', feature: true do +feature 'Setup Mattermost slash commands', :feature, :js do let(:user) { create(:user) } let(:project) { create(:empty_project) } let(:service) { project.create_mattermost_slash_commands_service } @@ -62,11 +62,11 @@ feature 'Setup Mattermost slash commands', feature: true do click_link 'Add to Mattermost' - team_name = teams.first[1]['display_name'] - select_element = find('select#mattermost_team_id') + team_name = teams.first['display_name'] + select_element = find('#mattermost_team_id') selected_option = select_element.find('option[selected]') - expect(select_element['disabled']).to eq('disabled') + expect(select_element['disabled']).to be(true) expect(selected_option).to have_content(team_name.to_s) end @@ -75,7 +75,7 @@ feature 'Setup Mattermost slash commands', feature: true do click_link 'Add to Mattermost' - expect(find('input#mattermost_team_id', visible: false).value).to eq(teams.first[0].to_s) + expect(find('input#mattermost_team_id', visible: false).value).to eq(teams.first['id']) end it 'shows an explanation user is a member of multiple teams' do @@ -92,12 +92,9 @@ feature 'Setup Mattermost slash commands', feature: true do click_link 'Add to Mattermost' - select_element = find('select#mattermost_team_id') - selected_option = select_element.find('option[selected]') + select_element = find('#mattermost_team_id') - expect(select_element['disabled']).to be(nil) - expect(selected_option).to have_content('Select team...') - # The 'Select team...' placeholder is item `0`. + expect(select_element['disabled']).to be(false) expect(select_element.all('option').count).to eq(3) end @@ -110,20 +107,37 @@ feature 'Setup Mattermost slash commands', feature: true do expect(page).to have_content('test mattermost error message') end + it 'enables the submit button if the required fields are provided', :js do + stub_teams(count: 1) + + click_link 'Add to Mattermost' + + expect(find('input[type="submit"]')['disabled']).not_to be(true) + end + + it 'disables the submit button if the required fields are not provided', :js do + stub_teams(count: 1) + + click_link 'Add to Mattermost' + + fill_in('mattermost_trigger', with: '') + + expect(find('input[type="submit"]')['disabled']).to be(true) + end + def stub_teams(count: 0) teams = create_teams(count) - allow_any_instance_of(MattermostSlashCommandsService).to receive(:list_teams) { teams } + allow_any_instance_of(MattermostSlashCommandsService).to receive(:list_teams) { [teams, nil] } teams end def create_teams(count = 0) - teams = {} + teams = [] count.times do |i| - i += 1 - teams[i] = { id: i, display_name: i } + teams.push({ "id" => "x#{i}", "display_name" => "x#{i}-name" }) end teams diff --git a/spec/lib/mattermost/team_spec.rb b/spec/lib/mattermost/team_spec.rb index 6a9c7cebfff..ac493fdb20f 100644 --- a/spec/lib/mattermost/team_spec.rb +++ b/spec/lib/mattermost/team_spec.rb @@ -13,7 +13,7 @@ describe Mattermost::Team do context 'for valid request' do let(:response) do - [{ + { "xiyro8huptfhdndadpz8r3wnbo" => { "id" => "xiyro8huptfhdndadpz8r3wnbo", "create_at" => 1482174222155, "update_at" => 1482174222155, @@ -26,7 +26,7 @@ describe Mattermost::Team do "allowed_domains" => "", "invite_id" => "o4utakb9jtb7imctdfzbf9r5ro", "allow_open_invite" => false - }] + } } end before do @@ -39,7 +39,7 @@ describe Mattermost::Team do end it 'returns a token' do - is_expected.to eq(response) + is_expected.to eq(response.values) end end 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 598ff232c82..f9531be5d25 100644 --- a/spec/models/project_services/mattermost_slash_commands_service_spec.rb +++ b/spec/models/project_services/mattermost_slash_commands_service_spec.rb @@ -92,7 +92,7 @@ describe MattermostSlashCommandsService, :models do to_return( status: 200, headers: { 'Content-Type' => 'application/json' }, - body: ['list'].to_json + body: { 'list' => true }.to_json ) end |