From e3bc88ae67a1cd7d6ac74e2c893ecdf56923c1e5 Mon Sep 17 00:00:00 2001 From: Dave Miller Date: Wed, 20 Nov 2013 09:32:47 -0800 Subject: [PATCH] "Seriously?" Seriously? Replace these resolve/delete confirmations with useful messages. --- app/helpers/problems_helper.rb | 4 ++-- app/views/apps/edit.html.haml | 3 ++- app/views/problems/_table.html.haml | 8 ++++++-- app/views/problems/show.html.haml | 9 ++++++--- app/views/users/show.html.haml | 3 ++- config/config.example.yml | 4 ++-- config/initializers/_load_config.rb | 2 +- config/locales/en.yml | 14 ++++++++++++++ spec/views/apps/edit.html.haml_spec.rb | 3 +-- spec/views/problems/show.html.haml_spec.rb | 12 ++++-------- spec/views/users/show.html.haml_spec.rb | 6 ++++++ 11 files changed, 46 insertions(+), 22 deletions(-) diff --git a/app/helpers/problems_helper.rb b/app/helpers/problems_helper.rb index a184c2a..e2c11d6 100644 --- a/app/helpers/problems_helper.rb +++ b/app/helpers/problems_helper.rb @@ -1,6 +1,6 @@ module ProblemsHelper - def problem_confirm - Errbit::Config.confirm_resolve_err === false ? nil : 'Seriously?' + def problem_confirm(action) + t('problems.confirm.%s' % action) unless Errbit::Config.confirm_err_actions.eql? false end def truncated_problem_message(problem) diff --git a/app/views/apps/edit.html.haml b/app/views/apps/edit.html.haml index df224b0..20c66cc 100644 --- a/app/views/apps/edit.html.haml +++ b/app/views/apps/edit.html.haml @@ -1,7 +1,8 @@ - content_for :title, 'Edit App' - content_for :action_bar do = link_to_copy_attributes_from_other_app - = link_to 'destroy application', app_path(app), :method => :delete, :data => { :confirm => 'Seriously?' }, :class => 'button' + = link_to 'delete application', app_path(app), :method => :delete, + :data => { :confirm => t('apps.confirm_delete') }, :class => 'button' = link_to('cancel', app_path(app), :class => 'button') = form_for app do |f| diff --git a/app/views/problems/_table.html.haml b/app/views/problems/_table.html.haml index 58f7eee..37a6225 100644 --- a/app/views/problems/_table.html.haml +++ b/app/views/problems/_table.html.haml @@ -40,7 +40,9 @@ %td.issue_link - if problem.app.issue_tracker_configured? && problem.issue_link.present? && problem.issue_link != 'pending' = link_to image_tag("#{problem.issue_type}_goto.png"), problem.issue_link, :target => "_blank" - %td.resolve= link_to image_tag("thumbs-up.png"), resolve_app_problem_path(problem.app, problem), :title => "Resolve", :method => :put, :data => { :confirm => problem_confirm }, :class => 'resolve' if problem.unresolved? + %td.resolve= link_to image_tag("thumbs-up.png"), resolve_app_problem_path(problem.app, problem), + :title => "Resolve", :method => :put, :data => { :confirm => problem_confirm('resolve_one') }, + :class => 'resolve' if problem.unresolved? - if problems.none? %tr %td{:colspan => (any_issue_links ? 8 : 7)} @@ -49,4 +51,6 @@ .tab-bar %ul - %w(merge unmerge resolve unresolve delete).each do |action| - %li= submit_tag action.capitalize, :id => "#{action}_problems", :class => 'button', :data => { :action => polymorphic_path([action == 'delete' ? 'destroy' : action, 'several_problems']), :confirm => problem_confirm } + %li= submit_tag action.capitalize, :id => "#{action}_problems", :class => 'button', + :data => { :action => polymorphic_path([action == 'delete' ? 'destroy' : action, 'several_problems']), + :confirm => problem_confirm(action) } diff --git a/app/views/problems/show.html.haml b/app/views/problems/show.html.haml index 1137fef..ac8f353 100644 --- a/app/views/problems/show.html.haml +++ b/app/views/problems/show.html.haml @@ -13,9 +13,11 @@ = problem.last_notice_at.to_s(:precise) - content_for :action_bar do - if problem.unresolved? - %span= link_to 'resolve', [:resolve, app, problem], :method => :put, :data => { :confirm => problem_confirm }, :class => 'resolve' + %span= link_to 'resolve', [:resolve, app, problem], :method => :put, + :data => { :confirm => problem_confirm('resolve_one') }, :class => 'resolve' - if current_user.authentication_token - %span= link_to 'iCal', polymorphic_path([app, problem], :format => "ics", :auth_token => current_user.authentication_token), :class => "calendar_link" + %span= link_to 'iCal', polymorphic_path([app, problem], :format => "ics", + :auth_token => current_user.authentication_token), :class => "calendar_link" %span>= link_to 'up', (request.env['HTTP_REFERER'] ? :back : app_problems_path(app)), :class => 'up' %br = render "issue_tracker_links" @@ -37,7 +39,8 @@ - else %span.comment-info = time_ago_in_words(comment.created_at, true) << " ago by [Unknown User]" - %span.delete= link_to '✘'.html_safe, [app, problem, comment], :method => :delete, :data => { :confirm => "Are you sure you don't need this comment?" }, :class => "destroy-comment" + %span.delete= link_to '✘'.html_safe, [app, problem, comment], :method => :delete, + :data => { :confirm => t("comments.confirm_delete") }, :class => "destroy-comment" %tr %td= simple_format comment.body - if problem.comments_allowed? diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 1e18043..9a0cff7 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -9,7 +9,8 @@ = render 'shared/link_github_account' %span= link_to('Add a New User', new_user_path, :class => 'add') = link_to 'edit', edit_user_path(user), :class => 'button' - = link_to 'destroy', user_path(user), :method => :delete, :data => { :confirm => 'Seriously?' }, :class => 'button' + = link_to 'destroy', user_path(user), :method => :delete, + :data => { :confirm => t('users.confirm_delete') }, :class => 'delete button' %table.single_user %tr diff --git a/config/config.example.yml b/config/config.example.yml index f53c8ce..1c8d145 100644 --- a/config/config.example.yml +++ b/config/config.example.yml @@ -36,8 +36,8 @@ per_app_notify_at_notices: false # [0] for all notices, provided notification service is configured notify_at_notices: [0] -# Configure whether or not the user should be prompted before resolving an error. -confirm_resolve_err: true +# Configure whether or not the user should be prompted before resolving/deleting/merging/etc an error. +confirm_err_actions: true # Add an optional 'username' field to Users. # Helpful when you need to plug in a custom authentication strategy, such as LDAP. diff --git a/config/initializers/_load_config.rb b/config/initializers/_load_config.rb index a6ab75e..e47bffb 100644 --- a/config/initializers/_load_config.rb +++ b/config/initializers/_load_config.rb @@ -12,7 +12,7 @@ unless defined?(Errbit::Config) Errbit::Config.email_from = ENV['ERRBIT_EMAIL_FROM'] # Not really easy to use like an env because need an array and ENV return a string :( # Errbit::Config.email_at_notices = ENV['ERRBIT_EMAIL_AT_NOTICES'] - Errbit::Config.confirm_resolve_err = ENV['ERRBIT_CONFIRM_RESOLVE_ERR'].to_i == 0 + Errbit::Config.confirm_err_actions = ENV['ERRBIT_CONFIRM_ERR_ACTIONS'].to_i == 0 Errbit::Config.user_has_username = ENV['ERRBIT_USER_HAS_USERNAME'].to_i == 1 Errbit::Config.allow_comments_with_issue_tracker = ENV['ERRBIT_ALLOW_COMMENTS_WITH_ISSUE_TRACKER'].to_i == 0 Errbit::Config.enforce_ssl = ENV['ERRBIT_ENFORCE_SSL'] diff --git a/config/locales/en.yml b/config/locales/en.yml index 7d49c3e..3b7ef75 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -69,7 +69,21 @@ en: new: sign_in: "Sign in" + problems: + confirm: + delete: "Permanently delete selected issues?" + resolve: "Resolve selected issues? They can be unresolved later." + resolve_one: "Resolve this issue? It can be unresolved later." + merge: "Merge select issues? They can be unmerged later." + unmerge: "Unmerge selected issues? They can be re-merged later." + unresolve: "Unresolve selected issues? They can be resolved again later." + + comments: + confirm_delete: "Premanently delete this comment?" + users: + confirm_delete: "Permanently delete this user?" apps: + confirm_delete: "Permanently delete this app?" index: notify: Notification Service tracker: Tracker diff --git a/spec/views/apps/edit.html.haml_spec.rb b/spec/views/apps/edit.html.haml_spec.rb index 22ef758..7e2aa8f 100644 --- a/spec/views/apps/edit.html.haml_spec.rb +++ b/spec/views/apps/edit.html.haml_spec.rb @@ -14,8 +14,7 @@ describe "apps/edit.html.haml" do it "should confirm the 'destroy' link" do render - - action_bar.should have_selector('a.button[data-confirm="Seriously?"]') + action_bar.should have_selector('a.button[data-confirm="%s"]' % I18n.t('apps.confirm_delete')) end end diff --git a/spec/views/problems/show.html.haml_spec.rb b/spec/views/problems/show.html.haml_spec.rb index 5f35843..fe9a21f 100644 --- a/spec/views/problems/show.html.haml_spec.rb +++ b/spec/views/problems/show.html.haml_spec.rb @@ -28,21 +28,18 @@ describe "problems/show.html.haml" do it "should confirm the 'resolve' link by default" do render - - action_bar.should have_selector('a.resolve[data-confirm="Seriously?"]') + action_bar.should have_selector('a.resolve[data-confirm="%s"]' % I18n.t('problems.confirm.resolve_one')) end it "should confirm the 'resolve' link if configuration is unset" do - Errbit::Config.stub(:confirm_resolve_err).and_return(nil) + Errbit::Config.stub(:confirm_err_actions).and_return(nil) render - - action_bar.should have_selector('a.resolve[data-confirm="Seriously?"]') + action_bar.should have_selector('a.resolve[data-confirm="%s"]' % I18n.t('problems.confirm.resolve_one')) end it "should not confirm the 'resolve' link if configured not to" do - Errbit::Config.stub(:confirm_resolve_err).and_return(false) + Errbit::Config.stub(:confirm_err_actions).and_return(false) render - action_bar.should have_selector('a.resolve[data-confirm="null"]') end @@ -50,7 +47,6 @@ describe "problems/show.html.haml" do url = 'http://localhost:3000/problems' controller.request.env['HTTP_REFERER'] = url render - action_bar.should have_selector("span a.up[href='#{url}']", :text => 'up') end diff --git a/spec/views/users/show.html.haml_spec.rb b/spec/views/users/show.html.haml_spec.rb index e97cae9..7aa2674 100644 --- a/spec/views/users/show.html.haml_spec.rb +++ b/spec/views/users/show.html.haml_spec.rb @@ -53,6 +53,12 @@ describe 'users/show.html.haml' do render view.content_for(:action_bar).should include('Unlink GitHub account') end + + it "should confirm the 'resolve' link by default" do + render + view.content_for(:action_bar).should have_selector('a.delete[data-confirm="%s"]' % I18n.t('.users.confirm_delete')) + end + end end end -- libgit2 0.21.2