From 0428092a287feca0cf301fdf1b7496b239816733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADtor=20Avelino?= Date: Wed, 26 Jul 2017 11:22:29 -0300 Subject: [PATCH] js: webhook headers refactoring --- .../components/new-webhook-header-form.js | 25 +++++ .../components/webhook-headers-panel.js | 35 +++++++ .../modules/webhooks/pages/show.js | 6 +- app/assets/javascripts/teams.js | 17 ---- .../webhook_headers/_webhook_header.html.slim | 2 +- app/views/webhook_headers/create.js.erb | 8 +- app/views/webhook_headers/destroy.js.erb | 8 +- app/views/webhooks/show.html.slim | 93 ++++++++++--------- spec/features/webhooks_spec.rb | 86 +++++++++++++++++ 9 files changed, 207 insertions(+), 73 deletions(-) create mode 100644 app/assets/javascripts/modules/webhooks/components/new-webhook-header-form.js create mode 100644 app/assets/javascripts/modules/webhooks/components/webhook-headers-panel.js diff --git a/app/assets/javascripts/modules/webhooks/components/new-webhook-header-form.js b/app/assets/javascripts/modules/webhooks/components/new-webhook-header-form.js new file mode 100644 index 000000000..d53c18170 --- /dev/null +++ b/app/assets/javascripts/modules/webhooks/components/new-webhook-header-form.js @@ -0,0 +1,25 @@ +import BaseComponent from '~/base/component'; + +const WEBHOOK_FORM_FIELDS = '.form-control'; + +// NewWebhookHeaderForm component refers to the new webhook header form +class NewWebhookHeaderForm extends BaseComponent { + elements() { + this.$fields = this.$el.find(WEBHOOK_FORM_FIELDS); + } + + toggle() { + this.$el.toggle(400, 'swing', () => { + const visible = this.$el.is(':visible'); + + if (visible) { + this.$fields.first().focus(); + } + + this.$fields.val(''); + layout_resizer(); + }); + } +} + +export default NewWebhookHeaderForm; diff --git a/app/assets/javascripts/modules/webhooks/components/webhook-headers-panel.js b/app/assets/javascripts/modules/webhooks/components/webhook-headers-panel.js new file mode 100644 index 000000000..84a10cdfb --- /dev/null +++ b/app/assets/javascripts/modules/webhooks/components/webhook-headers-panel.js @@ -0,0 +1,35 @@ +import BaseComponent from '~/base/component'; + +import NewWebhookHeaderForm from './new-webhook-header-form'; + +const TOGGLE_LINK = '#add_webhook_header_btn'; +const TOGGLE_LINK_ICON = `${TOGGLE_LINK} i`; +const NEW_WEBHOOK_HEADER_FORM = '#add_webhook_header_form'; + +// WebhookHeadersPanel component that lists webhook headers +// and contains new webhook header form. +class WebhookHeadersPanel extends BaseComponent { + elements() { + this.$toggle = this.$el.find(TOGGLE_LINK); + this.$toggleIcon = this.$el.find(TOGGLE_LINK_ICON); + this.$form = this.$el.find(NEW_WEBHOOK_HEADER_FORM); + } + + events() { + this.$el.on('click', TOGGLE_LINK, e => this.onToggleLinkClick(e)); + } + + mount() { + this.newForm = new NewWebhookHeaderForm(this.$form); + } + + onToggleLinkClick() { + const wasVisible = this.$form.is(':visible'); + + this.newForm.toggle(); + this.$toggleIcon.toggleClass('fa-minus-circle', !wasVisible); + this.$toggleIcon.toggleClass('fa-plus-circle', wasVisible); + } +} + +export default WebhookHeadersPanel; diff --git a/app/assets/javascripts/modules/webhooks/pages/show.js b/app/assets/javascripts/modules/webhooks/pages/show.js index aa5fca3b5..6f0c7f412 100644 --- a/app/assets/javascripts/modules/webhooks/pages/show.js +++ b/app/assets/javascripts/modules/webhooks/pages/show.js @@ -1,18 +1,22 @@ import BaseComponent from '~/base/component'; import WebhookDetails from '../components/webhook-details'; +import WebhookHeadersPanel from '../components/webhook-headers-panel'; const WEBHOOK_DETAILS = '.webhook-details'; +const WEBHOOK_HEADERS_PANEL = '.webhook-headers-panel'; // WebhookShowPage component responsible to instantiate -// the user's edit page components and handle interactions. +// the webhooks's show page components and handle interactions. class WebhookShowPage extends BaseComponent { elements() { this.$details = this.$el.find(WEBHOOK_DETAILS); + this.$headers = this.$el.find(WEBHOOK_HEADERS_PANEL); } mount() { this.details = new WebhookDetails(this.$details); + this.headers = new WebhookHeadersPanel(this.$headers); } } diff --git a/app/assets/javascripts/teams.js b/app/assets/javascripts/teams.js index 8d46801c0..5f51e4173 100644 --- a/app/assets/javascripts/teams.js +++ b/app/assets/javascripts/teams.js @@ -37,23 +37,6 @@ jQuery(function ($) { } }); - $('#add_webhook_header_btn').unbind('click').on('click', function () { - $('#webhook_header_name').val(''); - $('#webhook_header_value').val(''); - - $('#add_webhook_header_form').toggle(400, 'swing', function () { - if ($('#add_webhook_header_form').is(':visible')) { - $('#add_webhook_header_btn i').addClass('fa-minus-circle'); - $('#add_webhook_header_btn i').removeClass('fa-plus-circle'); - $('#webhook_header_name').focus(); - } else { - $('#add_webhook_header_btn i').removeClass('fa-minus-circle'); - $('#add_webhook_header_btn i').addClass('fa-plus-circle'); - } - layout_resizer(); - }); - }); - $('#add_team_btn').on('click', function () { $('#team_name').val(''); $('#team_description').val(''); diff --git a/app/views/webhook_headers/_webhook_header.html.slim b/app/views/webhook_headers/_webhook_header.html.slim index dc5a9e942..27534c827 100644 --- a/app/views/webhook_headers/_webhook_header.html.slim +++ b/app/views/webhook_headers/_webhook_header.html.slim @@ -6,7 +6,7 @@ tr id="webhook_header_#{webhook_header.id}" td= '*' * webhook_header.value.length - if can_manage_namespace?(@namespace) td - a[class="btn btn-default" + a[class="btn btn-default delete-webhook-header-btn" data-placement="left" data-toggle="popover" data-title="Please confirm" diff --git a/app/views/webhook_headers/create.js.erb b/app/views/webhook_headers/create.js.erb index 58d89d59a..eb5ad63f7 100644 --- a/app/views/webhook_headers/create.js.erb +++ b/app/views/webhook_headers/create.js.erb @@ -1,9 +1,9 @@ <% if @webhook_header.errors.any? %> - $('#alert p').html("<%= escape_javascript(@webhook_header.errors.full_messages.join('
')) %>"); - $('#alert').fadeIn(); + $('#float-alert p').html("<%= escape_javascript(@webhook_header.errors.full_messages.join('
')) %>"); + $('#float-alert').fadeIn(setTimeOutAlertDelay()); <% else %> - $('#alert p').html("Header '<%= @webhook_header.name %>' was created successfully"); - $('#alert').fadeIn(); + $('#float-alert p').html("Header '<%= @webhook_header.name %>' was created successfully"); + $('#float-alert').fadeIn(setTimeOutAlertDelay()); $('#add_webhook_header_form').fadeOut(); $('#add_webhook_header_btn i').addClass("fa-plus-circle") diff --git a/app/views/webhook_headers/destroy.js.erb b/app/views/webhook_headers/destroy.js.erb index 05b05f91b..9a9902f9b 100644 --- a/app/views/webhook_headers/destroy.js.erb +++ b/app/views/webhook_headers/destroy.js.erb @@ -1,8 +1,8 @@ <% if @webhook_header.errors.any? %> - $('#alert p').html("<%= escape_javascript(@webhook_header.errors.full_messages.join('
')) %>"); - $('#alert').fadeIn(); + $('#float-alert p').html("<%= escape_javascript(@webhook_header.errors.full_messages.join('
')) %>"); + $('#float-alert').fadeIn(setTimeOutAlertDelay()); <% else %> $("#webhook_header_<%= @webhook_header.id %>").remove(); - $('#alert p').html("Header '<%= @webhook_header.name %>' was removed successfully"); - $('#alert').fadeIn(); + $('#float-alert p').html("Header '<%= @webhook_header.name %>' was removed successfully"); + $('#float-alert').fadeIn(setTimeOutAlertDelay()); <% end %> diff --git a/app/views/webhooks/show.html.slim b/app/views/webhooks/show.html.slim index 18c27a1aa..6e02ead4a 100644 --- a/app/views/webhooks/show.html.slim +++ b/app/views/webhooks/show.html.slim @@ -49,53 +49,54 @@ .panel-body = render partial: "detail", locals: {webhook: @webhook} -- if can_manage_namespace?(@namespace) - #add_webhook_header_form.collapse - = form_for :webhook_header, url: namespace_webhook_headers_path(@namespace, @webhook), remote: true, html: {id: 'new-header-form', class: 'form-horizontal', role: 'form'} do |f| - .form-group - = f.label :name, {class: 'control-label col-md-2'} - .col-md-7 - = f.text_field(:name, class: 'form-control', required: true, placeholder: "Name") - .form-group - = f.label :value, {class: 'control-label col-md-2'} - .col-md-7 - = f.text_field(:value, class: 'form-control', required: true, placeholder: "Value") - .form-group - .col-md-offset-2.col-md-7 - = f.submit('Create', class: 'btn btn-primary') - .errors +.webhook-headers-panel + - if can_manage_namespace?(@namespace) + #add_webhook_header_form.collapse + = form_for :webhook_header, url: namespace_webhook_headers_path(@namespace, @webhook), remote: true, html: {id: 'new-header-form', class: 'form-horizontal', role: 'form'} do |f| + .form-group + = f.label :name, {class: 'control-label col-md-2'} + .col-md-7 + = f.text_field(:name, class: 'form-control', required: true, placeholder: "Name") + .form-group + = f.label :value, {class: 'control-label col-md-2'} + .col-md-7 + = f.text_field(:value, class: 'form-control', required: true, placeholder: "Value") + .form-group + .col-md-offset-2.col-md-7 + = f.submit('Create', class: 'btn btn-primary') + .errors -.panel.panel-default - .panel-heading - h5 - ' Headers - small - a[data-placement="right" data-toggle="popover" data-container=".panel-heading" data-content="A header is a HTTP header, i.e. is a key-value pair which is included in the HTTP request." data-original-title="What's this?" tabindex="0"] - i.fa.fa-info-circle - - if can_manage_namespace?(@namespace) - .pull-right - a#add_webhook_header_btn.btn.btn-xs.btn-link.js-toggle-button[role="button"] - i.fa.fa-plus-circle - | Create new header - .panel-body - .table-responsive - table.table.table-stripped.table-hover - colgroup - col.col-20 - - if can_manage_namespace?(@namespace) - col.col-70 - col.col-10 - - else - col.col-80 - thead - tr - th Name - th Value - - if can_manage_namespace?(@namespace) - th Remove - tbody#webhook_headers - - @webhook.headers.each do |header| - = render partial: 'webhook_headers/webhook_header', locals: {namespace: @namespace, webhook: @webhook, webhook_header: header} + .panel.panel-default + .panel-heading + h5 + ' Headers + small + a[data-placement="right" data-toggle="popover" data-container=".panel-heading" data-content="A header is a HTTP header, i.e. is a key-value pair which is included in the HTTP request." data-original-title="What's this?" tabindex="0"] + i.fa.fa-info-circle + - if can_manage_namespace?(@namespace) + .pull-right + a#add_webhook_header_btn.btn.btn-xs.btn-link.js-toggle-button[role="button"] + i.fa.fa-plus-circle + | Create new header + .panel-body + .table-responsive + table.table.table-stripped.table-hover + colgroup + col.col-20 + - if can_manage_namespace?(@namespace) + col.col-70 + col.col-10 + - else + col.col-80 + thead + tr + th Name + th Value + - if can_manage_namespace?(@namespace) + th Remove + tbody#webhook_headers + - @webhook.headers.each do |header| + = render partial: 'webhook_headers/webhook_header', locals: {namespace: @namespace, webhook: @webhook, webhook_header: header} .panel.panel-default .panel-heading diff --git a/spec/features/webhooks_spec.rb b/spec/features/webhooks_spec.rb index f121c3aff..a3e444722 100644 --- a/spec/features/webhooks_spec.rb +++ b/spec/features/webhooks_spec.rb @@ -8,6 +8,12 @@ let!(:team) { create(:team, owners: [user], contributors: [user2], viewers: [user3]) } let!(:namespace) { create(:namespace, team: team, registry: registry) } let!(:webhook) { create(:webhook, namespace: namespace) } + let!(:webhook_header) do + create(:webhook_header, + name: "h-name", + value: "h-value", + webhook: webhook) + end before do login_as user, scope: :user @@ -135,5 +141,85 @@ expect(page).to have_css(".edit-webhook-link .fa-pencil") expect(page).to_not have_css(".edit-webhook-link .fa-close") end + + describe "webhook_header" do + scenario "A user can create a header from webhook's page", js: true do + webhook_headers_count = webhook.headers.count + + visit namespace_webhook_path(namespace, webhook) + + find("#add_webhook_header_btn").click + wait_for_effect_on("#add_webhook_header_form") + + fill_in "Name", with: "cool-header" + fill_in "Value", with: "cool-value" + click_button "Create" + + wait_for_ajax + wait_for_effect_on("#float-alert") + + expect(page).to have_css("#float-alert") + expect(page).to have_content("cool-header") + expect(page).to have_content("cool-value") + expect(page).to have_content("Header 'cool-header' was created successfully") + expect(webhook.headers.count).to eql webhook_headers_count + 1 + end + + scenario "A user cannot create a header that already exists", js: true do + webhook_headers_count = webhook.headers.count + + visit namespace_webhook_path(namespace, webhook) + find("#add_webhook_header_btn").click + wait_for_effect_on("#add_webhook_header_form") + + expect(focused_element_id).to eq "webhook_header_name" + fill_in "Name", with: webhook_header.name + fill_in "Value", with: "something" + click_button "Create" + + wait_for_ajax + wait_for_effect_on("#float-alert") + + expect(page).to have_css("#float-alert") + expect(page).to have_content("Name has already been taken") + expect(webhook.headers.count).to eql webhook_headers_count + end + + scenario "A user deletes a webhook header", js: true do + visit namespace_webhook_path(namespace, webhook) + id = webhook_header.id + + expect(page).to have_content(webhook_header.name) + expect(page).to have_content(webhook_header.value) + + find("#webhook_header_#{id} .delete-webhook-header-btn").click + find(".popover-content .btn-primary").click + + wait_for_ajax + wait_for_effect_on("#float-alert") + + expect(page).to have_css("#float-alert") + expect(page).to have_content("Header '#{webhook_header.name}' was removed successfully") + expect(page).not_to have_content(webhook_header.name) + expect(page).not_to have_content(webhook_header.value) + end + + scenario 'The "Create new header" link has a toggle effect', js: true do + visit namespace_webhook_path(namespace, webhook) + + expect(page).to have_css("#add_webhook_header_btn i.fa-plus-circle") + expect(page).to_not have_css("#add_webhook_header_btn i.fa-minus-circle") + + find("#add_webhook_header_btn").click + + expect(page).to_not have_css("#add_webhook_header_btn i.fa-plus-circle") + expect(page).to have_css("#add_webhook_header_btn i.fa-minus-circle") + + find("#add_webhook_header_btn").click + + expect(page).to have_css("#add_webhook_header_btn i.fa-plus-circle") + expect(page).to_not have_css("#add_webhook_header_btn i.fa-minus-circle") + end + end end end