From 1b1e77c728e575a110e204e142e81bdff2737536 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 27 Jun 2012 21:20:35 +0300 Subject: [PATCH 1/3] Issue Labels: Edit, show, index + filter --- Gemfile | 2 +- Gemfile.lock | 6 +-- app/assets/stylesheets/gitlab_bootstrap.scss | 8 ++++ app/controllers/issues_controller.rb | 1 + app/helpers/issues_helper.rb | 4 ++ app/models/issue.rb | 2 + app/views/issues/_form.html.haml | 6 +++ app/views/issues/_show.html.haml | 5 ++ app/views/issues/index.html.haml | 46 ++++++++++--------- app/views/issues/show.html.haml | 8 ++-- features/projects/issues/issues.feature | 12 +++++ .../step_definitions/project_issues_steps.rb | 22 +++++++++ 12 files changed, 93 insertions(+), 29 deletions(-) create mode 100644 features/step_definitions/project_issues_steps.rb diff --git a/Gemfile b/Gemfile index 44bdb392..4c977dde 100644 --- a/Gemfile +++ b/Gemfile @@ -29,7 +29,7 @@ gem "thin" gem "unicorn" gem "git" gem "acts_as_list" -gem "acts-as-taggable-on", "~> 2.1.0" +gem "acts-as-taggable-on", "2.3.1" gem "drapper" gem "resque", "~> 1.20.0" gem "httparty" diff --git a/Gemfile.lock b/Gemfile.lock index 8265641f..d50a8d1e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -89,8 +89,8 @@ GEM activesupport (3.2.5) i18n (~> 0.6) multi_json (~> 1.0) - acts-as-taggable-on (2.1.1) - rails + acts-as-taggable-on (2.3.1) + rails (~> 3.0) acts_as_list (0.1.6) addressable (2.2.8) ansi (1.4.2) @@ -351,7 +351,7 @@ PLATFORMS ruby DEPENDENCIES - acts-as-taggable-on (~> 2.1.0) + acts-as-taggable-on (= 2.3.1) acts_as_list annotate! autotest diff --git a/app/assets/stylesheets/gitlab_bootstrap.scss b/app/assets/stylesheets/gitlab_bootstrap.scss index a347eeb2..c4491b3c 100644 --- a/app/assets/stylesheets/gitlab_bootstrap.scss +++ b/app/assets/stylesheets/gitlab_bootstrap.scss @@ -177,6 +177,14 @@ a:focus { &.label-important { background-color: #B94A48; } + + &.label-issue { + background-color: #eee; + border: 1px solid #ccc; + padding:4px 6px; + color:#444; + text-shadow:0 0 1px #fff; + } } .nav-tabs > li > a, .nav-pills > li > a { diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 48e8d72a..35a30ca0 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -139,6 +139,7 @@ class IssuesController < ApplicationController @issues = @issues.where(:assignee_id => params[:assignee_id]) if params[:assignee_id].present? @issues = @issues.where(:milestone_id => params[:milestone_id]) if params[:milestone_id].present? + @issues = @issues.tagged_with(params[:label_name]) if params[:label_name].present? @issues = @issues.includes(:author, :project).order("critical, updated_at") @issues end diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 31d862ef..148048a9 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -33,4 +33,8 @@ module IssuesHelper classes << " today" if issue.today? classes end + + def issue_tags + @project.issues.tag_counts_on(:labels).map(&:name) + end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 40e14dac..c961bbe2 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -1,6 +1,8 @@ class Issue < ActiveRecord::Base include Upvote + acts_as_taggable_on :labels + belongs_to :project belongs_to :milestone belongs_to :author, :class_name => "User" diff --git a/app/views/issues/_form.html.haml b/app/views/issues/_form.html.haml index 444fbd76..94a621c2 100644 --- a/app/views/issues/_form.html.haml +++ b/app/views/issues/_form.html.haml @@ -35,6 +35,12 @@ = f.text_area :description, :maxlength => 2000, :class => "xxlarge", :rows => 14 %p.hint Markdown is enabled. + .clearfix + = f.label :label_list, "Labels" + .input + = f.text_field :label_list, :maxlength => 2000, :class => "xxlarge" + %p.hint Separate with comma. + .actions - if @issue.new_record? = f.submit 'Submit new issue', :class => "primary btn" diff --git a/app/views/issues/_show.html.haml b/app/views/issues/_show.html.haml index fe9b6b37..797d2387 100644 --- a/app/views/issues/_show.html.haml +++ b/app/views/issues/_show.html.haml @@ -2,6 +2,11 @@ .list_legend .icon .right + - issue.labels.each do |label| + %span.label.label-issue + %i.icon-tag + = label.name +   - if issue.notes.any? %span.btn.small.disabled.padded %i.icon-comment diff --git a/app/views/issues/index.html.haml b/app/views/issues/index.html.haml index e49c9513..ebe432bc 100644 --- a/app/views/issues/index.html.haml +++ b/app/views/issues/index.html.haml @@ -31,28 +31,29 @@ %div#issues-table-holder.ui-box .title - .row - .span4 - %ul.nav.nav-pills.left - %li{:class => ("active" if (params[:f] == "0" || !params[:f]))} - = link_to project_issues_path(@project, :f => 0, :milestone_id => params[:milestone_id]) do - Open - %li{:class => ("active" if params[:f] == "2")} - = link_to project_issues_path(@project, :f => 2, :milestone_id => params[:milestone_id]) do - Closed - %li{:class => ("active" if params[:f] == "3")} - = link_to project_issues_path(@project, :f => 3, :milestone_id => params[:milestone_id]) do - To Me - %li{:class => ("active" if params[:f] == "1")} - = link_to project_issues_path(@project, :f => 1, :milestone_id => params[:milestone_id]) do - All - - .span6.right - = form_tag project_issues_path(@project), :method => :get, :class => :right do - = select_tag(:assignee_id, options_from_collection_for_select(@project.users.all, "id", "name", params[:assignee_id]), :prompt => "Assignee") - = select_tag(:milestone_id, options_from_collection_for_select(@project.milestones.order("id desc").all, "id", "title", params[:milestone_id]), :prompt => "Milestone") - = hidden_field_tag :f, params[:f] + .left + %ul.nav.nav-pills.left + %li{:class => ("active" if (params[:f] == "0" || !params[:f]))} + = link_to project_issues_path(@project, :f => 0, :milestone_id => params[:milestone_id]) do + Open + %li{:class => ("active" if params[:f] == "2")} + = link_to project_issues_path(@project, :f => 2, :milestone_id => params[:milestone_id]) do + Closed + %li{:class => ("active" if params[:f] == "3")} + = link_to project_issues_path(@project, :f => 3, :milestone_id => params[:milestone_id]) do + To Me + %li{:class => ("active" if params[:f] == "1")} + = link_to project_issues_path(@project, :f => 1, :milestone_id => params[:milestone_id]) do + All + .right + = form_tag project_issues_path(@project), :method => :get, :class => :right do + = select_tag(:label_name, options_for_select(issue_tags, params[:label_name]), :prompt => "Labels") + = select_tag(:assignee_id, options_from_collection_for_select(@project.users.all, "id", "name", params[:assignee_id]), :prompt => "Assignee") + = select_tag(:milestone_id, options_from_collection_for_select(@project.milestones.order("id desc").all, "id", "title", params[:milestone_id]), :prompt => "Milestone") + = hidden_field_tag :f, params[:f] + .clearfix + %ul#issues-table.unstyled.issues_table = render "issues" @@ -60,9 +61,10 @@ $(function(){ initIssuesSearch(); setSortable(); + $("#label_name").chosen(); $("#assignee_id").chosen(); $("#milestone_id").chosen(); - $("#milestone_id, #assignee_id").live("change", function(){ + $("#milestone_id, #assignee_id, #label_name").live("change", function(){ $(this).closest("form").submit(); }); }) diff --git a/app/views/issues/show.html.haml b/app/views/issues/show.html.haml index 1bb4e04d..cd7ad57a 100644 --- a/app/views/issues/show.html.haml +++ b/app/views/issues/show.html.haml @@ -51,9 +51,11 @@ = truncate(milestone.title, :length => 20) .right - - if @issue.critical - %span.label.label-important - Critical + - @issue.labels.each do |label| + %span.label.label-issue + %i.icon-tag + = label.name +   - if @issue.description.present? .bottom_box_content diff --git a/features/projects/issues/issues.feature b/features/projects/issues/issues.feature index e69de29b..0ca0792d 100644 --- a/features/projects/issues/issues.feature +++ b/features/projects/issues/issues.feature @@ -0,0 +1,12 @@ +Feature: Issues + Background: + Given I signin as a user + And I own project "Shop" + And project "Shop" have "Release 0.4" open issue + And project "Shop" have "Release 0.3" closed issue + And I visit project "Shop" issues page + + Scenario: I should see open issues + Given I should see "Release 0.4" open issue + And I should not see "Release 0.3" closed issue + diff --git a/features/step_definitions/project_issues_steps.rb b/features/step_definitions/project_issues_steps.rb new file mode 100644 index 00000000..e83c0e7f --- /dev/null +++ b/features/step_definitions/project_issues_steps.rb @@ -0,0 +1,22 @@ +Given /^project "(.*?)" have "(.*?)" open issue$/ do |arg1, arg2| + project = Project.find_by_name(arg1) + Factory.create(:issue, :title => arg2, :project => project, :author => project.users.first) +end + +Given /^project "(.*?)" have "(.*?)" closed issue$/ do |arg1, arg2| + project = Project.find_by_name(arg1) + Factory.create(:issue, :title => arg2, :project => project, :author => project.users.first, :closed => true) +end + +Given /^I visit project "(.*?)" issues page$/ do |arg1| + visit project_issues_path(Project.find_by_name(arg1)) +end + +Given /^I should see "(.*?)" open issue$/ do |arg1| + page.should have_content arg1 +end + +Given /^I should not see "(.*?)" closed issue$/ do |arg1| + page.should_not have_content arg1 +end + From 50fdb2e7dffd80265bc604ec8f9071e12f99af30 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Wed, 27 Jun 2012 21:30:35 +0300 Subject: [PATCH 2/3] Remove critical status from Issue. Move sort instead --- app/assets/stylesheets/common.scss | 12 ------- app/controllers/issues_controller.rb | 2 +- app/controllers/merge_requests_controller.rb | 2 +- app/helpers/issues_helper.rb | 1 - app/models/issue.rb | 3 -- app/models/project.rb | 2 +- app/views/dashboard/issues.html.haml | 9 ------ app/views/issues/_form.html.haml | 32 +++++++++---------- app/views/issues/_issues.html.haml | 5 +-- app/views/issues/index.html.haml | 15 --------- ...120627145613_remove_critical_from_issue.rb | 9 ++++++ db/schema.rb | 3 +- 12 files changed, 30 insertions(+), 65 deletions(-) create mode 100644 db/migrate/20120627145613_remove_critical_from_issue.rb diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index b8802527..8b76fe5a 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -622,10 +622,6 @@ li.note { margin-right:5px; margin-top: 2px; @include border-radius(4px); - &.critical { - background: #EAA; - border:1px solid #B88; - } &.today{ background: #ADA; border:1px solid #8B8; @@ -664,14 +660,6 @@ li.note { } } - &.critical { - background: #FEE; - border-color:#ECC; - .icon { - background: #EAA; - border:1px solid #B88; - } - } &.today{ background: #EFE; border-color:#CEC; diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 35a30ca0..e095c4dd 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -140,7 +140,7 @@ class IssuesController < ApplicationController @issues = @issues.where(:assignee_id => params[:assignee_id]) if params[:assignee_id].present? @issues = @issues.where(:milestone_id => params[:milestone_id]) if params[:milestone_id].present? @issues = @issues.tagged_with(params[:label_name]) if params[:label_name].present? - @issues = @issues.includes(:author, :project).order("critical, updated_at") + @issues = @issues.includes(:author, :project).order("updated_at") @issues end end diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index b0d122b5..b8643dce 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -30,7 +30,7 @@ class MergeRequestsController < ApplicationController else @merge_requests.opened end.page(params[:page]).per(20) - @merge_requests = @merge_requests.includes(:author, :project).order("created_at desc") + @merge_requests = @merge_requests.includes(:author, :project).order("closed, created_at desc") end def show diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 148048a9..0982aacc 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -28,7 +28,6 @@ module IssuesHelper def issue_css_classes issue classes = "issue" - classes << " critical" if issue.critical classes << " closed" if issue.closed classes << " today" if issue.today? classes diff --git a/app/models/issue.rb b/app/models/issue.rb index c961bbe2..7f935900 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -33,9 +33,6 @@ class Issue < ActiveRecord::Base validates :description, :length => { :within => 0..2000 } - scope :critical, where(:critical => true) - scope :non_critical, where(:critical => false) - scope :opened, where(:closed => false) scope :closed, where(:closed => true) scope :assigned, lambda { |u| where(:assignee_id => u.id)} diff --git a/app/models/project.rb b/app/models/project.rb index 9a4da197..5611eb60 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -13,7 +13,7 @@ class Project < ActiveRecord::Base has_many :users, :through => :users_projects has_many :events, :dependent => :destroy has_many :merge_requests, :dependent => :destroy - has_many :issues, :dependent => :destroy, :order => "position" + has_many :issues, :dependent => :destroy, :order => "closed, position" has_many :milestones, :dependent => :destroy has_many :users_projects, :dependent => :destroy has_many :notes, :dependent => :destroy diff --git a/app/views/dashboard/issues.html.haml b/app/views/dashboard/issues.html.haml index c9cc048c..6f623b21 100644 --- a/app/views/dashboard/issues.html.haml +++ b/app/views/dashboard/issues.html.haml @@ -3,15 +3,6 @@ %small (assigned to you) %small.right #{@issues.total_count} issues -%br -.issues_legend - .list_legend - .icon.critical - .text Critical - - .list_legend - .icon.today - .text Today .clearfix - if @issues.any? - @issues.group_by(&:project).each do |group| diff --git a/app/views/issues/_form.html.haml b/app/views/issues/_form.html.haml index 94a621c2..4f6f8396 100644 --- a/app/views/issues/_form.html.haml +++ b/app/views/issues/_form.html.haml @@ -9,37 +9,37 @@ .issue_form_box .issue_title .clearfix - = f.label :title, "Issue Subject *" + = f.label :title do + %strong= "Subject *" .input = f.text_field :title, :maxlength => 255, :class => "xxlarge" .issue_middle_block .issue_assignee - = f.label :assignee_id, "Assign to" - .input= f.select(:assignee_id, @project.users.all.collect {|p| [ p.name, p.id ] }, { :include_blank => "Assign to user" }) + = f.label :assignee_id do + %i.icon-user + Assign to + .input= f.select(:assignee_id, @project.users.all.collect {|p| [ p.name, p.id ] }, { :include_blank => "Select a user" }) .issue_milestone - = f.label :milestone_id + = f.label :milestone_id do + %i.icon-time + Milestone .input= f.select(:milestone_id, @project.milestones.active.all.collect {|p| [ p.title, p.id ] }, { :include_blank => "Select milestone" }) .issue_description .clearfix - = f.label :critical, "Critical" - .input= f.check_box :critical + = f.label :label_list do + %i.icon-tag + Labels + .input + = f.text_field :label_list, :maxlength => 2000, :class => "xxlarge" + %p.hint Separate with comma. - - unless @issue.new_record? - .clearfix - = f.label :closed - .input= f.check_box :closed .clearfix - = f.label :description, "Issue Details" + = f.label :description, "Details" .input = f.text_area :description, :maxlength => 2000, :class => "xxlarge", :rows => 14 %p.hint Markdown is enabled. - .clearfix - = f.label :label_list, "Labels" - .input - = f.text_field :label_list, :maxlength => 2000, :class => "xxlarge" - %p.hint Separate with comma. .actions - if @issue.new_record? diff --git a/app/views/issues/_issues.html.haml b/app/views/issues/_issues.html.haml index e1d0be08..a20df176 100644 --- a/app/views/issues/_issues.html.haml +++ b/app/views/issues/_issues.html.haml @@ -1,7 +1,4 @@ -- @issues.select(&:critical).each do |issue| - = render(:partial => 'issues/show', :locals => {:issue => issue}) - -- @issues.reject(&:critical).each do |issue| +- @issues.each do |issue| = render(:partial => 'issues/show', :locals => {:issue => issue}) - if @issues.present? diff --git a/app/views/issues/index.html.haml b/app/views/issues/index.html.haml index ebe432bc..e3480f6a 100644 --- a/app/views/issues/index.html.haml +++ b/app/views/issues/index.html.haml @@ -13,22 +13,7 @@ = hidden_field_tag :status, params[:f] = search_field_tag :issue_search, nil, { :placeholder => 'Search', :class => 'issue_search span3 right neib' } - %br - - .issues_legend - .list_legend - .icon.today - .text Today - - .list_legend - .icon.critical - .text Critical - - .list_legend - .icon.closed - .text Closed .clearfix - %div#issues-table-holder.ui-box .title .left diff --git a/db/migrate/20120627145613_remove_critical_from_issue.rb b/db/migrate/20120627145613_remove_critical_from_issue.rb new file mode 100644 index 00000000..f8d07971 --- /dev/null +++ b/db/migrate/20120627145613_remove_critical_from_issue.rb @@ -0,0 +1,9 @@ +class RemoveCriticalFromIssue < ActiveRecord::Migration + def up + remove_column :issues, :critical + end + + def down + add_column :issues, :critical, :boolean, :null => true, :default => false + end +end diff --git a/db/schema.rb b/db/schema.rb index 59098d7c..7ce89c6b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20120413135904) do +ActiveRecord::Schema.define(:version => 20120627145613) do create_table "events", :force => true do |t| t.string "target_type" @@ -34,7 +34,6 @@ ActiveRecord::Schema.define(:version => 20120413135904) do t.datetime "updated_at", :null => false t.boolean "closed", :default => false, :null => false t.integer "position", :default => 0 - t.boolean "critical", :default => false, :null => false t.string "branch_name" t.text "description" t.integer "milestone_id" From 5f356d69284850603893b8a82141e44d27eec89e Mon Sep 17 00:00:00 2001 From: randx Date: Wed, 27 Jun 2012 23:13:44 +0300 Subject: [PATCH 3/3] Issues tags: refactoring --- app/assets/javascripts/issues.js | 15 +++++++++++++++ app/controllers/issues_controller.rb | 19 +++++++++++++++---- app/views/issues/index.html.haml | 25 +++++++++---------------- 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/issues.js b/app/assets/javascripts/issues.js index 5f49567f..69ed4161 100644 --- a/app/assets/javascripts/issues.js +++ b/app/assets/javascripts/issues.js @@ -61,3 +61,18 @@ function initIssuesSearch() { $(this).closest('tr').fadeOut(); updatePage(); }); } + +/** + * Init issues page + * + */ +function issuesPage(){ + initIssuesSearch(); + setSortable(); + $("#label_name").chosen(); + $("#assignee_id").chosen(); + $("#milestone_id").chosen(); + $("#milestone_id, #assignee_id, #label_name").on("change", function(){ + $(this).closest("form").submit(); + }); +} diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index e095c4dd..c36258c8 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -3,6 +3,8 @@ class IssuesController < ApplicationController before_filter :project before_filter :module_enabled before_filter :issue, :only => [:edit, :update, :destroy, :show] + helper_method :issues_filter + layout "project" # Authorize @@ -130,10 +132,10 @@ class IssuesController < ApplicationController end def issues_filtered - @issues = case params[:f].to_i - when 1 then @project.issues - when 2 then @project.issues.closed - when 3 then @project.issues.opened.assigned(current_user) + @issues = case params[:f] + when issues_filter[:all] then @project.issues + when issues_filter[:closed] then @project.issues.closed + when issues_filter[:to_me] then @project.issues.opened.assigned(current_user) else @project.issues.opened end @@ -143,4 +145,13 @@ class IssuesController < ApplicationController @issues = @issues.includes(:author, :project).order("updated_at") @issues end + + def issues_filter + { + all: "1", + closed: "2", + to_me: "3", + open: "0" + } + end end diff --git a/app/views/issues/index.html.haml b/app/views/issues/index.html.haml index e3480f6a..b7e98ec8 100644 --- a/app/views/issues/index.html.haml +++ b/app/views/issues/index.html.haml @@ -18,17 +18,17 @@ .title .left %ul.nav.nav-pills.left - %li{:class => ("active" if (params[:f] == "0" || !params[:f]))} - = link_to project_issues_path(@project, :f => 0, :milestone_id => params[:milestone_id]) do + %li{:class => ("active" if (params[:f] == issues_filter[:open] || !params[:f]))} + = link_to project_issues_path(@project, :f => issues_filter[:open], :milestone_id => params[:milestone_id]) do Open - %li{:class => ("active" if params[:f] == "2")} - = link_to project_issues_path(@project, :f => 2, :milestone_id => params[:milestone_id]) do + %li{:class => ("active" if params[:f] == issues_filter[:closed])} + = link_to project_issues_path(@project, :f => issues_filter[:closed], :milestone_id => params[:milestone_id]) do Closed - %li{:class => ("active" if params[:f] == "3")} - = link_to project_issues_path(@project, :f => 3, :milestone_id => params[:milestone_id]) do + %li{:class => ("active" if params[:f] == issues_filter[:to_me])} + = link_to project_issues_path(@project, :f => issues_filter[:to_me], :milestone_id => params[:milestone_id]) do To Me - %li{:class => ("active" if params[:f] == "1")} - = link_to project_issues_path(@project, :f => 1, :milestone_id => params[:milestone_id]) do + %li{:class => ("active" if params[:f] == issues_filter[:all])} + = link_to project_issues_path(@project, :f => issues_filter[:all], :milestone_id => params[:milestone_id]) do All .right @@ -44,14 +44,7 @@ :javascript $(function(){ - initIssuesSearch(); - setSortable(); - $("#label_name").chosen(); - $("#assignee_id").chosen(); - $("#milestone_id").chosen(); - $("#milestone_id, #assignee_id, #label_name").live("change", function(){ - $(this).closest("form").submit(); - }); + issuesPage(); }) function setSortable(){