diff --git a/app/assets/stylesheets/errbit.css.erb b/app/assets/stylesheets/errbit.css.erb index be9840d..0688503 100644 --- a/app/assets/stylesheets/errbit.css.erb +++ b/app/assets/stylesheets/errbit.css.erb @@ -604,6 +604,11 @@ div.notification_service.nested .label_radio input { position: absolute; left: -9999px; } +.button-icon { + float: left; + margin: 5px 0 0 6px; +} + div.issue_tracker.nested label.r_on, div.issue_tracker.nested label.r_on:hover, div.notification_service.nested label.r_on, div.notification_service.nested label.r_on:hover { color: #191919; } diff --git a/app/controllers/apps_controller.rb b/app/controllers/apps_controller.rb index 0973fe5..beeea5e 100644 --- a/app/controllers/apps_controller.rb +++ b/app/controllers/apps_controller.rb @@ -12,7 +12,7 @@ class AppsController < ApplicationController } expose(:apps) { - app_scope.all.sort.to_a + app_scope.all.sort.map { |app| AppDecorator.new(app) } } expose(:app, ancestor: :app_scope, attributes: :app_params) diff --git a/app/controllers/problems_controller.rb b/app/controllers/problems_controller.rb index 554ee26..eae854b 100644 --- a/app/controllers/problems_controller.rb +++ b/app/controllers/problems_controller.rb @@ -12,28 +12,23 @@ class ProblemsController < ApplicationController :resolve_several, :unresolve_several, :unmerge_several ] + expose(:app_scope) { + apps = current_user.admin? ? App.all : current_user.apps + params[:app_id] ? apps.where(:_id => params[:app_id]) : apps + } + expose(:app) { - if current_user.admin? - App.find(params[:app_id]) - else - current_user.apps.find(params[:app_id]) - end + AppDecorator.new app_scope.find(params[:app_id]) } expose(:problem) { app.problems.find(params[:id]) } - expose(:all_errs) { params[:all_errs] } - expose(:app_scope) { - apps = current_user.admin? ? App.all : current_user.apps - params[:app_id] ? apps.where(:_id => params[:app_id]) : apps - } - expose(:params_environement) { params[:environment] } diff --git a/app/decorators/issue_tracker_decorator.rb b/app/decorators/issue_tracker_decorator.rb index 5f8e0a9..277f34d 100644 --- a/app/decorators/issue_tracker_decorator.rb +++ b/app/decorators/issue_tracker_decorator.rb @@ -1,46 +1,8 @@ +# Decorates an instance of IssueTracker class IssueTrackerDecorator < Draper::Decorator - - def initialize(object, key) - @object = object - @key = key - end - attr_reader :key - delegate_all - def issue_trackers - ErrbitPlugin::Registry.issue_trackers.each do |key, object| - yield IssueTrackerDecorator.new(object, key) - end - end - - def icons - tracker_icons = object.icons - return unless tracker_icons - - tracker_icons.reduce({}) do |c, (k,v)| - c[k] = "data:#{v[0]};base64,#{Base64.encode64(v[1])}"; c - end - end - - def note - object.note.html_safe + def type + @type ||= IssueTrackerTypeDecorator.new(object.tracker.class) end - - def fields - object.fields.each do |field, field_info| - yield IssueTrackerFieldDecorator.new(field, field_info) - end - end - - def params_class(tracker) - [chosen?(tracker), label].join(" ").strip - end - - private - - def chosen?(issue_tracker) - key == issue_tracker.type_tracker.to_s ? 'chosen' : '' - end - end diff --git a/app/decorators/issue_tracker_type_decorator.rb b/app/decorators/issue_tracker_type_decorator.rb new file mode 100644 index 0000000..194a9b7 --- /dev/null +++ b/app/decorators/issue_tracker_type_decorator.rb @@ -0,0 +1,30 @@ +# Decorates an IssueTracker class +class IssueTrackerTypeDecorator < Draper::Decorator + delegate_all + + # return hash of icons as data URIs + def icons + return unless object.icons + + object.icons.reduce({}) do |c, (k,v)| + c[k] = "data:#{v[0]};base64,#{Base64.encode64(v[1])}"; c + end + end + + # class name for tracker type form fields + # + # 'chosen github' or 'bitbucket' for example + def params_class(tracker) + [object.label == tracker.type_tracker ? 'chosen' : '', label].join(" ").strip + end + + def note + object.note.html_safe + end + + def fields + object.fields.each do |field, field_info| + yield IssueTrackerFieldDecorator.new(field, field_info) + end + end +end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 49f4948..7f50d8b 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -69,6 +69,12 @@ module ApplicationHelper collection.to_a[head_size..-1].to_a end + def issue_tracker_types + ErrbitPlugin::Registry.issue_trackers.map do |_, object| + IssueTrackerTypeDecorator.new(object) + end + end + private def total_from_tallies(tallies) tallies.values.inject(0) {|sum, n| sum + n} diff --git a/app/models/issue_tracker.rb b/app/models/issue_tracker.rb index 09cefba..b146748 100644 --- a/app/models/issue_tracker.rb +++ b/app/models/issue_tracker.rb @@ -14,7 +14,10 @@ class IssueTracker begin klass = ErrbitPlugin::Registry.issue_trackers[self.type_tracker] || ErrbitPlugin::NoneIssueTracker # TODO: we need to find out a better way to pass those config to the issue tracker - klass.new(options.merge(github_repo: app.github_repo, bitbucket_repo: app.bitbucket_repo)) + klass.new(options.merge( + github_repo: app.try(:github_repo), + bitbucket_repo: app.try(:bitbucket_repo) + )) end end diff --git a/app/views/apps/_issue_tracker_fields.html.haml b/app/views/apps/_issue_tracker_fields.html.haml index 735130a..98cf137 100644 --- a/app/views/apps/_issue_tracker_fields.html.haml +++ b/app/views/apps/_issue_tracker_fields.html.haml @@ -3,16 +3,17 @@ = f.fields_for :issue_tracker do |issue_tracker_form| %div.issue_tracker.nested %div.choose - - issue_tracker_form.object.issue_trackers do |tracker| - = issue_tracker_form.label :type_tracker, :class => "label_radio #{tracker.label}", :value => tracker.key do - %img.icon{"src" => tracker.icons[:inactive], "data-icon" => tracker.icons} - = issue_tracker_form.radio_button :type_tracker, tracker.key, 'data-section' => tracker.label - = tracker.label + - issue_tracker_types.each do |tracker_type| + = issue_tracker_form.label :type_tracker, :class => "label_radio #{tracker_type.label}", :value => tracker_type.label do + - icon = tracker_type == issue_tracker_form.object.type ? :create : :inactive + %img.icon{"src" => tracker_type.icons[icon], "data-icon" => tracker_type.icons} + = issue_tracker_form.radio_button :type_tracker, tracker_type.label, 'data-section' => tracker_type.label + = tracker_type.label - - issue_tracker_form.object.issue_trackers do |tracker| - %div.tracker_params{:class => tracker.params_class(issue_tracker_form.object)} + - issue_tracker_types.each do |tracker_type| + %div.tracker_params{:class => tracker_type.params_class(issue_tracker_form.object)} = issue_tracker_form.fields_for(:options) do |options_form| - %p= tracker.note - - tracker.fields do |field| + %p= tracker_type.note + - tracker_type.fields do |field| = options_form.label field.key, field.label = field.input(options_form, issue_tracker_form.object) diff --git a/app/views/apps/index.html.haml b/app/views/apps/index.html.haml index 0278076..bc0f59d 100644 --- a/app/views/apps/index.html.haml +++ b/app/views/apps/index.html.haml @@ -36,7 +36,7 @@ - if any_issue_trackers? %td.issue_tracker - if app.issue_tracker_configured? - - tracker_img = image_tag("#{app.issue_tracker.type_tracker}_goto.png") + - tracker_img = image_tag(app.issue_tracker.type.icons[:goto]) - if app.issue_tracker.url = link_to( tracker_img, app.issue_tracker.url ) - else diff --git a/app/views/problems/_issue_tracker_links.html.haml b/app/views/problems/_issue_tracker_links.html.haml index ce765d5..850368a 100644 --- a/app/views/problems/_issue_tracker_links.html.haml +++ b/app/views/problems/_issue_tracker_links.html.haml @@ -1,9 +1,17 @@ - if app.issue_tracker_configured? + - tracker = app.issue_tracker + - tracker_type = tracker.type - if problem.issue_link.present? - %span= link_to 'go to issue', problem.issue_link, :class => "#{problem.issue_type}_goto goto-issue" + %span + %img.button-icon{"src" => tracker_type.icons[:create]} + = link_to 'go to issue', problem.issue_link, :class => "goto-issue" = link_to 'unlink issue', unlink_issue_app_problem_path(app, problem), :method => :delete, :data => { :confirm => "Unlink err issues?" }, :class => "unlink-issue" - elsif problem.issue_link == "pending" - %span.disabled= link_to 'creating...', '#', :class => "#{problem.issue_type}_inactive create-issue" + %span.disabled + %img.button-icon{"src" => tracker_type.icons[:inactive]} + = link_to 'creating...', '#', :class => "create-issue" = link_to 'retry', create_issue_app_problem_path(app, problem), :method => :post - else - %span= link_to 'create issue', create_issue_app_problem_path(app, problem), method: :post, class: "#{app.issue_tracker.type_tracker}_create create-issue" + %span + %img.button-icon{"src" => tracker_type.icons[:goto]} + = link_to 'create issue', create_issue_app_problem_path(app, problem), method: :post, :class => "create-issue" diff --git a/lib/tasks/errbit/demo.rake b/lib/tasks/errbit/demo.rake index 078a57d..3203303 100644 --- a/lib/tasks/errbit/demo.rake +++ b/lib/tasks/errbit/demo.rake @@ -1,7 +1,7 @@ namespace :errbit do - desc "Add a demo app & errors to your database (for testing)" task :demo => :environment do + require 'fabrication' app = Fabricate(:app, :name => "Demo App #{Time.now.strftime("%N")}") @@ -68,5 +68,4 @@ namespace :errbit do Fabricate(:notice, :err => Fabricate(:err, :problem => Fabricate(:problem, :app => app))) puts "=== Created demo app: '#{app.name}', with example errors." end - end diff --git a/spec/controllers/problems_controller_spec.rb b/spec/controllers/problems_controller_spec.rb index f2a002d..3770330 100644 --- a/spec/controllers/problems_controller_spec.rb +++ b/spec/controllers/problems_controller_spec.rb @@ -218,9 +218,7 @@ describe ProblemsController, type: 'controller' do @err = Fabricate(:err) end - it 'finds the app and the err' do - expect(App).to receive(:find).with(@err.app.id.to_s).and_return(@err.app) - expect(@err.app.problems).to receive(:find).and_return(@err.problem) + it 'finds the app and the problem' do put :resolve, :app_id => @err.app.id, :id => @err.problem.id expect(controller.app).to eq @err.app expect(controller.problem).to eq @err.problem diff --git a/spec/decorators/issue_tracker_decorator_spec.rb b/spec/decorators/issue_tracker_decorator_spec.rb index 315c3bc..5d21edd 100644 --- a/spec/decorators/issue_tracker_decorator_spec.rb +++ b/spec/decorators/issue_tracker_decorator_spec.rb @@ -1,6 +1,6 @@ describe IssueTrackerDecorator do let(:fake_tracker) do - Class.new(ErrbitPlugin::IssueTracker) do + klass = Class.new(ErrbitPlugin::IssueTracker) { def self.label; 'fake'; end def self.note; 'a note'; end def self.fields @@ -11,43 +11,23 @@ describe IssueTrackerDecorator do end def configured?; true; end - end + } + klass.new 'nothing special' end - let(:decorator) do - IssueTrackerDecorator.new(fake_tracker, 'fake') + let(:issue_tracker) do + it = IssueTracker.new + allow(it).to receive(:tracker).and_return(fake_tracker) + it end - describe "#note" do - it 'return the html_safe of Note' do - expect(decorator.note).to eql fake_tracker.note - end - end - - describe "#issue_trackers" do - it 'return an array of IssueTrackerDecorator' do - decorator.issue_trackers do |it| - expect(it).to be_a(IssueTrackerDecorator) - end - end - end - - describe "#fields" do - it 'return all Fields define decorate' do - decorator.fields do |itf| - expect(itf).to be_a(IssueTrackerFieldDecorator) - expect([:foo, :bar]).to be_include(itf.object) - expect([{:label => 'foo'}, {:label => 'bar'}]).to be_include(itf.field_info) - end - end + let(:decorator) do + IssueTrackerDecorator.new(issue_tracker) end - describe "#params_class" do - it 'add the label in class' do - expect(decorator.params_class(IssueTracker.new(:type_tracker => 'none'))).to eql 'fake' - end - it 'add chosen class if _type is same' do - expect(decorator.params_class(IssueTracker.new(:type_tracker => 'fake'))).to eql 'chosen fake' + describe "#type" do + it 'returns decorator for the issue tracker class' do + expect(decorator.type.class).to eq(IssueTrackerTypeDecorator) end end end diff --git a/spec/decorators/issue_tracker_type_decorator_spec.rb b/spec/decorators/issue_tracker_type_decorator_spec.rb new file mode 100644 index 0000000..29af52d --- /dev/null +++ b/spec/decorators/issue_tracker_type_decorator_spec.rb @@ -0,0 +1,59 @@ +describe IssueTrackerDecorator do + let(:fake_tracker_class) do + klass = Class.new(ErrbitPlugin::IssueTracker) do + def self.label; 'fake'; end + def self.note; 'a note'; end + def self.fields + { + :foo => {:label => 'foo'}, + :bar => {:label => 'bar'} + } + end + def self.icons + { + one: ['text/plain', 'all your base are belong to us'], + two: ['application/xml', ''], + } + end + end + + allow(ErrbitPlugin::Registry).to receive(:issue_trackers).and_return({ + fake: klass + }) + + klass + end + + let(:fake_tracker) { IssueTrackerDecorator.new(fake_tracker_class.new) } + let(:decorator) { IssueTrackerTypeDecorator.new(fake_tracker_class) } + + describe "::note" do + it 'return the html_safe of Note' do + expect(decorator.note).to eql fake_tracker_class.note + end + end + + describe "#fields" do + it 'return all Fields define decorate' do + decorator.fields do |itf| + expect(itf).to be_a(IssueTrackerFieldDecorator) + expect([:foo, :bar]).to be_include(itf.object) + expect([{:label => 'foo'}, {:label => 'bar'}]).to be_include(itf.field_info) + end + end + end + + describe "#params_class" do + it 'adds the label in class' do + tracker = IssueTrackerDecorator.new(IssueTracker.new( + type_tracker: 'none')) + expect(decorator.params_class(tracker)).to eql 'fake' + end + + it 'adds chosen class if type is same' do + expect(decorator.params_class( + IssueTracker.new(type_tracker: 'fake').decorate + )).to eql 'chosen fake' + end + end +end diff --git a/spec/views/problems/show.html.haml_spec.rb b/spec/views/problems/show.html.haml_spec.rb index 5501e41..14ea145 100644 --- a/spec/views/problems/show.html.haml_spec.rb +++ b/spec/views/problems/show.html.haml_spec.rb @@ -4,12 +4,14 @@ describe "problems/show.html.haml", type: 'view' do let(:pivotal_tracker) { Class.new(ErrbitPlugin::MockIssueTracker) do def self.label; 'pivotal'; end + def self.icons; {}; end def configured?; true; end end } let(:github_tracker) { Class.new(ErrbitPlugin::MockIssueTracker) do - def label; 'github'; end + def self.label; 'github'; end + def self.icons; {}; end def configured?; true; end end } @@ -19,9 +21,10 @@ describe "problems/show.html.haml", type: 'view' do 'pivotal' => pivotal_tracker } } + let(:app) { AppDecorator.new(problem.app) } before do - allow(view).to receive(:app).and_return(problem.app) + allow(view).to receive(:app).and_return(app) allow(view).to receive(:problem).and_return(problem) assign :comment, comment @@ -33,7 +36,11 @@ describe "problems/show.html.haml", type: 'view' do def with_issue_tracker(tracker, problem) allow(ErrbitPlugin::Registry).to receive(:issue_trackers).and_return(trackers) - problem.app.issue_tracker = IssueTracker.new :type_tracker => tracker, :options => {:api_token => "token token token", :project_id => "1234"} + app.issue_tracker = IssueTrackerDecorator.new( + IssueTracker.new :type_tracker => tracker, :options => { + :api_token => "token token token", + :project_id => "1234" + }) end describe "content_for :action_bar" do @@ -86,7 +93,7 @@ describe "problems/show.html.haml", type: 'view' do allow(view).to receive(:app).and_return(problem.app) render - expect(action_bar).to have_selector("span a.github_create.create-issue", text: 'create issue') + expect(action_bar).to have_selector("span a.create-issue", text: 'create issue') end context "without issue tracker associate on app" do @@ -118,12 +125,7 @@ describe "problems/show.html.haml", type: 'view' do it 'links to the associated tracker' do render - expect(view.content_for(:action_bar)).to match(".pivotal_create.create-issue") - end - - it 'does not link to github' do - render - expect(view.content_for(:action_bar)).to_not match(".github_create.create-issue") + expect(view.content_for(:action_bar)).to match(".create-issue") end end @@ -179,4 +181,3 @@ describe "problems/show.html.haml", type: 'view' do end end end - -- libgit2 0.21.2