From 2f7effe804bdae9fca4ca0bb55c841664c2de978 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 19 Dec 2012 06:14:05 +0300 Subject: [PATCH] Refactor issues, Remove ajax --- app/assets/javascripts/issues.js | 73 ++------------------ app/controllers/issues_controller.rb | 14 +--- app/controllers/merge_requests_controller.rb | 13 +--- app/models/milestone.rb | 6 +- app/views/issues/_form.html.haml | 20 ++---- app/views/issues/_show.html.haml | 2 +- app/views/issues/create.js.haml | 10 --- app/views/issues/edit.js.haml | 4 -- app/views/issues/index.html.haml | 7 +- app/views/issues/new.js.haml | 3 - app/views/issues/update.js.haml | 14 ---- app/views/merge_requests/_form.html.haml | 2 +- app/views/milestones/_milestone.html.haml | 27 ++++---- config/routes.rb | 6 +- features/project/issues/issues.feature | 11 --- features/steps/project/project_issues.rb | 4 +- spec/requests/issues_spec.rb | 2 +- 17 files changed, 44 insertions(+), 174 deletions(-) delete mode 100644 app/views/issues/create.js.haml delete mode 100644 app/views/issues/edit.js.haml delete mode 100644 app/views/issues/new.js.haml delete mode 100644 app/views/issues/update.js.haml diff --git a/app/assets/javascripts/issues.js b/app/assets/javascripts/issues.js index e2fe1075..719d2c17 100644 --- a/app/assets/javascripts/issues.js +++ b/app/assets/javascripts/issues.js @@ -1,43 +1,3 @@ -function switchToNewIssue(){ - $(".issues_content").hide("fade", { direction: "left" }, 150, function(){ - $('select#issue_assignee_id').chosen(); - $('select#issue_milestone_id').chosen(); - $("#new_issue_dialog").show("fade", { direction: "right" }, 150); - $('.top-tabs .add_new').hide(); - disableButtonIfEmptyField("#issue_title", ".save-btn"); - GitLab.GfmAutoComplete.setup(); - }); -} - -function switchToEditIssue(){ - $(".issues_content").hide("fade", { direction: "left" }, 150, function(){ - $('select#issue_assignee_id').chosen(); - $('select#issue_milestone_id').chosen(); - $("#edit_issue_dialog").show("fade", { direction: "right" }, 150); - $('.add_new').hide(); - disableButtonIfEmptyField("#issue_title", ".save-btn"); - GitLab.GfmAutoComplete.setup(); - }); -} - -function switchFromNewIssue(){ - backToIssues(); -} - -function switchFromEditIssue(){ - backToIssues(); -} - -function backToIssues(){ - $("#edit_issue_dialog, #new_issue_dialog").hide("fade", { direction: "right" }, 150, function(){ - $(".issues_content").show("fade", { direction: "left" }, 150, function() { - $("#edit_issue_dialog").html(""); - $("#new_issue_dialog").html(""); - $('.add_new').show(); - }); - }); -} - function initIssuesSearch() { var href = $('#issue_search_form').attr('action'); var last_terms = ''; @@ -76,23 +36,15 @@ function issuesPage(){ $(this).closest("form").submit(); }); - $("#new_issue_link").click(function(){ - updateNewIssueURL(); - }); - - $('body').on('ajax:success', '.close_issue, .reopen_issue, #new_issue', function(){ + $('body').on('ajax:success', '.close_issue, .reopen_issue', function(){ var t = $(this), totalIssues, - reopen = t.hasClass('reopen_issue'), - newIssue = false; - if( this.id == 'new_issue' ){ - newIssue = true; - } - $('.issue_counter, #new_issue').each(function(){ + reopen = t.hasClass('reopen_issue'); + $('.issue_counter').each(function(){ var issue = $(this); totalIssues = parseInt( $(this).html(), 10 ); - if( newIssue || ( reopen && issue.closest('.main_menu').length ) ){ + if( reopen && issue.closest('.main_menu').length ){ $(this).html( totalIssues+1 ); }else { $(this).html( totalIssues-1 ); @@ -126,20 +78,3 @@ function issuesCheckChanged() { $('.issues_filters').show(); } } - -function updateNewIssueURL(){ - var new_issue_link = $("#new_issue_link"); - var milestone_id = $("#milestone_id").val(); - var assignee_id = $("#assignee_id").val(); - var new_href = ""; - if(milestone_id){ - new_href = "issue[milestone_id]=" + milestone_id + "&"; - } - if(assignee_id){ - new_href = new_href + "issue[assignee_id]=" + assignee_id; - } - if(new_href.length){ - new_href = new_issue_link.attr("href") + "?" + new_href; - new_issue_link.attr("href", new_href); - } -}; diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 0f28fc3a..5a1ce2cf 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -1,6 +1,6 @@ class IssuesController < ProjectResourceController before_filter :module_enabled - before_filter :issue, only: [:edit, :update, :destroy, :show] + before_filter :issue, only: [:edit, :update, :show] # Allow read any issue before_filter :authorize_read_issue! @@ -11,9 +11,6 @@ class IssuesController < ProjectResourceController # Allow modify issue before_filter :authorize_modify_issue!, only: [:edit, :update] - # Allow destroy issue - before_filter :authorize_admin_issue!, only: [:destroy] - respond_to :js, :html def index @@ -77,15 +74,6 @@ class IssuesController < ProjectResourceController end end - def destroy - @issue.destroy - - respond_to do |format| - format.html { redirect_to project_issues_path } - format.js { render nothing: true } - end - end - def sort return render_404 unless can?(current_user, :admin_issue, @project) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 72f6445a..fa4eaff8 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -1,6 +1,6 @@ class MergeRequestsController < ProjectResourceController before_filter :module_enabled - before_filter :merge_request, only: [:edit, :update, :destroy, :show, :commits, :diffs, :automerge, :automerge_check, :ci_status] + before_filter :merge_request, only: [:edit, :update, :show, :commits, :diffs, :automerge, :automerge_check, :ci_status] before_filter :validates_merge_request, only: [:show, :diffs] before_filter :define_show_vars, only: [:show, :diffs] @@ -13,9 +13,6 @@ class MergeRequestsController < ProjectResourceController # Allow modify merge_request before_filter :authorize_modify_merge_request!, only: [:close, :edit, :update, :sort] - # Allow destroy merge_request - before_filter :authorize_admin_merge_request!, only: [:destroy] - def index @merge_requests = MergeRequestsLoadContext.new(project, current_user, params).execute end @@ -85,14 +82,6 @@ class MergeRequestsController < ProjectResourceController end end - def destroy - @merge_request.destroy - - respond_to do |format| - format.html { redirect_to project_merge_requests_url(@project) } - end - end - def branch_from @commit = project.commit(params[:ref]) @commit = CommitDecorator.decorate(@commit) diff --git a/app/models/milestone.rb b/app/models/milestone.rb index 4be08736..4fac9bec 100644 --- a/app/models/milestone.rb +++ b/app/models/milestone.rb @@ -62,7 +62,11 @@ class Milestone < ActiveRecord::Base end def can_be_closed? - issues.count > 0 && open? && issues.opened.count.zero? + open? && issues.opened.count.zero? + end + + def is_empty? + total_items_count.zero? end def open? diff --git a/app/views/issues/_form.html.haml b/app/views/issues/_form.html.haml index 8a9a91a3..030f797c 100644 --- a/app/views/issues/_form.html.haml +++ b/app/views/issues/_form.html.haml @@ -1,18 +1,18 @@ %div.issue-form-holder %h3.page_title= @issue.new_record? ? "New Issue" : "Edit Issue ##{@issue.id}" - = form_for [@project, @issue], remote: request.xhr? do |f| + = form_for [@project, @issue] do |f| -if @issue.errors.any? .alert-message.block-message.error - %ul - - @issue.errors.full_messages.each do |msg| - %li= msg + - @issue.errors.full_messages.each do |msg| + %span= msg + %br .issue_form_box .issue_title .clearfix = f.label :title do %strong= "Subject *" .input - = f.text_field :title, maxlength: 255, class: "xxlarge js-gfm-input", autofocus: true + = f.text_field :title, maxlength: 255, class: "xxlarge js-gfm-input", autofocus: true, required: true .issue_middle_block .issue_assignee = f.label :assignee_id do @@ -47,14 +47,8 @@ -else = f.submit 'Save changes', class: "save-btn btn" - - cancel_class = 'btn cancel-btn' - - if request.xhr? - = link_to "Cancel", "#back", onclick: "backToIssues();", class: cancel_class - - else - - if @issue.new_record? - = link_to "Cancel", project_issues_path(@project), class: cancel_class - - else - = link_to "Cancel", project_issue_path(@project, @issue), class: cancel_class + - cancel_path = @issue.new_record? ? project_issues_path(@project) : project_issue_path(@project, @issue) + = link_to "Cancel", cancel_path, class: 'btn cancel-btn' diff --git a/app/views/issues/_show.html.haml b/app/views/issues/_show.html.haml index b4c9ed98..4641e8bd 100644 --- a/app/views/issues/_show.html.haml +++ b/app/views/issues/_show.html.haml @@ -16,7 +16,7 @@ = link_to 'Reopen', project_issue_path(issue.project, issue, issue: {closed: false }, status_only: true), method: :put, class: "btn small grouped reopen_issue", remote: true - else = link_to 'Close', project_issue_path(issue.project, issue, issue: {closed: true }, status_only: true), method: :put, class: "btn small grouped close_issue", remote: true - = link_to edit_project_issue_path(issue.project, issue), class: "btn small edit-issue-link grouped", remote: true do + = link_to edit_project_issue_path(issue.project, issue), class: "btn small edit-issue-link grouped" do %i.icon-edit Edit diff --git a/app/views/issues/create.js.haml b/app/views/issues/create.js.haml deleted file mode 100644 index d90cbf0d..00000000 --- a/app/views/issues/create.js.haml +++ /dev/null @@ -1,10 +0,0 @@ -- if @issue.valid? - :plain - switchFromNewIssue(); - $("#issues-table").prepend("#{escape_javascript(render(partial: 'show', locals: {issue: @issue}))}"); - $.ajax({type: "GET", url: location.href, dataType: "script"}); -- else - :plain - $("#new_issue_dialog").empty(); - $("#new_issue_dialog").append("#{escape_javascript(render('form'))}"); - $('select#issue_assignee_id').chosen(); diff --git a/app/views/issues/edit.js.haml b/app/views/issues/edit.js.haml deleted file mode 100644 index a994572f..00000000 --- a/app/views/issues/edit.js.haml +++ /dev/null @@ -1,4 +0,0 @@ -:plain - $("#edit_issue_dialog").html("#{escape_javascript(render('form'))}"); - switchToEditIssue(); - diff --git a/app/views/issues/index.html.haml b/app/views/issues/index.html.haml index 4f551fff..a3b47cab 100644 --- a/app/views/issues/index.html.haml +++ b/app/views/issues/index.html.haml @@ -1,4 +1,6 @@ = render "issues/head" +#new_issue_dialog +#edit_issue_dialog .issues_content %h3.page_title Issues @@ -6,7 +8,7 @@ .right .span5 - if can? current_user, :write_issue, @project - = link_to new_project_issue_path(@project), class: "right btn", title: "New Issue", remote: true, id: "new_issue_link" do + = link_to new_project_issue_path(@project, issue: { assignee_id: params[:assignee_id], milestone_id: params[:milestone_id]}), class: "right btn", title: "New Issue", id: "new_issue_link" do %i.icon-plus New Issue = form_tag search_project_issues_path(@project), method: :get, remote: true, id: "issue_search_form", class: :right do @@ -58,9 +60,6 @@ %ul#issues-table.well-list.issues_table = render "issues" -#new_issue_dialog -#edit_issue_dialog - :javascript $(function(){ issuesPage(); diff --git a/app/views/issues/new.js.haml b/app/views/issues/new.js.haml deleted file mode 100644 index 4cbcc563..00000000 --- a/app/views/issues/new.js.haml +++ /dev/null @@ -1,3 +0,0 @@ -:plain - $("#new_issue_dialog").html("#{escape_javascript(render('form'))}"); - switchToNewIssue(); diff --git a/app/views/issues/update.js.haml b/app/views/issues/update.js.haml deleted file mode 100644 index 44722895..00000000 --- a/app/views/issues/update.js.haml +++ /dev/null @@ -1,14 +0,0 @@ -- if params[:status_only] - - if @issue.valid? - :plain - $("##{dom_id(@issue)}").fadeOut(); -- else - - if @issue.valid? - :plain - updatePage(); - switchFromEditIssue(); - - else - :plain - $("#edit_issue_dialog").empty(); - $("#edit_issue_dialog").append("#{escape_javascript(render('form'))}"); - $('select#issue_assignee_id').chosen(); diff --git a/app/views/merge_requests/_form.html.haml b/app/views/merge_requests/_form.html.haml index 302e75cf..9606e2e5 100644 --- a/app/views/merge_requests/_form.html.haml +++ b/app/views/merge_requests/_form.html.haml @@ -32,7 +32,7 @@ .top_box_content = f.label :title do %strong= "Title *" - .input= f.text_field :title, class: "input-xxlarge pad js-gfm-input", maxlength: 255, rows: 5 + .input= f.text_field :title, class: "input-xxlarge pad js-gfm-input", maxlength: 255, rows: 5, required: true .merge_requests_middle_box .merge_requests_assignee = f.label :assignee_id do diff --git a/app/views/milestones/_milestone.html.haml b/app/views/milestones/_milestone.html.haml index 462b9e39..3864792f 100644 --- a/app/views/milestones/_milestone.html.haml +++ b/app/views/milestones/_milestone.html.haml @@ -10,15 +10,18 @@ %span.cred (Expired) %small = milestone.expires_at - .row - .span4 - .progress.progress-info - .bar{style: "width: #{milestone.percent_complete}%;"} - .span6 - = link_to project_issues_path(milestone.project, milestone_id: milestone.id) do - = pluralize milestone.issues.count, 'Issue' -   - = link_to project_merge_requests_path(milestone.project, milestone_id: milestone.id) do - = pluralize milestone.merge_requests.count, 'Merge Request' -   - %span.light #{milestone.percent_complete}% complete + - if milestone.is_empty? + %span.muted Empty + - else + .row + .span4 + .progress.progress-info + .bar{style: "width: #{milestone.percent_complete}%;"} + .span6 + = link_to project_issues_path(milestone.project, milestone_id: milestone.id) do + = pluralize milestone.issues.count, 'Issue' +   + = link_to project_merge_requests_path(milestone.project, milestone_id: milestone.id) do + = pluralize milestone.merge_requests.count, 'Merge Request' +   + %span.light #{milestone.percent_complete}% complete diff --git a/config/routes.rb b/config/routes.rb index 6e20ae77..7214a786 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -164,7 +164,7 @@ Gitlab::Application.routes.draw do end end - resources :merge_requests, constraints: {id: /\d+/} do + resources :merge_requests, constraints: {id: /\d+/}, except: [:destroy] do member do get :diffs get :automerge @@ -200,9 +200,9 @@ Gitlab::Application.routes.draw do :via => [:get, :post], constraints: {from: /.+/, to: /.+/} resources :team, controller: 'team_members', only: [:index] - resources :milestones + resources :milestones, except: [:destroy] resources :labels, only: [:index] - resources :issues do + resources :issues, except: [:destroy] do collection do post :sort post :bulk_update diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature index 99529373..5da42601 100644 --- a/features/project/issues/issues.feature +++ b/features/project/issues/issues.feature @@ -24,11 +24,9 @@ Feature: Project Issues Given I click link "Release 0.4" Then I should see issue "Release 0.4" - @javascript Scenario: I submit new unassigned issue Given I click link "New Issue" And I submit new issue "500 error on profile" - Given I click link "500 error on profile" Then I should see issue "500 error on profile" @javascript @@ -57,15 +55,6 @@ Feature: Project Issues Then I should see "Release 0.3" in issues And I should not see "Release 0.4" in issues - # TODO: find out solution for poltergeist/phantomjs or remove - # @javascript - # Scenario: I clear search - # Given I click link "All" - # And I fill in issue search with "Something" - # And I fill in issue search with "" - # Then I should see "Release 0.4" in issues - # And I should see "Release 0.3" in issues - @javascript Scenario: I create Issue with pre-selected milestone Given project "Shop" has milestone "v2.2" diff --git a/features/steps/project/project_issues.rb b/features/steps/project/project_issues.rb index cc0acb5b..2103aeb1 100644 --- a/features/steps/project/project_issues.rb +++ b/features/steps/project/project_issues.rb @@ -95,7 +95,7 @@ class ProjectIssues < Spinach::FeatureSteps end Then 'I should see selected milestone with title "v3.0"' do - issues_milestone_selector = "#milestone_id_chzn > a" + issues_milestone_selector = "#issue_milestone_id_chzn > a" page.find(issues_milestone_selector).should have_content("v3.0") end @@ -106,7 +106,7 @@ class ProjectIssues < Spinach::FeatureSteps end Then 'I should see first assignee from "Shop" as selected assignee' do - issues_assignee_selector = "#assignee_id_chzn > a" + issues_assignee_selector = "#issue_assignee_id_chzn > a" project = Project.find_by_name "Shop" assignee_name = project.users.first.name page.find(issues_assignee_selector).should have_content(assignee_name) diff --git a/spec/requests/issues_spec.rb b/spec/requests/issues_spec.rb index e5ecb959..08141085 100644 --- a/spec/requests/issues_spec.rb +++ b/spec/requests/issues_spec.rb @@ -11,7 +11,7 @@ describe "Issues" do project.add_access(user2, :read, :write) end - describe "Edit issue", js: true do + describe "Edit issue" do let!(:issue) do create(:issue, author: @user,