diff options
author | Zeger-Jan van de Weg <zegerjan@gitlab.com> | 2016-04-25 20:10:20 +0200 |
---|---|---|
committer | Zeger-Jan van de Weg <zegerjan@gitlab.com> | 2016-05-10 12:03:13 +0200 |
commit | dccf8a9fc8d4dde91942944f6b47387bfb13c380 (patch) | |
tree | 7574eea0adcc0cd46bf3d685fccec0d08c37b252 | |
parent | 4eb16290e4e95c0a9bcf3d01ecc8060d91eec021 (diff) | |
download | gitlab-ce-dccf8a9fc8d4dde91942944f6b47387bfb13c380.tar.gz |
Add tests on Awardables and Award Emoji
-rw-r--r-- | app/assets/javascripts/awards_handler.coffee | 20 | ||||
-rw-r--r-- | app/assets/javascripts/notes.js.coffee | 2 | ||||
-rw-r--r-- | app/controllers/projects/notes_controller.rb | 3 | ||||
-rw-r--r-- | app/helpers/issues_helper.rb | 11 | ||||
-rw-r--r-- | app/models/merge_request.rb | 1 | ||||
-rw-r--r-- | db/schema.rb | 19 | ||||
-rw-r--r-- | features/steps/project/issues/issues.rb | 10 | ||||
-rw-r--r-- | features/steps/project/merge_requests.rb | 10 | ||||
-rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 14 | ||||
-rw-r--r-- | spec/factories/award_emoji.rb | 11 | ||||
-rw-r--r-- | spec/features/issues_spec.rb | 2 | ||||
-rw-r--r-- | spec/features/notes_on_merge_requests_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/concerns/awardable_spec.rb | 49 | ||||
-rw-r--r-- | spec/models/concerns/issuable_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/user_spec.rb | 1 | ||||
-rw-r--r-- | spec/services/notes/create_service_spec.rb | 20 | ||||
-rw-r--r-- | spec/services/todo_service_spec.rb | 17 | ||||
-rw-r--r-- | spec/services/toggle_award_emoji_service_spec.rb | 39 |
18 files changed, 181 insertions, 56 deletions
diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 4c0a274b793..589caf011ed 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -4,21 +4,21 @@ class @AwardsHandler $(document) .off "click", ".js-add-award" - .on "click", ".js-add-award", (event) => - event.stopPropagation() - event.preventDefault() + .on "click", ".js-add-award", (e) => + e.stopPropagation() + e.preventDefault() - @showEmojiMenu $(event.currentTarget) + @showEmojiMenu $(e.currentTarget) - $("html").on 'click', (event) -> - if !$(event.target).closest(".emoji-menu").length + $("html").on 'click', (e) -> + if !$(e.target).closest(".emoji-menu").length if $(".emoji-menu").is(":visible") $('.js-add-award.is-active').removeClass 'is-active' $(".emoji-menu").removeClass "is-visible" $(document) .off "click", ".js-emoji-btn" - .on "click", ".js-emoji-btn", (e) => @handleClick(e) + .on "click", ".js-emoji-btn", @handleClick.bind(@) handleClick: (e) -> e.preventDefault() @@ -31,7 +31,8 @@ class @AwardsHandler else if $votesBlock.length is 0 $votesBlock = $addAwardBtn.closest('.js-awards-block') - $votesBlock.addClass 'js-awards-block-current' + @currentVoteBlock = $votesBlock + awardUrl = $votesBlock.data 'award-url' emoji = $emojiBtn .find(".icon") @@ -103,7 +104,6 @@ class @AwardsHandler emoji = @normilizeEmojiName(emoji) @postEmoji awardUrl, emoji, => @addAwardToEmojiBar(emoji) - $('.js-awards-block').removeClass 'js-awards-block-current' $(".emoji-menu").removeClass "is-visible" @@ -210,7 +210,7 @@ class @AwardsHandler callback.call() findEmojiIcon: (emoji) -> - $(".js-awards-block-current.awards > .js-emoji-btn [data-emoji='#{emoji}']") + @currentVoteBlock.find(".js-emoji-btn [data-emoji='#{emoji}']") scrollToAwards: -> $('body, html').animate({ diff --git a/app/assets/javascripts/notes.js.coffee b/app/assets/javascripts/notes.js.coffee index ae8c1f22e4c..74ae897b84a 100644 --- a/app/assets/javascripts/notes.js.coffee +++ b/app/assets/javascripts/notes.js.coffee @@ -150,7 +150,7 @@ class @Notes renderNote: (note) -> unless note.valid if note.award - flash = new Flash('You have already awarded this emoji, and it we\'ve removed it', 'alert') + flash = new Flash('You have already awarded this emoji, it has been removed', 'alert') flash.pinTo('.header-content') return diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 9000e0adf63..eb5137fe999 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -22,7 +22,7 @@ class Projects::NotesController < Projects::ApplicationController def create @note = Notes::CreateService.new(project, current_user, note_params).execute - @note = note.is_a?(AwardEmoji) ? @note.to_note_json : note_json(@note) + @note = @note.is_a?(AwardEmoji) ? @note.to_note_json : note_json(@note) respond_to do |format| format.json { render json: @note } @@ -63,7 +63,6 @@ class Projects::NotesController < Projects::ApplicationController def note @note ||= @project.notes.find(params[:id]) end - alias_method :awardable, :note def note_to_html(note) render_to_string( diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 38de0b442ca..ac6c6fb25bb 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -131,7 +131,7 @@ module IssuesHelper class: "icon emoji-icon emoji-#{unicode}", title: name, data: data - else + else # Emoji icons displayed separately, used for the awards already given # to an issue or merge request. content_tag :img, "", @@ -145,12 +145,9 @@ module IssuesHelper end def award_user_list(awards, current_user) - list = - awards.map do |award| - award.user == current_user ? "me" : award.user.name - end - - list.join(", ") + awards.map do |award| + award.user == current_user ? 'me' : award.user.name + end.join(', ') end def award_active_class(awards, current_user) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2cb3e8b0176..e410febdfff 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -36,7 +36,6 @@ class MergeRequest < ActiveRecord::Base include Referable include Sortable include Taskable - include Awardable belongs_to :target_project, foreign_key: :target_project_id, class_name: "Project" belongs_to :source_project, foreign_key: :source_project_id, class_name: "Project" diff --git a/db/schema.rb b/db/schema.rb index 354d7390a5b..7dc7b78ec6c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160416190505) do +ActiveRecord::Schema.define(version: 20160421130527) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -77,7 +77,9 @@ ActiveRecord::Schema.define(version: 20160416190505) do t.string "akismet_api_key" t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" - t.boolean "repository_checks_enabled", default: true + t.boolean "repository_checks_enabled", default: false + t.integer "metrics_packet_size", default: 1 + t.text "shared_runners_text" end create_table "audit_events", force: :cascade do |t| @@ -182,14 +184,21 @@ ActiveRecord::Schema.define(version: 20160416190505) do t.text "yaml_errors" t.datetime "committed_at" t.integer "gl_project_id" + t.string "status" + t.datetime "started_at" + t.datetime "finished_at" + t.integer "duration" end + add_index "ci_commits", ["gl_project_id", "sha"], name: "index_ci_commits_on_gl_project_id_and_sha", using: :btree + add_index "ci_commits", ["gl_project_id", "status"], name: "index_ci_commits_on_gl_project_id_and_status", using: :btree add_index "ci_commits", ["gl_project_id"], name: "index_ci_commits_on_gl_project_id", using: :btree add_index "ci_commits", ["project_id", "committed_at", "id"], name: "index_ci_commits_on_project_id_and_committed_at_and_id", using: :btree add_index "ci_commits", ["project_id", "committed_at"], name: "index_ci_commits_on_project_id_and_committed_at", using: :btree add_index "ci_commits", ["project_id", "sha"], name: "index_ci_commits_on_project_id_and_sha", using: :btree add_index "ci_commits", ["project_id"], name: "index_ci_commits_on_project_id", using: :btree add_index "ci_commits", ["sha"], name: "index_ci_commits_on_sha", using: :btree + add_index "ci_commits", ["status"], name: "index_ci_commits_on_status", using: :btree create_table "ci_events", force: :cascade do |t| t.integer "project_id" @@ -433,6 +442,7 @@ ActiveRecord::Schema.define(version: 20160416190505) do t.integer "moved_to_id" t.boolean "confidential", default: false t.datetime "deleted_at" + t.date "due_date" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -442,6 +452,7 @@ ActiveRecord::Schema.define(version: 20160416190505) do add_index "issues", ["created_at"], name: "index_issues_on_created_at", using: :btree add_index "issues", ["deleted_at"], name: "index_issues_on_deleted_at", using: :btree add_index "issues", ["description"], name: "index_issues_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"} + add_index "issues", ["due_date"], name: "index_issues_on_due_date", using: :btree add_index "issues", ["milestone_id"], name: "index_issues_on_milestone_id", using: :btree add_index "issues", ["project_id", "iid"], name: "index_issues_on_project_id_and_iid", unique: true, using: :btree add_index "issues", ["project_id"], name: "index_issues_on_project_id", using: :btree @@ -635,12 +646,14 @@ ActiveRecord::Schema.define(version: 20160416190505) do t.boolean "system", default: false, null: false t.text "st_diff" t.integer "updated_by_id" + t.boolean "is_award", default: false, null: false end add_index "notes", ["author_id"], name: "index_notes_on_author_id", using: :btree add_index "notes", ["commit_id"], name: "index_notes_on_commit_id", using: :btree add_index "notes", ["created_at", "id"], name: "index_notes_on_created_at_and_id", using: :btree add_index "notes", ["created_at"], name: "index_notes_on_created_at", using: :btree + add_index "notes", ["is_award"], name: "index_notes_on_is_award", using: :btree add_index "notes", ["line_code"], name: "index_notes_on_line_code", using: :btree add_index "notes", ["note"], name: "index_notes_on_note_trigram", using: :gin, opclasses: {"note"=>"gin_trgm_ops"} add_index "notes", ["noteable_id", "noteable_type"], name: "index_notes_on_noteable_id_and_noteable_type", using: :btree @@ -829,6 +842,7 @@ ActiveRecord::Schema.define(version: 20160416190505) do t.boolean "build_events", default: false, null: false t.string "category", default: "common", null: false t.boolean "default", default: false + t.boolean "wiki_page_events", default: true end add_index "services", ["category"], name: "index_services_on_category", using: :btree @@ -1023,6 +1037,7 @@ ActiveRecord::Schema.define(version: 20160416190505) do t.boolean "note_events", default: false, null: false t.boolean "enable_ssl_verification", default: true t.boolean "build_events", default: false, null: false + t.boolean "wiki_page_events", default: false, null: false end add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index fc12843ea5c..78ddaee8771 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -191,15 +191,15 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps end step 'issue "Release 0.4" have 2 upvotes and 1 downvote' do - issue = Issue.find_by(title: 'Release 0.4') - create_list(:upvote_note, 2, project: project, noteable: issue) - create(:downvote_note, project: project, noteable: issue) + awardable = Issue.find_by(title: 'Release 0.4') + create_list(:upvote, 2, project: project, awardable: awardable) + create(:downvote, project: project, awardable: awardable) end step 'issue "Tweet control" have 1 upvote and 2 downvotes' do issue = Issue.find_by(title: 'Tweet control') - create(:upvote_note, project: project, noteable: issue) - create_list(:downvote_note, 2, project: project, noteable: issue) + create(:upvote, project: project, noteable: issue) + create_list(:downvote, 2, project: project, noteable: issue) end step 'The list should be sorted by "Least popular"' do diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index 4f883fe7c27..1d619a11d23 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -179,14 +179,14 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'merge request "Bug NS-04" have 2 upvotes and 1 downvote' do merge_request = MergeRequest.find_by(title: 'Bug NS-04') - create_list(:upvote_note, 2, project: project, noteable: merge_request) - create(:downvote_note, project: project, noteable: merge_request) + create_list(:upvote, 2, project: project, awardable: merge_request) + create(:downvote, project: project, awardable: merge_request) end step 'merge request "Bug NS-06" have 1 upvote and 2 downvotes' do - merge_request = MergeRequest.find_by(title: 'Bug NS-06') - create(:upvote_note, project: project, noteable: merge_request) - create_list(:downvote_note, 2, project: project, noteable: merge_request) + awardable = MergeRequest.find_by(title: 'Bug NS-06') + create(:upvote, project: project, awardable: awardable) + create_list(:downvote, 2, project: project, awardable: awardable) end step 'The list should be sorted by "Least popular"' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index d6e4cd71ce6..f7cb7ca8a40 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -211,4 +211,18 @@ describe Projects::IssuesController do end end end + + describe 'POST #toggle_award_emoji' do + before do + sign_in(user) + project.team << [user, :developer] + end + + it "yields status code 200" do + post(:toggle_award_emoji, namespace_id: project.namespace.path, + project_id: project.path, id: issue.iid, name: "thumbsup") + + expect(response.status).to eq(200) + end + end end diff --git a/spec/factories/award_emoji.rb b/spec/factories/award_emoji.rb index a1173834b29..b09f8b0bc74 100644 --- a/spec/factories/award_emoji.rb +++ b/spec/factories/award_emoji.rb @@ -3,5 +3,16 @@ FactoryGirl.define do name "thumbsup" user awardable factory: :issue + + trait :thumbs_up + trait :upvote + + trait :thumbs_down do + name "thumbsdown" + end + + trait :downvote do + name "thumbsdown" + end end end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 1ce0024e93c..7da87c2d174 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -67,7 +67,7 @@ describe 'Issues', feature: true do describe 'Issue info' do it 'excludes award_emoji from comment count' do issue = create(:issue, author: @user, assignee: @user, project: project, title: 'foobar') - create(:upvote_note, noteable: issue) + create(:award_emoji, awardable: issue) visit namespace_project_issues_path(project.namespace, project, assignee_id: @user.id) diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 389812ff7e1..9e2bdd7f5bb 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -8,7 +8,7 @@ describe 'Comments', feature: true do it 'excludes award_emoji from comment count' do merge_request = create(:merge_request) project = merge_request.source_project - create(:upvote_note, noteable: merge_request, project: project) + create(:award_emoji, awardable: merge_request, project: project) login_as :admin visit namespace_project_merge_requests_path(project.namespace, project) @@ -146,7 +146,7 @@ describe 'Comments', feature: true do describe 'comment info' do it 'excludes award_emoji from comment count' do - create(:upvote_note, noteable: merge_request, project: project) + create(:award_emoji, awardable: merge_request, project: project) visit namespace_project_merge_request_path(project.namespace, project, merge_request) diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb new file mode 100644 index 00000000000..6851d068367 --- /dev/null +++ b/spec/models/concerns/awardable_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Issue, "Awardable" do + let!(:issue) { create(:issue) } + let!(:award_emoji) { create(:award_emoji, :downvote, awardable: issue) } + + describe "Associations" do + it { is_expected.to have_many(:award_emoji).dependent(:destroy) } + end + + describe "ClassMethods" do + let!(:issue2) { create(:issue) } + + before do + create(:award_emoji, awardable: issue2) + end + + it "orders on upvotes" do + expect(Issue.order_upvotes_desc.to_a).to eq [issue2, issue] + end + + it "orders on downvotes" do + expect(Issue.order_downvotes_desc.to_a).to eq [issue, issue2] + end + end + + describe "#upvotes" do + it "counts the number of upvotes" do + expect(issue.upvotes).to be 0 + end + end + + describe "#downvotes" do + it "counts the number of downvotes" do + expect(issue.downvotes).to be 1 + end + end + + describe "#toggle_award_emoji" do + it "adds an emoji if it isn't awarded yet" do + expect { issue.toggle_award_emoji("thumbsup", award_emoji.user) }.to change { AwardEmoji.count }.by 1 + end + + it "toggles already awarded emoji" do + + expect { issue.toggle_award_emoji("thumbsdown", award_emoji.user) }.to change { AwardEmoji.count }.by -1 + end + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index d5435916ea1..86ad9de883f 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -12,6 +12,10 @@ describe Issue, "Issuable" do it { is_expected.to have_many(:todos).dependent(:destroy) } end + describe 'Included modules' do + it { is_expected.to include_module(Awardable) } + end + describe "Validation" do before do allow(subject).to receive(:set_iid).and_return(false) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8b2fb77e28e..de1a233dfff 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -93,6 +93,7 @@ describe User, models: true do it { is_expected.to have_one(:abuse_report) } it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } + it { is_expected.to have_many(:award_emoji).dependent(:destroy) } end describe 'validations' do diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index ff23f13e1cb..93e8a7094e2 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -14,7 +14,7 @@ describe Notes::CreateService, services: true do noteable_type: 'Issue', noteable_id: issue.id } - + @note = Notes::CreateService.new(project, user, opts).execute end @@ -28,18 +28,16 @@ describe Notes::CreateService, services: true do project.team << [user, :master] end - it "creates emoji note" do + it "creates an award emoji" do opts = { note: ':smile: ', noteable_type: 'Issue', noteable_id: issue.id } + note = Notes::CreateService.new(project, user, opts).execute - @note = Notes::CreateService.new(project, user, opts).execute - - expect(@note).to be_valid - expect(@note.note).to eq('smile') - expect(@note.is_award).to be_truthy + expect(note).to be_valid + expect(note.name).to eq('smile') end it "creates regular note if emoji name is invalid" do @@ -48,12 +46,10 @@ describe Notes::CreateService, services: true do noteable_type: 'Issue', noteable_id: issue.id } + note = Notes::CreateService.new(project, user, opts).execute - @note = Notes::CreateService.new(project, user, opts).execute - - expect(@note).to be_valid - expect(@note.note).to eq(opts[:note]) - expect(@note.is_award).to be_falsy + expect(note).to be_valid + expect(note.note).to eq(opts[:note]) end end end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 82b7fbfa816..455b19c89cc 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -137,7 +137,6 @@ describe TodoService, services: true do let(:note_on_commit) { create(:note_on_commit, project: project, author: john_doe, note: mentions) } let(:note_on_confidential_issue) { create(:note_on_issue, noteable: confidential_issue, project: project, note: mentions) } let(:note_on_project_snippet) { create(:note_on_project_snippet, project: project, author: john_doe, note: mentions) } - let(:award_note) { create(:note, :award, project: project, noteable: issue, author: john_doe, note: 'thumbsup') } let(:system_note) { create(:system_note, project: project, noteable: issue) } it 'mark related pending todos to the noteable for the note author as done' do @@ -150,13 +149,6 @@ describe TodoService, services: true do expect(second_todo.reload).to be_done end - it 'mark related pending todos to the noteable for the award note author as done' do - service.new_note(award_note, john_doe) - - expect(first_todo.reload).to be_done - expect(second_todo.reload).to be_done - end - it 'does not mark related pending todos it is a system note' do service.new_note(system_note, john_doe) @@ -286,6 +278,15 @@ describe TodoService, services: true do expect(second_todo.reload).to be_done end end + + describe '#new_award_emoji' do + it 'marks related pending todos to the target for the user as done' do + todo = create(:todo, user: john_doe, project: project, target: mr_assigned, author: author) + service.new_award_emoji(mr_assigned, john_doe) + + expect(todo.reload).to be_done + end + end end def should_create_todo(attributes = {}) diff --git a/spec/services/toggle_award_emoji_service_spec.rb b/spec/services/toggle_award_emoji_service_spec.rb new file mode 100644 index 00000000000..3d2f497fdec --- /dev/null +++ b/spec/services/toggle_award_emoji_service_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe ToggleAwardEmoji, services: true do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + + before do + project.team << [user, :master] + end + + describe '#execute' do + it 'removes related todos' do + expect_any_instance_of(TodoService).to receive(:new_award_emoji).with(issue, user) + + ToggleAwardEmojiService.new(project, user).execute(issue, "thumbsdown") + end + + it 'normalizes the emoji name' do + expect(issue).to receive(:toggle_award_emoji).with("thumbsup", user) + + ToggleAwardEmojiService.new(project, user).execute(issue, ":+1:") + end + + context 'when the emoji is set' do + it 'removes the emoji' do + create(:award_emoji, awardable: issue, user: user) + + expect { ToggleAwardEmojiService.new(project, user).execute(issue, ":+1:") }.to change { AwardEmoji.count }.by(-1) + end + end + + context 'when the award is not set yet' do + it 'awards the emoji' do + expect { ToggleAwardEmojiService.new(project, user).execute(issue, ":+1:") }.to change { AwardEmoji.count }.by(1) + end + end + end +end |