From b0c872ed3cff3d7df926da78c26ee300eacba6c5 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Mon, 14 May 2018 21:25:12 +0200 Subject: [PATCH 1/5] Helpers cleanup: - remove empty helpers to avoid load unnecessary constants (can be added on demand easily) - add some tests for ApplicationHelper and GlyphHelper --- app/helpers/application_helper.rb | 13 ++++++++----- app/helpers/categories_helper.rb | 2 -- app/helpers/devise_helper.rb | 2 -- app/helpers/global_helper.rb | 2 -- app/helpers/organizations_helper.rb | 2 -- app/helpers/sessions_helper.rb | 2 -- app/helpers/tags_helper.rb | 2 -- app/helpers/users_helper.rb | 2 -- spec/helpers/.gitkeep | 0 spec/helpers/application_helper_spec.rb | 22 ++++++++++++++++++++++ spec/helpers/glyph_helper_spec.rb | 14 ++++++++++++++ 11 files changed, 44 insertions(+), 19 deletions(-) delete mode 100644 app/helpers/categories_helper.rb delete mode 100644 app/helpers/devise_helper.rb delete mode 100644 app/helpers/global_helper.rb delete mode 100644 app/helpers/organizations_helper.rb delete mode 100644 app/helpers/sessions_helper.rb delete mode 100644 app/helpers/tags_helper.rb delete mode 100644 spec/helpers/.gitkeep create mode 100644 spec/helpers/application_helper_spec.rb create mode 100644 spec/helpers/glyph_helper_spec.rb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 5c4512e0d..f88b4c56b 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -6,11 +6,14 @@ module ApplicationHelper # from gravatar def avatar_url(user, size = 32) gravatar_id = Digest::MD5::hexdigest(user.email).downcase - gravatar_options = Hash[set: "set1", - gravatar: "hashed", - size: "#{size}x#{size}"] - "https://www.gravatar.com/avatar/#{gravatar_id}.png?" + - "#{Rack::Utils.build_query(gravatar_options)}&d=identicon" + gravatar_options = { + set: "set1", + gravatar: "hashed", + size: "#{size}x#{size}", + d: "identicon" + } + + "https://www.gravatar.com/avatar/#{gravatar_id}.png?#{gravatar_options.to_param}" end def mdash diff --git a/app/helpers/categories_helper.rb b/app/helpers/categories_helper.rb deleted file mode 100644 index e06f31554..000000000 --- a/app/helpers/categories_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module CategoriesHelper -end diff --git a/app/helpers/devise_helper.rb b/app/helpers/devise_helper.rb deleted file mode 100644 index 6db8fc929..000000000 --- a/app/helpers/devise_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module DeviseHelper -end diff --git a/app/helpers/global_helper.rb b/app/helpers/global_helper.rb deleted file mode 100644 index be6bce4b2..000000000 --- a/app/helpers/global_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module GlobalHelper -end diff --git a/app/helpers/organizations_helper.rb b/app/helpers/organizations_helper.rb deleted file mode 100644 index 24cc9a80e..000000000 --- a/app/helpers/organizations_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module OrganizationsHelper -end diff --git a/app/helpers/sessions_helper.rb b/app/helpers/sessions_helper.rb deleted file mode 100644 index 309f8b2eb..000000000 --- a/app/helpers/sessions_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module SessionsHelper -end diff --git a/app/helpers/tags_helper.rb b/app/helpers/tags_helper.rb deleted file mode 100644 index 23450bc5c..000000000 --- a/app/helpers/tags_helper.rb +++ /dev/null @@ -1,2 +0,0 @@ -module TagsHelper -end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index e7526201c..edd02894b 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -1,6 +1,4 @@ module UsersHelper - private - def edit_user_path(user) can_edit_user?(user) ? super : "" end diff --git a/spec/helpers/.gitkeep b/spec/helpers/.gitkeep deleted file mode 100644 index e69de29bb..000000000 diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb new file mode 100644 index 000000000..3129207d2 --- /dev/null +++ b/spec/helpers/application_helper_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +RSpec.describe ApplicationHelper do + it 'avatar_url returns url to gravatar' do + user = Fabricate(:user) + gravatar_id = Digest::MD5::hexdigest(user.email).downcase + expect(helper.avatar_url(user, 50)).to eq("https://www.gravatar.com/avatar/#{gravatar_id}.png?d=identicon&gravatar=hashed&set=set1&size=50x50") + end + + describe 'seconds_to_hm' do + it 'with 0 returns em dash' do + expect(helper.seconds_to_hm(0)).to eq("—") + end + + it 'with non-zero returns specific format' do + expect(helper.seconds_to_hm(10)).to eq("0:00") + expect(helper.seconds_to_hm(60)).to eq("0:01") + expect(helper.seconds_to_hm(-60)).to eq("-0:01") + expect(helper.seconds_to_hm(3600)).to eq("1:00") + end + end +end \ No newline at end of file diff --git a/spec/helpers/glyph_helper_spec.rb b/spec/helpers/glyph_helper_spec.rb new file mode 100644 index 000000000..9c3578395 --- /dev/null +++ b/spec/helpers/glyph_helper_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +RSpec.describe GlyphHelper do + describe 'glyph helper' do + it 'renders an span with glyphicon classes' do + expect(helper.glyph('foo')).to match(/<\/span>/) + end + + it 'special mappings' do + sample = GlyphHelper::GLYPHS.to_a.sample[0] + expect(helper.glyph(sample)).to match(GlyphHelper::GLYPHS[sample]) + end + end +end \ No newline at end of file From 84505ec749a257279ef95940cd24474dec140865 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Sat, 19 May 2018 14:37:43 +0200 Subject: [PATCH 2/5] admin: user form: ensure orgs order by ID asc in membership selector --- app/admin/user.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/admin/user.rb b/app/admin/user.rb index 09d337b42..203743025 100644 --- a/app/admin/user.rb +++ b/app/admin/user.rb @@ -43,7 +43,7 @@ end f.inputs "Members" do f.has_many :members do |m| - m.input :organization + m.input :organization, collection: Organization.order(id: :asc).pluck(:name, :id) m.input :manager end end From dd2170f79307c16b658aca8d9687bd92ee4c5376 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Mon, 21 May 2018 14:09:39 +0200 Subject: [PATCH 3/5] RSpec.describe => describe for consistency across the suite; minor spec_helper.rb edits --- spec/helpers/application_helper_spec.rb | 2 +- spec/helpers/glyph_helper_spec.rb | 2 +- spec/spec_helper.rb | 6 ++---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 3129207d2..0f985bfd0 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -RSpec.describe ApplicationHelper do +describe ApplicationHelper do it 'avatar_url returns url to gravatar' do user = Fabricate(:user) gravatar_id = Digest::MD5::hexdigest(user.email).downcase diff --git a/spec/helpers/glyph_helper_spec.rb b/spec/helpers/glyph_helper_spec.rb index 9c3578395..a946cce6a 100644 --- a/spec/helpers/glyph_helper_spec.rb +++ b/spec/helpers/glyph_helper_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -RSpec.describe GlyphHelper do +describe GlyphHelper do describe 'glyph helper' do it 'renders an span with glyphicon classes' do expect(helper.glyph('foo')).to match(/<\/span>/) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 8e737cd26..e156c9489 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -11,6 +11,7 @@ require 'selenium/webdriver' require 'faker' require 'shoulda/matchers' + I18n.reload! Capybara.register_driver :chrome do |app| @@ -36,7 +37,7 @@ # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } -# Make sure schem persists when running tests. +# Make sure schema persists when running tests. # We ran into an error that forced us to run rake db:migrate RAILS_ENV=test # before running tests. This kind of fixes it, although we should have a closer # look at this and find a better solution @@ -67,15 +68,12 @@ # the seed, which is printed after each run. # --seed 1234 - puts "Randomized with seed #{config.seed}." - config.register_ordering(:global) do |items| items_by_type = items.group_by { |item| item.metadata[:type] === :feature ? :feature : :rest } feature_specs = items_by_type[:feature] || [] rest_of_specs = items_by_type[:rest] || [] - random = Random.new(config.seed) [ From 898d6feb35a0921f84c644f0f09f996f1d296266 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Sun, 10 Jun 2018 21:53:44 +0200 Subject: [PATCH 4/5] Partially revert https://github.com/coopdevs/timeoverflow/commit/727a8653141d22f4cbbb5d790439720b716e50f7: - return to RSpec.describe as per https://github.com/coopdevs/timeoverflow/issues/362 - manually print seed as we don't use "config.order = 'random'" (we're using a custom ordering) ~> more info. https://github.com/coopdevs/timeoverflow/commit/94c50fbec07ae4fcecfd7d31914456d42d5b1dc1 - [EXTRA] remove sudo in Travis --- .travis.yml | 1 - spec/helpers/application_helper_spec.rb | 2 +- spec/helpers/glyph_helper_spec.rb | 2 +- spec/spec_helper.rb | 4 +++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 3d8884632..e0e22e1d0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,4 @@ language: ruby -sudo: required cache: bundler bundler_args: '--without production development' rvm: diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 0f985bfd0..3129207d2 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe ApplicationHelper do +RSpec.describe ApplicationHelper do it 'avatar_url returns url to gravatar' do user = Fabricate(:user) gravatar_id = Digest::MD5::hexdigest(user.email).downcase diff --git a/spec/helpers/glyph_helper_spec.rb b/spec/helpers/glyph_helper_spec.rb index a946cce6a..9c3578395 100644 --- a/spec/helpers/glyph_helper_spec.rb +++ b/spec/helpers/glyph_helper_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe GlyphHelper do +RSpec.describe GlyphHelper do describe 'glyph helper' do it 'renders an span with glyphicon classes' do expect(helper.glyph('foo')).to match(/<\/span>/) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e156c9489..5aa7072d1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,6 @@ # This file is copied to spec/ when you run 'rails generate rspec:install' ENV["RAILS_ENV"] ||= 'test' -ENV["ADMINS"] = "superadmin@example.com" +ENV["ADMINS"] = "admin@timeoverflow.org" require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' @@ -68,6 +68,8 @@ # the seed, which is printed after each run. # --seed 1234 + puts "Randomized with seed #{config.seed}." + config.register_ordering(:global) do |items| items_by_type = items.group_by { |item| item.metadata[:type] === :feature ? :feature : :rest } From 761c7ff9d38c74fae3a31a285387b36d3892e6c1 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Thu, 21 Jun 2018 21:57:31 +0200 Subject: [PATCH 5/5] move alert_class to ApplicationHelper (+ minor refactor); use valid css class in show_error_messages! helper --- app/helpers/application_helper.rb | 13 ++++++++++++- app/helpers/messages_helper.rb | 13 ------------- app/views/layouts/_messages.html.erb | 10 +++++----- config/initializers/simple_form.rb | 2 +- spec/helpers/application_helper_spec.rb | 7 +++++++ 5 files changed, 25 insertions(+), 20 deletions(-) delete mode 100644 app/helpers/messages_helper.rb diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index f88b4c56b..6c198475c 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -41,7 +41,7 @@ def show_error_messages!(resource) messages = resource.errors. full_messages.map { |msg| content_tag(:li, msg) }.join html = <<-HTML -
+
    #{messages} @@ -83,4 +83,15 @@ def get_body_css_class(controller) "#{classes[controller]}" end + + def alert_class(alert) + case alert + when 'error', 'alert' + 'alert-danger' + when 'notice' + 'alert-success' + else + 'alert-info' + end + end end diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb deleted file mode 100644 index 18a2aea43..000000000 --- a/app/helpers/messages_helper.rb +++ /dev/null @@ -1,13 +0,0 @@ -module MessagesHelper - def alert_class(alert) - if alert == 'error' || alert == 'alert' - 'alert-danger' - elsif alert == 'success' - 'alert-success' - elsif alert == 'notice' - 'alert-info' - else - 'alert-info' - end - end -end diff --git a/app/views/layouts/_messages.html.erb b/app/views/layouts/_messages.html.erb index b5e081551..0222a2c35 100644 --- a/app/views/layouts/_messages.html.erb +++ b/app/views/layouts/_messages.html.erb @@ -3,10 +3,10 @@
    -
      -
    • - <%= value %> -
      • -
    +
      +
    • + <%= value %> +
      • +
    <% end %> diff --git a/config/initializers/simple_form.rb b/config/initializers/simple_form.rb index 4b8786f02..d86db2045 100644 --- a/config/initializers/simple_form.rb +++ b/config/initializers/simple_form.rb @@ -66,7 +66,7 @@ config.error_notification_tag = :div # CSS class to add for error notification helper. - config.error_notification_class = 'alert alert-error' + config.error_notification_class = 'alert alert-danger' # ID to add for error notification helper. # config.error_notification_id = nil diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 3129207d2..dee9814e5 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -19,4 +19,11 @@ expect(helper.seconds_to_hm(3600)).to eq("1:00") end end + + it 'alert_class returns specific error classes' do + expect(helper.alert_class('error')).to eq('alert-danger') + expect(helper.alert_class('alert')).to eq('alert-danger') + expect(helper.alert_class('notice')).to eq('alert-success') + expect(helper.alert_class('foo')).to eq('alert-info') + end end \ No newline at end of file