Commit 5ee7f6cbccfdb3d7be6bf43179238fd370b8562b
1 parent
6cb19c23
Exists in
master
and in
1 other branch
783 allow custom issue tracker templates
Showing
4 changed files
with
66 additions
and
47 deletions
Show diff stats
app/controllers/problems_controller.rb
| @@ -61,10 +61,9 @@ class ProblemsController < ApplicationController | @@ -61,10 +61,9 @@ class ProblemsController < ApplicationController | ||
| 61 | end | 61 | end |
| 62 | 62 | ||
| 63 | def create_issue | 63 | def create_issue |
| 64 | - body = render_to_string "issue_trackers/issue", layout: false, formats: [:md] | ||
| 65 | - title = "[#{ problem.environment }][#{ problem.where }] #{problem.message.to_s.truncate(100)}" | 64 | + issue = Issue.new(problem: problem, user: current_user) |
| 65 | + issue.body = render_to_string(*issue.render_body_args) | ||
| 66 | 66 | ||
| 67 | - issue = Issue.new(problem: problem, user: current_user, title: title, body: body) | ||
| 68 | unless issue.save | 67 | unless issue.save |
| 69 | flash[:error] = issue.errors.full_messages.join(', ') | 68 | flash[:error] = issue.errors.full_messages.join(', ') |
| 70 | end | 69 | end |
app/models/issue.rb
| 1 | class Issue | 1 | class Issue |
| 2 | include ActiveModel::Model | 2 | include ActiveModel::Model |
| 3 | - attr_accessor :problem, :user, :title, :body | 3 | + attr_accessor :problem, :user, :body |
| 4 | 4 | ||
| 5 | def issue_tracker | 5 | def issue_tracker |
| 6 | - problem.app.issue_tracker | 6 | + @issue_tracker ||= problem.app.issue_tracker |
| 7 | end | 7 | end |
| 8 | 8 | ||
| 9 | - def save | ||
| 10 | - unless body | ||
| 11 | - errors.add :base, "The issue has no body" | ||
| 12 | - return false | ||
| 13 | - end | 9 | + def tracker |
| 10 | + @tracker ||= issue_tracker && issue_tracker.tracker | ||
| 11 | + end | ||
| 14 | 12 | ||
| 15 | - unless title | ||
| 16 | - errors.add :base, "The issue has no title" | ||
| 17 | - return false | 13 | + def render_body_args |
| 14 | + if tracker.respond_to?(:render_body_args) | ||
| 15 | + tracker.render_body_args | ||
| 16 | + else | ||
| 17 | + [ 'issue_trackers/issue', formats: [:md] ] | ||
| 18 | end | 18 | end |
| 19 | + end | ||
| 19 | 20 | ||
| 20 | - if issue_tracker | ||
| 21 | - issue_tracker.tracker.errors.each do |k, err| | ||
| 22 | - errors.add k, err | ||
| 23 | - end | ||
| 24 | - return false if errors.present? | ||
| 25 | - | ||
| 26 | - url = issue_tracker.create_issue(title, body, user: user.as_document) | ||
| 27 | - problem.update_attributes(issue_link: url, issue_type: issue_tracker.tracker.class.label) | 21 | + def title |
| 22 | + if tracker.respond_to?(:title) | ||
| 23 | + tracker.title | ||
| 28 | else | 24 | else |
| 29 | - errors.add :base, "This app has no issue tracker setup." | 25 | + "[#{ problem.environment }][#{ problem.where }] #{problem.message.to_s.truncate(100)}" |
| 30 | end | 26 | end |
| 27 | + end | ||
| 28 | + | ||
| 29 | + def save | ||
| 30 | + errors.add :base, "The issue has no body" unless body | ||
| 31 | + errors.add :base, "This app has no issue tracker" unless issue_tracker | ||
| 32 | + return false if errors.present? | ||
| 33 | + | ||
| 34 | + tracker.errors.each { |k, err| errors.add k, err } | ||
| 35 | + return false if errors.present? | ||
| 36 | + | ||
| 37 | + url = issue_tracker.create_issue(title, body, user: user.as_document) | ||
| 38 | + problem.update_attributes(issue_link: url, issue_type: tracker.class.label) | ||
| 31 | 39 | ||
| 32 | errors.empty? | 40 | errors.empty? |
| 33 | rescue => ex | 41 | rescue => ex |
spec/controllers/problems_controller_spec.rb
| @@ -290,7 +290,6 @@ describe ProblemsController, type: 'controller' do | @@ -290,7 +290,6 @@ describe ProblemsController, type: 'controller' do | ||
| 290 | expect(problem.issue_type).to eq("mock") | 290 | expect(problem.issue_type).to eq("mock") |
| 291 | end | 291 | end |
| 292 | 292 | ||
| 293 | - | ||
| 294 | context "when rendering views" do | 293 | context "when rendering views" do |
| 295 | render_views | 294 | render_views |
| 296 | 295 | ||
| @@ -299,6 +298,14 @@ describe ProblemsController, type: 'controller' do | @@ -299,6 +298,14 @@ describe ProblemsController, type: 'controller' do | ||
| 299 | line = issue_tracker.tracker.output.shift | 298 | line = issue_tracker.tracker.output.shift |
| 300 | expect(line[1]).to include(app_problem_url problem.app, problem) | 299 | expect(line[1]).to include(app_problem_url problem.app, problem) |
| 301 | end | 300 | end |
| 301 | + | ||
| 302 | + it "should render whatever the issue tracker says" do | ||
| 303 | + allow_any_instance_of(Issue).to receive(:render_body_args).and_return( | ||
| 304 | + [{ :inline => 'one <%= problem.id %> two' }]) | ||
| 305 | + post :create_issue, app_id: problem.app.id, id: problem.id, format: 'html' | ||
| 306 | + line = issue_tracker.tracker.output.shift | ||
| 307 | + expect(line[1]).to eq("one #{problem.id} two") | ||
| 308 | + end | ||
| 302 | end | 309 | end |
| 303 | end | 310 | end |
| 304 | 311 | ||
| @@ -306,7 +313,7 @@ describe ProblemsController, type: 'controller' do | @@ -306,7 +313,7 @@ describe ProblemsController, type: 'controller' do | ||
| 306 | it "should redirect to problem page" do | 313 | it "should redirect to problem page" do |
| 307 | post :create_issue, app_id: problem.app.id, id: problem.id | 314 | post :create_issue, app_id: problem.app.id, id: problem.id |
| 308 | expect(response).to redirect_to( app_problem_path(problem.app, problem) ) | 315 | expect(response).to redirect_to( app_problem_path(problem.app, problem) ) |
| 309 | - expect(flash[:error]).to eql "This app has no issue tracker setup." | 316 | + expect(flash[:error]).to eql "This app has no issue tracker" |
| 310 | end | 317 | end |
| 311 | end | 318 | end |
| 312 | end | 319 | end |
spec/models/issue_spec.rb
| 1 | describe Issue, type: 'model' do | 1 | describe Issue, type: 'model' do |
| 2 | - subject(:issue) { Issue.new(problem: problem, user: user, title: title, body: body) } | 2 | + subject(:issue) { Issue.new(problem: problem, user: user, body: body) } |
| 3 | 3 | ||
| 4 | let(:problem) { notice.problem } | 4 | let(:problem) { notice.problem } |
| 5 | let(:notice) { Fabricate(:notice) } | 5 | let(:notice) { Fabricate(:notice) } |
| @@ -12,7 +12,6 @@ describe Issue, type: 'model' do | @@ -12,7 +12,6 @@ describe Issue, type: 'model' do | ||
| 12 | let(:errors) { issue.errors[:base] } | 12 | let(:errors) { issue.errors[:base] } |
| 13 | 13 | ||
| 14 | context "when app has no issue tracker" do | 14 | context "when app has no issue tracker" do |
| 15 | - let(:title) { "Foo" } | ||
| 16 | let(:body) { "barrr" } | 15 | let(:body) { "barrr" } |
| 17 | 16 | ||
| 18 | context "#save" do | 17 | context "#save" do |
| @@ -22,29 +21,12 @@ describe Issue, type: 'model' do | @@ -22,29 +21,12 @@ describe Issue, type: 'model' do | ||
| 22 | 21 | ||
| 23 | it "returns an error" do | 22 | it "returns an error" do |
| 24 | issue.save | 23 | issue.save |
| 25 | - expect(errors).to include("This app has no issue tracker setup.") | ||
| 26 | - end | ||
| 27 | - end | ||
| 28 | - end | ||
| 29 | - | ||
| 30 | - context "when has no title" do | ||
| 31 | - let(:title) { nil } | ||
| 32 | - let(:body) { "barrr" } | ||
| 33 | - | ||
| 34 | - context "#save" do | ||
| 35 | - it "returns false" do | ||
| 36 | - expect(issue.save).to be false | ||
| 37 | - end | ||
| 38 | - | ||
| 39 | - it "returns an error" do | ||
| 40 | - issue.save | ||
| 41 | - expect(errors).to include("The issue has no title") | 24 | + expect(errors).to include("This app has no issue tracker") |
| 42 | end | 25 | end |
| 43 | end | 26 | end |
| 44 | end | 27 | end |
| 45 | 28 | ||
| 46 | context "when has no body" do | 29 | context "when has no body" do |
| 47 | - let(:title) { "Foo" } | ||
| 48 | let(:body) { nil } | 30 | let(:body) { nil } |
| 49 | 31 | ||
| 50 | context "#save" do | 32 | context "#save" do |
| @@ -60,15 +42,38 @@ describe Issue, type: 'model' do | @@ -60,15 +42,38 @@ describe Issue, type: 'model' do | ||
| 60 | end | 42 | end |
| 61 | 43 | ||
| 62 | context "when app has a issue tracker" do | 44 | context "when app has a issue tracker" do |
| 63 | - let(:title) { "Foo" } | ||
| 64 | let(:body) { "barrr" } | 45 | let(:body) { "barrr" } |
| 65 | 46 | ||
| 66 | before do | 47 | before do |
| 67 | problem.app.issue_tracker = issue_tracker | 48 | problem.app.issue_tracker = issue_tracker |
| 68 | end | 49 | end |
| 69 | 50 | ||
| 70 | - context "#save" do | 51 | + context "#render_body_args" do |
| 52 | + it "returns custom args if they exist" do | ||
| 53 | + allow(issue.tracker).to receive(:render_body_args).and_return( | ||
| 54 | + [ 'my', { custom: 'args' } ] | ||
| 55 | + ) | ||
| 56 | + expect(issue.render_body_args).to eq [ 'my', { custom: 'args' } ] | ||
| 57 | + end | ||
| 58 | + | ||
| 59 | + it "returns default args if none exist" do | ||
| 60 | + expect(issue.render_body_args).to eq [ | ||
| 61 | + 'issue_trackers/issue', formats: [:md] ] | ||
| 62 | + end | ||
| 63 | + end | ||
| 71 | 64 | ||
| 65 | + context "#title" do | ||
| 66 | + it "returns custom title if it exists" do | ||
| 67 | + allow(issue.tracker).to receive(:title).and_return('kustomtitle') | ||
| 68 | + expect(issue.title).to eq('kustomtitle') | ||
| 69 | + end | ||
| 70 | + | ||
| 71 | + it "returns default title when tracker has none" do | ||
| 72 | + expect(issue.title).to include(problem.message.to_s) | ||
| 73 | + end | ||
| 74 | + end | ||
| 75 | + | ||
| 76 | + context "#save" do | ||
| 72 | context "when issue tracker has errors" do | 77 | context "when issue tracker has errors" do |
| 73 | before do | 78 | before do |
| 74 | issue_tracker.tracker.options.clear | 79 | issue_tracker.tracker.options.clear |
| @@ -94,7 +99,7 @@ describe Issue, type: 'model' do | @@ -94,7 +99,7 @@ describe Issue, type: 'model' do | ||
| 94 | it "sends the title" do | 99 | it "sends the title" do |
| 95 | issue.save | 100 | issue.save |
| 96 | saved_issue = issue_tracker.tracker.output.first | 101 | saved_issue = issue_tracker.tracker.output.first |
| 97 | - expect(saved_issue.first).to be title | 102 | + expect(saved_issue.first).to eq issue.title |
| 98 | end | 103 | end |
| 99 | 104 | ||
| 100 | it "sends the body" do | 105 | it "sends the body" do |