From 07bdc70aba01fefc2bdb62fdbb9862409c4d1811 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saray=20Cabrera=20Padr=C3=B3n?= Date: Thu, 13 Jun 2019 19:48:50 +0200 Subject: [PATCH] Fix bug on Request bugowner change link In the Search Owner page there is a link to request a new bugowner for the package. The new bugowner can be a user or a group, not both. However, the action was always requiring both fields by mistake. Now only one is required and sent. Fixed in both old and new UI. Reuses the same JavaScript function used to add a reviewer to a requet, which has been slightly improved. Co-authored-by: David Kang --- .../app/assets/javascripts/webui2/request.js | 6 +- .../controllers/webui/request_controller.rb | 2 +- .../_set_bugowner_request_dialog.html.erb | 6 +- .../request/_add_reviewer_dialog.html.haml | 8 +-- .../_request_bugowner_change_dialog.html.haml | 15 +++-- .../features/webui/change_bugowner_spec.rb | 60 +++++++++++++++++++ .../owner_search_bugownership_collection.xml | 5 ++ 7 files changed, 85 insertions(+), 17 deletions(-) create mode 100644 src/api/spec/bootstrap/features/webui/change_bugowner_spec.rb create mode 100644 src/api/spec/fixtures/files/owner_search_bugownership_collection.xml diff --git a/src/api/app/assets/javascripts/webui2/request.js b/src/api/app/assets/javascripts/webui2/request.js index f52c6ded937..23662843eed 100644 --- a/src/api/app/assets/javascripts/webui2/request.js +++ b/src/api/app/assets/javascripts/webui2/request.js @@ -58,15 +58,15 @@ function setupRequestDialog() { // jshint ignore:line function requestAddReviewAutocomplete() { // jshint ignore:line $('.modal').on('shown.bs.modal', function() { - $('.hideable input:not(:visible)').removeAttr('required'); + $('.hideable input:not(:visible)').attr('disabled', true); }); $('#review_type').change(function () { $('.hideable').addClass('d-none'); - $('.hideable input:not(:visible)').removeAttr('required'); + $('.hideable input:not(:visible)').attr('disabled', true); var selected = $('#review_type option:selected').attr('value'); $('.' + selected).removeClass('d-none'); - $('.hideable input:visible').attr('required', true); + $('.hideable input:visible').removeAttr('disabled'); }); } diff --git a/src/api/app/controllers/webui/request_controller.rb b/src/api/app/controllers/webui/request_controller.rb index 16c0ff37cc2..74d677b84de 100644 --- a/src/api/app/controllers/webui/request_controller.rb +++ b/src/api/app/controllers/webui/request_controller.rb @@ -217,7 +217,7 @@ def set_bugowner_request_dialog end def set_bugowner_request - required_parameters :project, :user, :group + required_parameters :project request = nil begin request = BsRequest.create!( diff --git a/src/api/app/views/webui/request/_set_bugowner_request_dialog.html.erb b/src/api/app/views/webui/request/_set_bugowner_request_dialog.html.erb index b97c91c1e3a..33b27082af9 100644 --- a/src/api/app/views/webui/request/_set_bugowner_request_dialog.html.erb +++ b/src/api/app/views/webui/request/_set_bugowner_request_dialog.html.erb @@ -5,8 +5,7 @@ <%= form_tag({:action => "set_bugowner_request"}, :name => 'bugowner') do %>

<%= label_tag(:user, "User:") %>
- - <%= text_field_tag(:user, nil, :onclick => "javascript:document.getElementById('group').disabled=true", :name => 'user' ) %>
+ <%= text_field_tag(:user, nil, :onclick => "javascript:document.getElementById('group').disabled=true", :name => 'user', required: true) %>
<%= javascript_tag do %> $("#user").autocomplete({source: '<%= url_for :controller => 'user', :action => 'autocomplete' %>', search: function(event, ui) { $(this).addClass('loading-spinner'); @@ -16,8 +15,7 @@ }, minLength: 2}); <% end %> <%= label_tag(:user, "Group:") %>
- - <%= text_field_tag(:group, nil, :onclick => "javascript:document.getElementById('user').disabled=true", :name => 'group' ) %>
+ <%= text_field_tag(:group, nil, :onclick => "javascript:document.getElementById('user').disabled=true", :name => 'group', required: true) %>
<%= javascript_tag do %> $("#group").autocomplete({source: '<%= url_for :controller => 'groups', :action => 'autocomplete' %>', search: function(event, ui) { $(this).addClass('loading-spinner'); diff --git a/src/api/app/views/webui2/webui/request/_add_reviewer_dialog.html.haml b/src/api/app/views/webui2/webui/request/_add_reviewer_dialog.html.haml index b3e016cbbd0..58a50ab1385 100644 --- a/src/api/app/views/webui2/webui/request/_add_reviewer_dialog.html.haml +++ b/src/api/app/views/webui2/webui/request/_add_reviewer_dialog.html.haml @@ -11,18 +11,18 @@ = select_tag(:review_type, options_for_select([%w[User review-user], %w[Group review-group], %w[Project review-project], %w[Package review-package]]), class: 'custom-select') .hideable.review-user - = render partial: 'webui/autocomplete', locals: { html_id: 'review_user', label: 'User:', + = render partial: 'webui/autocomplete', locals: { html_id: 'review_user', label: 'User:', required: true, data: { source: autocomplete_users_path } } .hideable.review-group.d-none - = render partial: 'webui/autocomplete', locals: { html_id: 'review_group', label: 'Group:', + = render partial: 'webui/autocomplete', locals: { html_id: 'review_group', label: 'Group:', required: true, data: { source: autocomplete_groups_path } } .hideable.review-project.review-package.d-none - = render partial: 'webui/autocomplete', locals: { html_id: 'review_project', label: 'Project:', + = render partial: 'webui/autocomplete', locals: { html_id: 'review_project', label: 'Project:', required: true, data: { source: autocomplete_projects_path } } .hideable.review-package.d-none - = render partial: 'webui/autocomplete', locals: { html_id: 'review_package', label: 'Package:', + = render partial: 'webui/autocomplete', locals: { html_id: 'review_package', label: 'Package:', required: true, data: { source: autocomplete_packages_path } } .form-group diff --git a/src/api/app/views/webui2/webui/search/_request_bugowner_change_dialog.html.haml b/src/api/app/views/webui2/webui/search/_request_bugowner_change_dialog.html.haml index 240c3c5d165..7972287dbc3 100644 --- a/src/api/app/views/webui2/webui/search/_request_bugowner_change_dialog.html.haml +++ b/src/api/app/views/webui2/webui/search/_request_bugowner_change_dialog.html.haml @@ -5,13 +5,15 @@ .modal-header %h5.modal-title#request-bugowner-change-modal-label Submit Request to Change Bugowner .modal-body - %p.font-italic - A user or a group is required. .form-group - = render partial: 'webui/autocomplete', locals: { html_id: 'user', label: 'User:', required: false, + = label_tag(:review_type, 'Bugowner can be a user or a group:') + = select_tag(:review_type, options_for_select([%w[User review-user], %w[Group review-group]], 'review-user'), class: 'custom-select') + + .hideable.review-user + = render partial: 'webui/autocomplete', locals: { html_id: 'user', label: 'User:', required: true, data: { source: autocomplete_users_path } } - .form-group - = render partial: 'webui/autocomplete', locals: { html_id: 'group', label: 'Group:', required: false, + .hideable.review-group.d-none + = render partial: 'webui/autocomplete', locals: { html_id: 'group', label: 'Group:', required: true, data: { source: autocomplete_groups_path } } .form-group For: @@ -37,3 +39,6 @@ .modal-footer = render partial: 'webui2/shared/dialog_action_buttons', locals: { submit_tag_text: 'Submit' } + +- content_for :ready_function do + requestAddReviewAutocomplete(); diff --git a/src/api/spec/bootstrap/features/webui/change_bugowner_spec.rb b/src/api/spec/bootstrap/features/webui/change_bugowner_spec.rb new file mode 100644 index 00000000000..06587112453 --- /dev/null +++ b/src/api/spec/bootstrap/features/webui/change_bugowner_spec.rb @@ -0,0 +1,60 @@ +require 'browser_helper' + +RSpec.feature 'Bootstrap_ChangeBugowner', type: :feature, js: true do + let!(:bugowner) { create(:confirmed_user, :with_home, login: 'Iggy') } + let!(:package) { create(:package, name: 'TestPack', project: project) } + let(:project) { Project.find_by(name: 'home:Iggy') } + let!(:new_bugowner) { create(:confirmed_user, :with_home, login: 'Milo') } + let!(:group) { create(:group, title: 'Heroes') } + + let!(:collection) do + file_fixture('owner_search_collection.xml').read + end + let!(:bug_collection) do + file_fixture('owner_search_bugownership_collection.xml').read + end + + before do + login bugowner + create(:attrib, attrib_type: AttribType.where(name: 'OwnerRootProject').first, project: Project.find_by(name: 'home:Iggy')) + create(:relationship_package_user, package: package, user: bugowner, role: Role.find_by_title('bugowner')) + allow(Backend::Api::Search).to receive(:binary).and_return(collection) + + visit search_owner_path + fill_in :search_input, with: package.name + click_button 'Search' + click_link 'Request bugowner change' + end + + context 'with a user as new bugowner' do + scenario 'the bugowner is changed' do + fill_in :user, with: 'Milo' + fill_in :description, with: 'Replace current bugowner by Milo' + click_button 'Submit' + expect(page).to have_text("#{bugowner.name} (#{bugowner.login}) wants the user #{new_bugowner.name} (#{new_bugowner.login}) to become bugowner (previous bugowners will be deleted)") + end + end + + context 'with a group as new bugowner' do + scenario 'the bugowner is changed by a group' do + find(:id, 'review_type').select('Group') + fill_in :group, with: 'Heroes' + fill_in :description, with: 'Replace current bugowner by group Heroes' + click_button 'Submit' + expect(page).to have_text("#{bugowner.name} (#{bugowner.login}) wants the group #{group.title} to become bugowner (previous bugowners will be deleted)") + end + end + + context 'forcing to add both user and group as bugowner' do + scenario 'only the visible one before submitting is added' do + find(:id, 'review_type').select('Group') + fill_in :group, with: 'Heroes' + find(:id, 'review_type').select('User') + fill_in :user, with: 'Milo' + fill_in :description, with: 'Replace current bugowner by something else' + click_button 'Submit' + expect(page).to have_text("#{bugowner.name} (#{bugowner.login}) wants the user #{new_bugowner.name} (#{new_bugowner.login}) to become bugowner (previous bugowners will be deleted)") + expect(page).not_to have_text('Heroes') + end + end +end diff --git a/src/api/spec/fixtures/files/owner_search_bugownership_collection.xml b/src/api/spec/fixtures/files/owner_search_bugownership_collection.xml new file mode 100644 index 00000000000..92e502f6b43 --- /dev/null +++ b/src/api/spec/fixtures/files/owner_search_bugownership_collection.xml @@ -0,0 +1,5 @@ + + + + +