From 60baa1e866013d66d25531e5f8a8f62ec49b228b Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 15:10:21 +0200 Subject: [PATCH 01/13] Move issuable partials in common directory Signed-off-by: Dmitriy Zaporozhets --- app/views/dashboard/issues.html.haml | 2 +- app/views/dashboard/merge_requests.html.haml | 2 +- app/views/groups/issues.html.haml | 2 +- app/views/groups/merge_requests.html.haml | 2 +- app/views/projects/issues/_form.html.haml | 2 +- app/views/projects/issues/index.html.haml | 4 ++-- app/views/projects/merge_requests/_form.html.haml | 2 +- app/views/projects/merge_requests/_new_submit.html.haml | 2 +- app/views/projects/merge_requests/index.html.haml | 4 ++-- .../_filter.html.haml} | 0 .../issuable/_form.html.haml} | 0 .../_search_form.html.haml} | 0 12 files changed, 11 insertions(+), 11 deletions(-) rename app/views/shared/{_issuable_filter.html.haml => issuable/_filter.html.haml} (100%) rename app/views/{projects/_issuable_form.html.haml => shared/issuable/_form.html.haml} (100%) rename app/views/shared/{_issuable_search_form.html.haml => issuable/_search_form.html.haml} (100%) diff --git a/app/views/dashboard/issues.html.haml b/app/views/dashboard/issues.html.haml index 0dd2edbb1bc..94318d1bcf5 100644 --- a/app/views/dashboard/issues.html.haml +++ b/app/views/dashboard/issues.html.haml @@ -17,5 +17,5 @@ = link_to issues_dashboard_url(format: :atom, private_token: current_user.private_token), class: 'btn' do %i.fa.fa-rss - = render 'shared/issuable_filter', type: :issues + = render 'shared/issuable/filter', type: :issues = render 'shared/issues' diff --git a/app/views/dashboard/merge_requests.html.haml b/app/views/dashboard/merge_requests.html.haml index 61d2fbe538c..90611d562b0 100644 --- a/app/views/dashboard/merge_requests.html.haml +++ b/app/views/dashboard/merge_requests.html.haml @@ -7,5 +7,5 @@ List all merge requests from all projects you have access to. %hr .append-bottom-20 - = render 'shared/issuable_filter', type: :merge_requests + = render 'shared/issuable/filter', type: :merge_requests = render 'shared/merge_requests' diff --git a/app/views/groups/issues.html.haml b/app/views/groups/issues.html.haml index e0756e909be..f0d90782556 100644 --- a/app/views/groups/issues.html.haml +++ b/app/views/groups/issues.html.haml @@ -21,5 +21,5 @@ = link_to issues_group_url(@group, format: :atom, private_token: current_user.private_token), class: 'btn' do %i.fa.fa-rss - = render 'shared/issuable_filter', type: :issues + = render 'shared/issuable/filter', type: :issues = render 'shared/issues' diff --git a/app/views/groups/merge_requests.html.haml b/app/views/groups/merge_requests.html.haml index 3d9e857cc52..ca85a158707 100644 --- a/app/views/groups/merge_requests.html.haml +++ b/app/views/groups/merge_requests.html.haml @@ -10,5 +10,5 @@ To see all merge requests you should visit #{link_to 'dashboard', merge_requests_dashboard_path} page. %hr .append-bottom-20 - = render 'shared/issuable_filter', type: :merge_requests + = render 'shared/issuable/filter', type: :merge_requests = render 'shared/merge_requests' diff --git a/app/views/projects/issues/_form.html.haml b/app/views/projects/issues/_form.html.haml index 8d2564be55e..f39bb7d2574 100644 --- a/app/views/projects/issues/_form.html.haml +++ b/app/views/projects/issues/_form.html.haml @@ -3,7 +3,7 @@ %hr = form_for [@project.namespace.becomes(Namespace), @project, @issue], html: { class: 'form-horizontal issue-form gfm-form' } do |f| - = render 'projects/issuable_form', f: f, issuable: @issue + = render 'shared/issuable/form', f: f, issuable: @issue :javascript $('.assign-to-me-link').on('click', function(e){ diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml index 1d5597602d1..2785ff25e69 100644 --- a/app/views/projects/issues/index.html.haml +++ b/app/views/projects/issues/index.html.haml @@ -11,14 +11,14 @@ = link_to namespace_project_issues_path(@project.namespace, @project, :atom, { private_token: current_user.private_token }), class: 'btn append-right-10' do %i.fa.fa-rss - = render 'shared/issuable_search_form', path: namespace_project_issues_path(@project.namespace, @project) + = render 'shared/issuable/search_form', path: namespace_project_issues_path(@project.namespace, @project) - if can? current_user, :write_issue, @project = link_to new_namespace_project_issue_path(@project.namespace, @project, issue: { assignee_id: @issuable_finder.assignee.try(:id), milestone_id: @issuable_finder.milestones.try(:first).try(:id) }), class: "btn btn-new pull-left", title: "New Issue", id: "new_issue_link" do %i.fa.fa-plus New Issue - = render 'shared/issuable_filter', type: :issues + = render 'shared/issuable/filter', type: :issues .issues-holder = render "issues" diff --git a/app/views/projects/merge_requests/_form.html.haml b/app/views/projects/merge_requests/_form.html.haml index be73f087449..8f225a432e4 100644 --- a/app/views/projects/merge_requests/_form.html.haml +++ b/app/views/projects/merge_requests/_form.html.haml @@ -1,6 +1,6 @@ = form_for [@project.namespace.becomes(Namespace), @project, @merge_request], html: { class: 'merge-request-form form-horizontal gfm-form' } do |f| .merge-request-form-info - = render 'projects/issuable_form', f: f, issuable: @merge_request + = render 'shared/issuable/form', f: f, issuable: @merge_request :javascript disableButtonIfEmptyField("#merge_request_title", ".btn-save"); diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index 6792104569b..2f147f9095d 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -11,7 +11,7 @@ %hr = form_for [@project.namespace.becomes(Namespace), @project, @merge_request], html: { class: 'merge-request-form form-horizontal gfm-form' } do |f| .merge-request-form-info - = render 'projects/issuable_form', f: f, issuable: @merge_request + = render 'shared/issuable/form', f: f, issuable: @merge_request = f.hidden_field :source_project_id = f.hidden_field :source_branch = f.hidden_field :target_project_id diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml index fa591b0537e..750cc3e6eea 100644 --- a/app/views/projects/merge_requests/index.html.haml +++ b/app/views/projects/merge_requests/index.html.haml @@ -1,13 +1,13 @@ - page_title "Merge Requests" .append-bottom-10 .pull-right - = render 'shared/issuable_search_form', path: namespace_project_merge_requests_path(@project.namespace, @project) + = render 'shared/issuable/search_form', path: namespace_project_merge_requests_path(@project.namespace, @project) - if can? current_user, :write_merge_request, @project .pull-left.hidden-xs = link_to new_namespace_project_merge_request_path(@project.namespace, @project), class: "btn btn-new", title: "New Merge Request" do %i.fa.fa-plus New Merge Request - = render 'shared/issuable_filter', type: :merge_requests + = render 'shared/issuable/filter', type: :merge_requests .merge-requests-holder = render 'merge_requests' diff --git a/app/views/shared/_issuable_filter.html.haml b/app/views/shared/issuable/_filter.html.haml similarity index 100% rename from app/views/shared/_issuable_filter.html.haml rename to app/views/shared/issuable/_filter.html.haml diff --git a/app/views/projects/_issuable_form.html.haml b/app/views/shared/issuable/_form.html.haml similarity index 100% rename from app/views/projects/_issuable_form.html.haml rename to app/views/shared/issuable/_form.html.haml diff --git a/app/views/shared/_issuable_search_form.html.haml b/app/views/shared/issuable/_search_form.html.haml similarity index 100% rename from app/views/shared/_issuable_search_form.html.haml rename to app/views/shared/issuable/_search_form.html.haml From d315d1eba4f45c4c4f0417fbdcd3eaf69e102282 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 15:38:04 +0200 Subject: [PATCH 02/13] Dont allow guest to set assigne, milestone and label when create new issue or merge request Signed-off-by: Dmitriy Zaporozhets --- app/models/ability.rb | 3 + app/views/shared/issuable/_form.html.haml | 77 ++++++++++++----------- 2 files changed, 43 insertions(+), 37 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index a5db22040e0..b611276cf9c 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -154,6 +154,9 @@ class Ability project_guest_rules + [ :download_code, :fork_project, + :set_assignee, + :set_milestone, + :set_label, :write_project_snippet ] end diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 496fad34dc2..7095402bf05 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -37,47 +37,50 @@ .clearfix .error-alert -%hr -.form-group - .issue-assignee - = f.label :assignee_id, class: 'control-label' do - %i.fa.fa-user - Assign to + %hr +- if can?(current_user, :set_assignee, @project) + .form-group + .issue-assignee + = f.label :assignee_id, class: 'control-label' do + %i.fa.fa-user + Assign to + .col-sm-10 + = users_select_tag("#{issuable.class.model_name.param_key}[assignee_id]", + placeholder: 'Select a user', class: 'custom-form-control', null_user: true, + selected: issuable.assignee_id, project: @target_project || @project) +   + = link_to 'Assign to me', '#', class: 'btn assign-to-me-link' +- if can?(current_user, :set_milestone, @project) + .form-group + .issue-milestone + = f.label :milestone_id, class: 'control-label' do + %i.fa.fa-clock-o + Milestone + .col-sm-10 + - if milestone_options(issuable).present? + = f.select(:milestone_id, milestone_options(issuable), + { include_blank: 'Select milestone' }, { class: 'select2' }) + - else + .prepend-top-10 + %span.light No open milestones available. +   + - if can? current_user, :admin_milestone, issuable.project + = link_to 'Create new milestone', new_namespace_project_milestone_path(issuable.project.namespace, issuable.project), target: :blank +- if can?(current_user, :set_label, @project) + .form-group + = f.label :label_ids, class: 'control-label' do + %i.fa.fa-tag + Labels .col-sm-10 - = users_select_tag("#{issuable.class.model_name.param_key}[assignee_id]", - placeholder: 'Select a user', class: 'custom-form-control', null_user: true, - selected: issuable.assignee_id, project: @target_project || @project) -   - = link_to 'Assign to me', '#', class: 'btn assign-to-me-link' -.form-group - .issue-milestone - = f.label :milestone_id, class: 'control-label' do - %i.fa.fa-clock-o - Milestone - .col-sm-10 - - if milestone_options(issuable).present? - = f.select(:milestone_id, milestone_options(issuable), - { include_blank: 'Select milestone' }, { class: 'select2' }) + - if issuable.project.labels.any? + = f.collection_select :label_ids, issuable.project.labels.all, :id, :name, + { selected: issuable.label_ids }, multiple: true, class: 'select2' - else .prepend-top-10 - %span.light No open milestones available. + %span.light No labels yet.   - - if can? current_user, :admin_milestone, issuable.project - = link_to 'Create new milestone', new_namespace_project_milestone_path(issuable.project.namespace, issuable.project), target: :blank -.form-group - = f.label :label_ids, class: 'control-label' do - %i.fa.fa-tag - Labels - .col-sm-10 - - if issuable.project.labels.any? - = f.collection_select :label_ids, issuable.project.labels.all, :id, :name, - { selected: issuable.label_ids }, multiple: true, class: 'select2' - - else - .prepend-top-10 - %span.light No labels yet. -   - - if can? current_user, :admin_label, issuable.project - = link_to 'Create new label', new_namespace_project_label_path(issuable.project.namespace, issuable.project), target: :blank + - if can? current_user, :admin_label, issuable.project + = link_to 'Create new label', new_namespace_project_label_path(issuable.project.namespace, issuable.project), target: :blank - if issuable.is_a?(MergeRequest) %hr From 2d31568b27d60b2868d60da905018bdcba862c59 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 16:04:50 +0200 Subject: [PATCH 03/13] Refactor issue and merge request sidebar Signed-off-by: Dmitriy Zaporozhets --- app/assets/javascripts/dispatcher.js.coffee | 2 - .../javascripts/issuable_context.js.coffee | 22 ++++++++ app/assets/javascripts/issue.js.coffee | 17 ------- .../javascripts/merge_request.js.coffee | 18 ------- app/helpers/gitlab_routing_helper.rb | 8 +++ .../projects/issues/_discussion.html.haml | 2 +- .../projects/issues/_issue_context.html.haml | 47 ----------------- app/views/projects/issues/show.html.haml | 2 +- app/views/projects/issues/update.js.haml | 18 +------ .../merge_requests/_discussion.html.haml | 2 +- .../projects/merge_requests/_show.html.haml | 2 +- .../merge_requests/show/_context.html.haml | 49 ------------------ .../projects/merge_requests/update.js.haml | 11 ++-- app/views/shared/issuable/_context.html.haml | 50 +++++++++++++++++++ 14 files changed, 89 insertions(+), 161 deletions(-) create mode 100644 app/assets/javascripts/issuable_context.js.coffee delete mode 100644 app/views/projects/issues/_issue_context.html.haml delete mode 100644 app/views/projects/merge_requests/show/_context.html.haml create mode 100644 app/views/shared/issuable/_context.html.haml diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 84873e389ea..45383fc96aa 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -135,8 +135,6 @@ class Dispatcher new ProjectNew() when 'show' new ProjectShow() - when 'issues', 'merge_requests' - new UsersSelect() when 'wikis' new Wikis() shortcut_handler = new ShortcutsNavigation() diff --git a/app/assets/javascripts/issuable_context.js.coffee b/app/assets/javascripts/issuable_context.js.coffee new file mode 100644 index 00000000000..176d9cabefa --- /dev/null +++ b/app/assets/javascripts/issuable_context.js.coffee @@ -0,0 +1,22 @@ +#= require jquery.waitforimages + +class @IssuableContext + constructor: -> + new UsersSelect() + $('select.select2').select2({width: 'resolve', dropdownAutoWidth: true}) + + $(".context .inline-update").on "change", "select", -> + $(this).submit() + $(".context .inline-update").on "change", ".js-assignee", -> + $(this).submit() + + $('.issuable-details').waitForImages -> + $('.issuable-affix').affix offset: + top: -> + @top = ($('.issuable-affix').offset().top - 70) + bottom: -> + @bottom = $('.footer').outerHeight(true) + $('.issuable-affix').on 'affix.bs.affix', -> + $(@).width($(@).outerWidth()) + .on 'affixed-top.bs.affix affixed-bottom.bs.affix', -> + $(@).width('') diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee index 74d6b80be5e..9bb85300848 100644 --- a/app/assets/javascripts/issue.js.coffee +++ b/app/assets/javascripts/issue.js.coffee @@ -3,29 +3,12 @@ class @Issue constructor: -> - $('.edit-issue.inline-update input[type="submit"]').hide() - $(".context .inline-update").on "change", "select", -> - $(this).submit() - $(".context .inline-update").on "change", "#issue_assignee_id", -> - $(this).submit() - # Prevent duplicate event bindings @disableTaskList() if $("a.btn-close").length @initTaskList() - $('.issue-details').waitForImages -> - $('.issuable-affix').affix offset: - top: -> - @top = ($('.issuable-affix').offset().top - 70) - bottom: -> - @bottom = $('.footer').outerHeight(true) - $('.issuable-affix').on 'affix.bs.affix', -> - $(@).width($(@).outerWidth()) - .on 'affixed-top.bs.affix affixed-bottom.bs.affix', -> - $(@).width('') - initTaskList: -> $('.issue-details .js-task-list-container').taskList('enable') $(document).on 'tasklist:changed', '.issue-details .js-task-list-container', @updateTaskList diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index 5c0bc686111..ac5471702f3 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -26,28 +26,10 @@ class @MergeRequest if $("a.btn-close").length @initTaskList() - $('.merge-request-details').waitForImages -> - $('.issuable-affix').affix offset: - top: -> - @top = ($('.issuable-affix').offset().top - 70) - bottom: -> - @bottom = $('.footer').outerHeight(true) - $('.issuable-affix').on 'affix.bs.affix', -> - $(@).width($(@).outerWidth()) - .on 'affixed-top.bs.affix affixed-bottom.bs.affix', -> - $(@).width('') - # Local jQuery finder $: (selector) -> this.$el.find(selector) - initContextWidget: -> - $('.edit-merge_request.inline-update input[type="submit"]').hide() - $(".context .inline-update").on "change", "select", -> - $(this).submit() - $(".context .inline-update").on "change", "#merge_request_assignee_id", -> - $(this).submit() - showAllCommits: -> this.$('.first-commits').remove() this.$('.all-commits').removeClass 'hide' diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index 9703c8d9e9c..9d072f81092 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -52,4 +52,12 @@ module GitlabRoutingHelper def project_snippet_url(entity, *args) namespace_project_snippet_url(entity.project.namespace, entity.project, entity, *args) end + + def toggle_subscription_path(entity, *args) + if entity.is_a?(Issue) + toggle_subscription_namespace_project_issue_path(entity.project.namespace, entity.project, entity) + else + toggle_subscription_namespace_project_merge_request_path(entity.project.namespace, entity.project, entity) + end + end end diff --git a/app/views/projects/issues/_discussion.html.haml b/app/views/projects/issues/_discussion.html.haml index 656e06ca105..a099e597294 100644 --- a/app/views/projects/issues/_discussion.html.haml +++ b/app/views/projects/issues/_discussion.html.haml @@ -23,7 +23,7 @@ = cross_project_reference(@project, @issue) %hr .context - = render partial: 'issue_context', locals: { issue: @issue } + = render 'shared/issuable/context', issuable: @issue - if @issue.labels.any? .issuable-context-title diff --git a/app/views/projects/issues/_issue_context.html.haml b/app/views/projects/issues/_issue_context.html.haml deleted file mode 100644 index 88b63946905..00000000000 --- a/app/views/projects/issues/_issue_context.html.haml +++ /dev/null @@ -1,47 +0,0 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @issue], remote: true, html: {class: 'edit-issue inline-update js-issue-update'} do |f| - %div.prepend-top-20 - .issuable-context-title - %label - Assignee: - - if issue.assignee - %strong= link_to_member(@project, @issue.assignee, size: 24) - - else - none - - if can?(current_user, :modify_issue, @issue) - = users_select_tag('issue[assignee_id]', placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: @issue.assignee_id, null_user: true, first_user: true) - - %div.prepend-top-20.clearfix - .issuable-context-title - %label - Milestone: - - if issue.milestone - %span.back-to-milestone - = link_to namespace_project_milestone_path(@project.namespace, @project, @issue.milestone) do - %strong - %i.fa.fa-clock-o - = @issue.milestone.title - - else - none - - if can?(current_user, :modify_issue, @issue) - = f.select(:milestone_id, milestone_options(@issue), { include_blank: "Select milestone" }, {class: 'select2 select2-compact js-select2 js-milestone'}) - = hidden_field_tag :issue_context - = f.submit class: 'btn' - - - if current_user - - subscribed = @issue.subscribed?(current_user) - %div.prepend-top-20.clearfix - .issuable-context-title - %label - Subscription: - %button.btn.btn-block.subscribe-button{:type => 'button'} - %i.fa.fa-eye - %span= subscribed ? "Unsubscribe" : "Subscribe" - - subscribtion_status = subscribed ? "subscribed" : "unsubscribed" - .subscription-status{"data-status" => subscribtion_status} - .description-block.unsubscribed{class: ( "hidden" if subscribed )} - You're not receiving notifications from this thread. - .description-block.subscribed{class: ( "hidden" unless subscribed )} - You're receiving notifications because you're subscribed to this thread. - -:coffeescript - new Subscription("#{toggle_subscription_namespace_project_issue_path(@issue.project.namespace, @project, @issue)}") diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index ee1b2a08bc4..5bbb1fd4e92 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -1,6 +1,6 @@ - page_title "#{@issue.title} (##{@issue.iid})", "Issues" .issue - .issue-details + .issue-details.issuable-details %h4.page-title .issue-box{ class: issue_box_class(@issue) } - if @issue.closed? diff --git a/app/views/projects/issues/update.js.haml b/app/views/projects/issues/update.js.haml index 1d38662bff8..b7735aaf3c1 100644 --- a/app/views/projects/issues/update.js.haml +++ b/app/views/projects/issues/update.js.haml @@ -1,17 +1,3 @@ -- if params[:status_only] - - if @issue.valid? - :plain - $("##{dom_id(@issue)}").fadeOut(); -- elsif params[:issue_context] - $('.context').html("#{escape_javascript(render partial: 'issue_context', locals: { issue: @issue })}"); - $('.context').effect('highlight'); - - if @issue.milestone - $('.milestone-nav-link').replaceWith("| Milestone #{escape_javascript(link_to @issue.milestone.title, namespace_project_milestone_path(@issue.project.namespace, @issue.project, @issue.milestone))}") - - else - $('.milestone-nav-link').html('') - - -$('select.select2').select2({width: 'resolve', dropdownAutoWidth: true}) -$('.edit-issue.inline-update input[type="submit"]').hide(); -new UsersSelect() +$('.context').html("#{escape_javascript(render 'shared/issuable/context', issuable: @issue)}"); +$('.context').effect('highlight') new Issue(); diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml index eb3dba6858d..76088b9c862 100644 --- a/app/views/projects/merge_requests/_discussion.html.haml +++ b/app/views/projects/merge_requests/_discussion.html.haml @@ -20,7 +20,7 @@ = cross_project_reference(@project, @merge_request) %hr .context - = render partial: 'projects/merge_requests/show/context', locals: { merge_request: @merge_request } + = render 'shared/issuable/context', issuable: @merge_request - if @merge_request.labels.any? .issuable-context-title diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 9dc4a47258e..b6d9b135c70 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -1,6 +1,6 @@ - page_title "#{@merge_request.title} (##{@merge_request.iid})", "Merge Requests" .merge-request{'data-url' => merge_request_path(@merge_request)} - .merge-request-details + .merge-request-details.issuable-details = render "projects/merge_requests/show/mr_title" %hr = render "projects/merge_requests/show/mr_box" diff --git a/app/views/projects/merge_requests/show/_context.html.haml b/app/views/projects/merge_requests/show/_context.html.haml deleted file mode 100644 index 5f2f65e0087..00000000000 --- a/app/views/projects/merge_requests/show/_context.html.haml +++ /dev/null @@ -1,49 +0,0 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @merge_request], remote: true, html: {class: 'edit-merge_request inline-update js-merge-request-update'} do |f| - %div.prepend-top-20 - .issuable-context-title - %label - Assignee: - - if @merge_request.assignee - %strong= link_to_member(@project, @merge_request.assignee, size: 24) - - else - none - .issuable-context-selectbox - - if can?(current_user, :modify_merge_request, @merge_request) - = users_select_tag('merge_request[assignee_id]', placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: @merge_request.assignee_id, project: @target_project, null_user: true) - - %div.prepend-top-20.clearfix - .issuable-context-title - %label - Milestone: - - if @merge_request.milestone - %span.back-to-milestone - = link_to namespace_project_milestone_path(@project.namespace, @project, @merge_request.milestone) do - %strong - = icon('clock-o') - = @merge_request.milestone.title - - else - none - .issuable-context-selectbox - - if can?(current_user, :modify_merge_request, @merge_request) - = f.select(:milestone_id, milestone_options(@merge_request), { include_blank: 'Select milestone' }, {class: 'select2 select2-compact js-select2 js-milestone'}) - = hidden_field_tag :merge_request_context - = f.submit class: 'btn' - - - if current_user - - subscribed = @merge_request.subscribed?(current_user) - %div.prepend-top-20.clearfix - .issuable-context-title - %label - Subscription: - %button.btn.btn-block.subscribe-button{:type => 'button'} - = icon('eye') - %span= subscribed ? 'Unsubscribe' : 'Subscribe' - - subscribtion_status = subscribed ? 'subscribed' : 'unsubscribed' - .subscription-status{data: {status: subscribtion_status}} - .description-block.unsubscribed{class: ( 'hidden' if subscribed )} - You're not receiving notifications from this thread. - .description-block.subscribed{class: ( 'hidden' unless subscribed )} - You're receiving notifications because you're subscribed to this thread. - -:coffeescript - new Subscription("#{toggle_subscription_namespace_project_merge_request_path(@merge_request.project.namespace, @project, @merge_request)}") diff --git a/app/views/projects/merge_requests/update.js.haml b/app/views/projects/merge_requests/update.js.haml index b4df1d20737..25583b2cc6f 100644 --- a/app/views/projects/merge_requests/update.js.haml +++ b/app/views/projects/merge_requests/update.js.haml @@ -1,8 +1,3 @@ -- if params[:merge_request_context] - $('.context').html("#{escape_javascript(render partial: 'projects/merge_requests/show/context', locals: { issue: @issue })}"); - $('.context').effect('highlight'); - - new UsersSelect() - - $('select.select2').select2({width: 'resolve', dropdownAutoWidth: true}); - merge_request = new MergeRequest(); +$('.context').html("#{escape_javascript(render 'shared/issuable/context', issuable: @merge_request)}"); +$('.context').effect('highlight') +merge_request = new MergeRequest(); diff --git a/app/views/shared/issuable/_context.html.haml b/app/views/shared/issuable/_context.html.haml new file mode 100644 index 00000000000..2a41448bb04 --- /dev/null +++ b/app/views/shared/issuable/_context.html.haml @@ -0,0 +1,50 @@ += form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, html: {class: 'issuable-context-form inline-update js-merge-request-update'} do |f| + %div.prepend-top-20 + .issuable-context-title + %label + Assignee: + - if issuable.assignee + %strong= link_to_member(@project, issuable.assignee, size: 24) + - else + none + .issuable-context-selectbox + - if can?(current_user, :set_assignee, @project) + = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: issuable.assignee_id, project: @target_project, null_user: true) + + %div.prepend-top-20.clearfix + .issuable-context-title + %label + Milestone: + - if issuable.milestone + %span.back-to-milestone + = link_to namespace_project_milestone_path(@project.namespace, @project, issuable.milestone) do + %strong + = icon('clock-o') + = issuable.milestone.title + - else + none + .issuable-context-selectbox + - if can?(current_user, :set_milestone, @project) + = f.select(:milestone_id, milestone_options(issuable), { include_blank: 'Select milestone' }, {class: 'select2 select2-compact js-select2 js-milestone'}) + = hidden_field_tag :issuable_context + = f.submit class: 'btn hide' + + - if current_user + - subscribed = issuable.subscribed?(current_user) + %div.prepend-top-20.clearfix + .issuable-context-title + %label + Subscription: + %button.btn.btn-block.subscribe-button{:type => 'button'} + = icon('eye') + %span= subscribed ? 'Unsubscribe' : 'Subscribe' + - subscribtion_status = subscribed ? 'subscribed' : 'unsubscribed' + .subscription-status{data: {status: subscribtion_status}} + .description-block.unsubscribed{class: ( 'hidden' if subscribed )} + You're not receiving notifications from this thread. + .description-block.subscribed{class: ( 'hidden' unless subscribed )} + You're receiving notifications because you're subscribed to this thread. + +:coffeescript + new Subscription("#{toggle_subscription_path(issuable)}") + new IssuableContext() From 5ff870a044150bef027b903bd944b8cc0578f798 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 16:17:06 +0200 Subject: [PATCH 04/13] Move duplicate js functionality into IssuableForm Signed-off-by: Dmitriy Zaporozhets --- app/assets/javascripts/dispatcher.js.coffee | 10 ++-------- app/assets/javascripts/issuable_form.js.coffee | 4 ++++ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 45383fc96aa..809b06a18a8 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -31,20 +31,14 @@ class Dispatcher when 'projects:compare:show' new Diff() when 'projects:issues:new','projects:issues:edit' - GitLab.GfmAutoComplete.setup() shortcut_handler = new ShortcutsNavigation() - new ZenMode() new DropzoneInput($('.issue-form')) - if page == 'projects:issues:new' - new IssuableForm($('.issue-form')) + new IssuableForm($('.issue-form')) when 'projects:merge_requests:new', 'projects:merge_requests:edit' - GitLab.GfmAutoComplete.setup() new Diff() shortcut_handler = new ShortcutsNavigation() - new ZenMode() new DropzoneInput($('.merge-request-form')) - if page == 'projects:merge_requests:new' - new IssuableForm($('.merge-request-form')) + new IssuableForm($('.merge-request-form')) when 'projects:merge_requests:show' new Diff() shortcut_handler = new ShortcutsIssuable() diff --git a/app/assets/javascripts/issuable_form.js.coffee b/app/assets/javascripts/issuable_form.js.coffee index abd58bcf978..48c249943f2 100644 --- a/app/assets/javascripts/issuable_form.js.coffee +++ b/app/assets/javascripts/issuable_form.js.coffee @@ -1,5 +1,9 @@ class @IssuableForm constructor: (@form) -> + GitLab.GfmAutoComplete.setup() + new UsersSelect() + new ZenMode() + @titleField = @form.find("input[name*='[title]']") @descriptionField = @form.find("textarea[name*='[description]']") From 0bcfe9a0dcf630b166376bf05de966132d6ee45d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 16:17:48 +0200 Subject: [PATCH 05/13] Dont allow set assignee, milestone or labels if user is guest Signed-off-by: Dmitriy Zaporozhets --- app/services/issuable_base_service.rb | 14 ++++++++++++++ app/services/issues/create_service.rb | 1 + app/services/issues/update_service.rb | 1 + app/services/merge_requests/create_service.rb | 1 + app/services/merge_requests/update_service.rb | 1 + 5 files changed, 18 insertions(+) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 1d99223cfe6..cb544eaf89b 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -26,4 +26,18 @@ class IssuableBaseService < BaseService issuable, issuable.project, current_user, branch_type, old_branch, new_branch) end + + def filter_params + unless can?(current_user, :set_milestone, project) + params.delete(:milestone_id) + end + + unless can?(current_user, :set_label, project) + params.delete(:label_ids) + end + + unless can?(current_user, :set_assignee, project) + params.delete(:assignee_id) + end + end end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index d5c17906a55..1ea4b72216c 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -1,6 +1,7 @@ module Issues class CreateService < Issues::BaseService def execute + filter_params label_params = params[:label_ids] issue = project.issues.new(params.except(:label_ids)) issue.author = current_user diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 6af942a5ca4..3220facaf7c 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -17,6 +17,7 @@ module Issues params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE + filter_params old_labels = issue.labels.to_a if params.present? && issue.update_attributes(params.except(:state_event, diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index ca8d80f6c0c..f431c5d5534 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -1,6 +1,7 @@ module MergeRequests class CreateService < MergeRequests::BaseService def execute + filter_params label_params = params[:label_ids] merge_request = MergeRequest.new(params.except(:label_ids)) merge_request.source_project = project diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 4f6c6cba9a9..f6570f52241 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -27,6 +27,7 @@ module MergeRequests params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE + filter_params old_labels = merge_request.labels.to_a if params.present? && merge_request.update_attributes( From 6006065e0606da79915f3ad3053dff70d4e5893d Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 16:18:32 +0200 Subject: [PATCH 06/13] Update CHANGELOG Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 4095ac48782..3971b924277 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -21,6 +21,7 @@ v 7.13.0 (unreleased) - Show a user's Two-factor Authentication status in the administration area. - Explicit error when commit not found in the CI - Improve performance for issue and merge request pages + - Users with guest access level can not set assignee, labels or milestones for issue and merge request v 7.12.0 (unreleased) - Fix Error 500 when one user attempts to access a personal, internal snippet (Stan Hu) From cbb64a4f6344cd0719d048ba41147a58c0cc2065 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 16:37:52 +0200 Subject: [PATCH 07/13] Fix js error on merge request page Signed-off-by: Dmitriy Zaporozhets --- app/assets/javascripts/merge_request.js.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index ac5471702f3..d9b8a98ea28 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -10,7 +10,6 @@ class @MergeRequest # action - String, current controller action # constructor: (@opts) -> - @initContextWidget() this.$el = $('.merge-request') this.$('.show-all-commits').on 'click', => From 304386aa02647a8079c6df5706786fbf89c4ad72 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 16:38:14 +0200 Subject: [PATCH 08/13] Write test for submitting issue as guest Signed-off-by: Dmitriy Zaporozhets --- features/project/issues/issues.feature | 12 ++++++++++++ features/steps/project/issues/issues.rb | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index bf84e2f8e87..a15298fc452 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -184,3 +184,15 @@ Feature: Project Issues Then I should see that I am subscribed When I click button "Unsubscribe" Then I should see that I am unsubscribed + + Scenario: I submit new unassigned issue as guest + Given I logout + Given public project "Community" + When I visit project "Community" page + And I click link "New Issue" + And I should not see assignee field + And I should not see milestone field + And I should not see labels field + And I submit new issue "500 error on profile" + Then I should see issue "500 error on profile" + diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index 6873c043e19..c3b34ab3f19 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -262,6 +262,24 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps end end + step 'I should not see labels field' do + within '.issue-form' do + expect(page).not_to have_content("Labels") + end + end + + step 'I should not see milestone field' do + within '.issue-form' do + expect(page).not_to have_content("Milestone") + end + end + + step 'I should not see assignee field' do + within '.issue-form' do + expect(page).not_to have_content("Assign to") + end + end + def filter_issue(text) fill_in 'issue_search', with: text end From a9095280c7954873fa391c9b91822308bdb52901 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Thu, 25 Jun 2015 16:54:35 +0200 Subject: [PATCH 09/13] UsersSelect is part of filter so I rather keep js executed in partial Signed-off-by: Dmitriy Zaporozhets --- app/assets/javascripts/dispatcher.js.coffee | 7 ------- app/views/shared/issuable/_filter.html.haml | 2 ++ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/dispatcher.js.coffee b/app/assets/javascripts/dispatcher.js.coffee index 809b06a18a8..a8ec0abc264 100644 --- a/app/assets/javascripts/dispatcher.js.coffee +++ b/app/assets/javascripts/dispatcher.js.coffee @@ -107,13 +107,6 @@ class Dispatcher new NamespaceSelect() when 'dashboard' shortcut_handler = new ShortcutsDashboardNavigation() - switch path[1] - when 'issues', 'merge_requests' - new UsersSelect() - when 'groups' - switch path[1] - when 'issues', 'merge_requests' - new UsersSelect() when 'profiles' new Profile() when 'projects' diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index a355eb62813..c03a34de446 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -64,6 +64,8 @@ = button_tag "Update issues", class: "btn update_selected_issues btn-save" :coffeescript + new UsersSelect() + $('form.filter-form').on 'submit', (event) -> event.preventDefault() Turbolinks.visit @.action + '&' + $(@).serialize() From b9fa82daeeb91d8121e096fec0db4981b46dd5b9 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Jun 2015 15:05:30 +0200 Subject: [PATCH 10/13] Update tests and use js-issuable class for context form Signed-off-by: Dmitriy Zaporozhets --- app/assets/javascripts/issue.js.coffee | 2 +- app/assets/javascripts/merge_request.js.coffee | 2 +- app/views/projects/issues/_issue.html.haml | 2 +- app/views/shared/issuable/_context.html.haml | 2 +- spec/features/issues_spec.rb | 4 ++-- spec/features/task_lists_spec.rb | 6 +++--- spec/javascripts/fixtures/issues_show.html.haml | 2 +- spec/javascripts/fixtures/merge_requests_show.html.haml | 2 +- 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee index 9bb85300848..603a16da1ce 100644 --- a/app/assets/javascripts/issue.js.coffee +++ b/app/assets/javascripts/issue.js.coffee @@ -25,5 +25,5 @@ class @Issue $.ajax type: 'PATCH' - url: $('form.js-issue-update').attr('action') + url: $('form.js-issuable-update').attr('action') data: patchData diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee index d9b8a98ea28..7462975bd3d 100644 --- a/app/assets/javascripts/merge_request.js.coffee +++ b/app/assets/javascripts/merge_request.js.coffee @@ -49,5 +49,5 @@ class @MergeRequest $.ajax type: 'PATCH' - url: $('form.js-merge-request-update').attr('action') + url: $('form.js-issuable-update').attr('action') data: patchData diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 2c296cab977..3791af55e92 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -1,7 +1,7 @@ %li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue) } - if controller.controller_name == 'issues' .issue-check - = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue", disabled: !can?(current_user, :modify_issue, issue) + = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue", disabled: !can?(current_user, :set_assignee, issue) .issue-title %span.issue-title-text diff --git a/app/views/shared/issuable/_context.html.haml b/app/views/shared/issuable/_context.html.haml index 2a41448bb04..5fb6c5155cb 100644 --- a/app/views/shared/issuable/_context.html.haml +++ b/app/views/shared/issuable/_context.html.haml @@ -1,4 +1,4 @@ -= form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, html: {class: 'issuable-context-form inline-update js-merge-request-update'} do |f| += form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f| %div.prepend-top-20 .issuable-context-title %label diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 158a5c0c29c..808a6eeb958 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -218,7 +218,7 @@ describe 'Issues', feature: true do it 'with dropdown menu' do visit namespace_project_issue_path(project.namespace, project, issue) - find('.edit-issue.inline-update #issue_assignee_id'). + find('.context #issue_assignee_id'). set project.team.members.first.id click_button 'Update Issue' @@ -257,7 +257,7 @@ describe 'Issues', feature: true do it 'with dropdown menu' do visit namespace_project_issue_path(project.namespace, project, issue) - find('.edit-issue.inline-update'). + find('.context'). select(milestone.title, from: 'issue_milestone_id') click_button 'Update Issue' diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index 2099fc40cca..fca3c77fc64 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'Task Lists' do +feature 'Task Lists', feature: true do include Warden::Test::Helpers let(:project) { create(:project) } @@ -52,7 +52,7 @@ feature 'Task Lists' do expect(page).to have_selector(container) expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox") expect(page).to have_selector("#{container} .js-task-list-field") - expect(page).to have_selector('form.js-issue-update') + expect(page).to have_selector('form.js-issuable-update') expect(page).to have_selector('a.btn-close') end @@ -128,7 +128,7 @@ feature 'Task Lists' do expect(page).to have_selector(container) expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox") expect(page).to have_selector("#{container} .js-task-list-field") - expect(page).to have_selector('form.js-merge-request-update') + expect(page).to have_selector('form.js-issuable-update') expect(page).to have_selector('a.btn-close') end diff --git a/spec/javascripts/fixtures/issues_show.html.haml b/spec/javascripts/fixtures/issues_show.html.haml index db5abe0cae3..7e8b2a64351 100644 --- a/spec/javascripts/fixtures/issues_show.html.haml +++ b/spec/javascripts/fixtures/issues_show.html.haml @@ -10,4 +10,4 @@ %textarea.js-task-list-field \- [ ] Task List Item -%form.js-issue-update{action: '/foo'} +%form.js-issuable-update{action: '/foo'} diff --git a/spec/javascripts/fixtures/merge_requests_show.html.haml b/spec/javascripts/fixtures/merge_requests_show.html.haml index c4329b8f94a..f0c622935f8 100644 --- a/spec/javascripts/fixtures/merge_requests_show.html.haml +++ b/spec/javascripts/fixtures/merge_requests_show.html.haml @@ -10,4 +10,4 @@ %textarea.js-task-list-field \- [ ] Task List Item -%form.js-merge-request-update{action: '/foo'} +%form.js-issuable-update{action: '/foo'} From d5947ada4493335d49368ec193f4b9b801582deb Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Jun 2015 15:45:45 +0200 Subject: [PATCH 11/13] Simplify set of assignee, milestone and label to admin_issue rule Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/pages/issues.scss | 6 ------ app/models/ability.rb | 4 ---- app/services/issuable_base_service.rb | 8 +------- app/views/projects/issues/_issue.html.haml | 4 ++-- app/views/shared/issuable/_context.html.haml | 4 ++-- app/views/shared/issuable/_filter.html.haml | 5 ++--- app/views/shared/issuable/_form.html.haml | 4 +--- 7 files changed, 8 insertions(+), 27 deletions(-) diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index ed938f86b35..3572f33e91f 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -145,9 +145,3 @@ h2.issue-title { .issue-form .select2-container { width: 250px !important; } - -.issues-holder { - .issue-info { - margin-left: 20px; - } -} diff --git a/app/models/ability.rb b/app/models/ability.rb index b611276cf9c..c90c99c5b5f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -154,9 +154,6 @@ class Ability project_guest_rules + [ :download_code, :fork_project, - :set_assignee, - :set_milestone, - :set_label, :write_project_snippet ] end @@ -188,7 +185,6 @@ class Ability :modify_issue, :modify_project_snippet, :modify_merge_request, - :admin_issue, :admin_milestone, :admin_project_snippet, :admin_project_member, diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index cb544eaf89b..f1ef5ca84fe 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -28,15 +28,9 @@ class IssuableBaseService < BaseService end def filter_params - unless can?(current_user, :set_milestone, project) + unless can?(current_user, :admin_issue, project) params.delete(:milestone_id) - end - - unless can?(current_user, :set_label, project) params.delete(:label_ids) - end - - unless can?(current_user, :set_assignee, project) params.delete(:assignee_id) end end diff --git a/app/views/projects/issues/_issue.html.haml b/app/views/projects/issues/_issue.html.haml index 3791af55e92..cdb3839d13b 100644 --- a/app/views/projects/issues/_issue.html.haml +++ b/app/views/projects/issues/_issue.html.haml @@ -1,7 +1,7 @@ %li{ id: dom_id(issue), class: issue_css_classes(issue), url: issue_path(issue) } - - if controller.controller_name == 'issues' + - if controller.controller_name == 'issues' && can?(current_user, :admin_issue, @project) .issue-check - = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue", disabled: !can?(current_user, :set_assignee, issue) + = check_box_tag dom_id(issue,"selected"), nil, false, 'data-id' => issue.id, class: "selected_issue" .issue-title %span.issue-title-text diff --git a/app/views/shared/issuable/_context.html.haml b/app/views/shared/issuable/_context.html.haml index 5fb6c5155cb..46990895d33 100644 --- a/app/views/shared/issuable/_context.html.haml +++ b/app/views/shared/issuable/_context.html.haml @@ -8,7 +8,7 @@ - else none .issuable-context-selectbox - - if can?(current_user, :set_assignee, @project) + - if can?(current_user, :admin_issue, @project) = users_select_tag("#{issuable.class.table_name.singularize}[assignee_id]", placeholder: 'Select assignee', class: 'custom-form-control js-select2 js-assignee', selected: issuable.assignee_id, project: @target_project, null_user: true) %div.prepend-top-20.clearfix @@ -24,7 +24,7 @@ - else none .issuable-context-selectbox - - if can?(current_user, :set_milestone, @project) + - if can?(current_user, :admin_issue, @project) = f.select(:milestone_id, milestone_options(issuable), { include_blank: 'Select milestone' }, {class: 'select2 select2-compact js-select2 js-milestone'}) = hidden_field_tag :issuable_context = f.submit class: 'btn hide' diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index c03a34de446..a829782fc4f 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -29,11 +29,10 @@ .issues-details-filters = form_tag page_filter_path(without: [:assignee_id, :author_id, :milestone_title, :label_name]), method: :get, class: 'filter-form' do - - if controller.controller_name == 'issues' + - if controller.controller_name == 'issues' && can?(current_user, :admin_issue, @project) .check-all-holder = check_box_tag "check_all_issues", nil, false, - class: "check_all_issues left", - disabled: !can?(current_user, :modify_issue, @project) + class: "check_all_issues left" .issues-other-filters .filter-item.inline = users_select_tag(:assignee_id, selected: params[:assignee_id], diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index 7095402bf05..e434e1b6b98 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -38,7 +38,7 @@ .clearfix .error-alert %hr -- if can?(current_user, :set_assignee, @project) +- if can?(current_user, :admin_issue, @project) .form-group .issue-assignee = f.label :assignee_id, class: 'control-label' do @@ -50,7 +50,6 @@ selected: issuable.assignee_id, project: @target_project || @project)   = link_to 'Assign to me', '#', class: 'btn assign-to-me-link' -- if can?(current_user, :set_milestone, @project) .form-group .issue-milestone = f.label :milestone_id, class: 'control-label' do @@ -66,7 +65,6 @@   - if can? current_user, :admin_milestone, issuable.project = link_to 'Create new milestone', new_namespace_project_milestone_path(issuable.project.namespace, issuable.project), target: :blank -- if can?(current_user, :set_label, @project) .form-group = f.label :label_ids, class: 'control-label' do %i.fa.fa-tag From c9f21c9691033b1fb1d6ddb7dd48057ec4f25aa3 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Jun 2015 16:01:02 +0200 Subject: [PATCH 12/13] Use page.within in tests Signed-off-by: Dmitriy Zaporozhets --- features/steps/project/issues/issues.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index c3b34ab3f19..91d5c3688e2 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -263,19 +263,19 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps end step 'I should not see labels field' do - within '.issue-form' do + page.within '.issue-form' do expect(page).not_to have_content("Labels") end end step 'I should not see milestone field' do - within '.issue-form' do + page.within '.issue-form' do expect(page).not_to have_content("Milestone") end end step 'I should not see assignee field' do - within '.issue-form' do + page.within '.issue-form' do expect(page).not_to have_content("Assign to") end end From 58ceb8e95097df51d08a74c16de83152044e9c58 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Fri, 26 Jun 2015 16:02:57 +0200 Subject: [PATCH 13/13] Only people who can manage issue can assign labels to it Signed-off-by: Dmitriy Zaporozhets --- lib/api/issues.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/issues.rb b/lib/api/issues.rb index c8db93eb778..4d632ce77c1 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -157,7 +157,7 @@ module API if issue.valid? # Find or create labels and attach to issue. Labels are valid because # we already checked its name, so there can't be an error here - unless params[:labels].nil? + if params[:labels] && can?(current_user, :admin_issue, user_project) issue.remove_labels # Create and add labels to the new created issue issue.add_labels_by_names(params[:labels].split(','))