From e3511edf418e316408cbb8be2e1c75cd6616b428 Mon Sep 17 00:00:00 2001 From: Dex Bleeker Date: Fri, 10 Sep 2021 10:23:49 +0200 Subject: [PATCH 1/7] WIP --- app/controllers/api_keys_controller.rb | 2 +- app/models/api_key.rb | 9 ++++++ app/models/user.rb | 4 --- config/environments/test.rb | 2 ++ db/seeds.rb | 4 +-- test/models/api_key_test.rb | 39 +++++++++++++------------- test/models/quote_test.rb | 2 +- test/test_helper.rb | 13 ++++++++- 8 files changed, 46 insertions(+), 29 deletions(-) diff --git a/app/controllers/api_keys_controller.rb b/app/controllers/api_keys_controller.rb index c98fe8cb6..a059986b9 100644 --- a/app/controllers/api_keys_controller.rb +++ b/app/controllers/api_keys_controller.rb @@ -3,7 +3,7 @@ class ApiKeysController < ApplicationController before_action :correct_user?, only: [:show] def create - current_user.generate_api_key(api_key_params[:name]) + ApiKey.create(user: current_user, name: api_key_params[:name]) redirect_to request.referer end diff --git a/app/models/api_key.rb b/app/models/api_key.rb index 7dcb230eb..9d13e584d 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -1,4 +1,13 @@ class ApiKey < ActiveRecord::Base acts_as_paranoid belongs_to :user + after_initialize :set_key + + has_rich_text :actiontext_description + + scope :with_user, -> { includes(:user) } + + def set_key + self.key ||= SecureRandom.urlsafe_base64 + end end diff --git a/app/models/user.rb b/app/models/user.rb index f8ea0c87a..e75760da1 100755 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -52,10 +52,6 @@ def signup(event, status, reason) event end - def generate_api_key(name) - ApiKey.new(user_id: id, name: name, key: SecureRandom.urlsafe_base64).save - end - def join_group(group) if !groups.only_deleted.where(group_id: group.id).empty? groups.with_deleted.where(group_id: group.id).first.restore diff --git a/config/environments/test.rb b/config/environments/test.rb index b0c7c4b22..1e90bd035 100755 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -40,4 +40,6 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true + + PaperTrail.enabled = false end diff --git a/db/seeds.rb b/db/seeds.rb index 4fd8383b3..0445fee35 100755 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -13,8 +13,8 @@ users.each do |email, options| u = User.create!(email: email, password: "12345678", password_confirmation: "12345678", **options) - u.generate_api_key(SecureRandom.hex[0, 6]) - u.generate_api_key(SecureRandom.hex[0, 6]) + ApiKey.create(user: u, name: "Key 1") + ApiKey.create(user: u, name: "Key 2") end # Create groups diff --git a/test/models/api_key_test.rb b/test/models/api_key_test.rb index 26031e5c2..1bc21e797 100644 --- a/test/models/api_key_test.rb +++ b/test/models/api_key_test.rb @@ -1,34 +1,33 @@ require 'test_helper' class ApiKeyTest < ActiveSupport::TestCase - test 'Create ApiKey' do - u = users(:one) + setup do + @user = users(:one) + @key = ApiKey.new(user: @user) + end - old_count = ApiKey.count + test 'valid api key' do + assert @key.valid? + end - apikey = ApiKey.new - apikey.user_id = u.id - apikey.save! + test 'creates random key' do + @key.save - assert ApiKey.count == old_count + 1 + assert_not_nil @key.key end - test 'ApiKey belongs to user' do - # user two doesn't have an api key yet - u = users(:two) + test 'api key belongs to user' do + @key.save - apikey = ApiKey.new - apikey.user_id = u.id - apikey.save! + key2 = ApiKey.create(user: users(:two)) - apikey2 = ApiKey.new - apikey2.user_id = u.id - apikey2.save! + @user.reload + + assert @key.user = @user + assert key2.user = @user - assert apikey.user_id == u.id - assert apikey2.user_id == u.id - assert_includes u.api_keys, apikey - assert_includes u.api_keys, apikey2 + assert_includes @user.api_keys, @key + assert_includes @user.api_keys, key2 end test 'ApiKey only belongs to 1 user' do diff --git a/test/models/quote_test.rb b/test/models/quote_test.rb index 819fdf891..c335dfc94 100755 --- a/test/models/quote_test.rb +++ b/test/models/quote_test.rb @@ -45,7 +45,7 @@ class QuoteTest < ActiveSupport::TestCase q2 = Quote.create(user: @user, text: "Quote 2", reporter: users(:two)) q3 = Quote.create(user: @user, text: "Quote 3", reporter: users(:two)) - assert_includes @user.quotes.all, @quote + assert_includes @user.quotes, @quote assert_includes @user.quotes, q2 assert_includes @user.quotes, q3 end diff --git a/test/test_helper.rb b/test/test_helper.rb index a693460f5..097578417 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -14,5 +14,16 @@ class ActiveSupport::TestCase # -- they do not yet inherit this setting fixtures :all - # Add more helper methods to be used by all tests here... + def with_versioning + was_enabled = PaperTrail.enabled? + was_enabled_for_request = PaperTrail.request.enabled? + PaperTrail.enabled = true + PaperTrail.request.enabled = true + begin + yield + ensure + PaperTrail.enabled = was_enabled + PaperTrail.request.enabled = was_enabled_for_request + end + end end From 243ecbf901f3c5c021a3214ea73b8377fef09e39 Mon Sep 17 00:00:00 2001 From: Dex Bleeker Date: Fri, 10 Sep 2021 10:29:18 +0200 Subject: [PATCH 2/7] Finish test for now --- app/models/api_key.rb | 4 ---- test/models/api_key_test.rb | 31 +++++++------------------------ 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/app/models/api_key.rb b/app/models/api_key.rb index 9d13e584d..01fb3162b 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -3,10 +3,6 @@ class ApiKey < ActiveRecord::Base belongs_to :user after_initialize :set_key - has_rich_text :actiontext_description - - scope :with_user, -> { includes(:user) } - def set_key self.key ||= SecureRandom.urlsafe_base64 end diff --git a/test/models/api_key_test.rb b/test/models/api_key_test.rb index 1bc21e797..f05ca772f 100644 --- a/test/models/api_key_test.rb +++ b/test/models/api_key_test.rb @@ -16,35 +16,18 @@ class ApiKeyTest < ActiveSupport::TestCase assert_not_nil @key.key end - test 'api key belongs to user' do + test 'api key belongs to (single) user' do @key.save - key2 = ApiKey.create(user: users(:two)) + key2 = ApiKey.create(user: users(:one)) + key3 = ApiKey.create(user: users(:three)) - @user.reload - - assert @key.user = @user - assert key2.user = @user + assert_equal @key.user, @user + assert_equal key2.user, @user + assert_not_equal key3.user, @user assert_includes @user.api_keys, @key assert_includes @user.api_keys, key2 - end - - test 'ApiKey only belongs to 1 user' do - u = users(:one) - u2 = users(:two) - - apikey = ApiKey.new - apikey.user_id = u.id - apikey.save! - - apikey2 = ApiKey.new - apikey2.user_id = u2.id - apikey2.save! - - assert apikey.user_id == u.id && apikey.user_id != u2.id - assert apikey2.user_id == u2.id && apikey2.user_id != u.id - - assert u.api_keys != u2.api_keys + assert_not_includes @user.api_keys, key3 end end From 59db5a2d1adeed7e0deeb8df172d9887c5657fe6 Mon Sep 17 00:00:00 2001 From: Dex Bleeker Date: Fri, 10 Sep 2021 10:40:52 +0200 Subject: [PATCH 3/7] Valide presence of user and name for api key --- app/models/api_key.rb | 2 ++ test/models/api_key_test.rb | 28 ++++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/app/models/api_key.rb b/app/models/api_key.rb index 01fb3162b..2285947f1 100644 --- a/app/models/api_key.rb +++ b/app/models/api_key.rb @@ -1,6 +1,8 @@ class ApiKey < ActiveRecord::Base acts_as_paranoid belongs_to :user + validates :user, presence: true + validates :name, presence: true, allow_blank: false after_initialize :set_key def set_key diff --git a/test/models/api_key_test.rb b/test/models/api_key_test.rb index f05ca772f..909803b57 100644 --- a/test/models/api_key_test.rb +++ b/test/models/api_key_test.rb @@ -3,24 +3,38 @@ class ApiKeyTest < ActiveSupport::TestCase setup do @user = users(:one) - @key = ApiKey.new(user: @user) + @key = ApiKey.new(user: @user, name: "Key 1") end test 'valid api key' do assert @key.valid? end + test 'invalid without name' do + @key.name = nil + refute @key.valid? + end + + test 'invalid with blank name' do + @key.name = '' + refute @key.valid? + end + + test 'invalid if user does not exist' do + @key.user_id = -1 + refute @key.valid? + end + test 'creates random key' do @key.save - assert_not_nil @key.key end test 'api key belongs to (single) user' do @key.save - key2 = ApiKey.create(user: users(:one)) - key3 = ApiKey.create(user: users(:three)) + key2 = ApiKey.create(user: users(:one), name: "Key 2") + key3 = ApiKey.create(user: users(:three), name: "Key 3") assert_equal @key.user, @user assert_equal key2.user, @user @@ -30,4 +44,10 @@ class ApiKeyTest < ActiveSupport::TestCase assert_includes @user.api_keys, key2 assert_not_includes @user.api_keys, key3 end + + test 'api key does not have versions (papertrail)' do + assert_raises NoMethodError do + @key.versions + end + end end From cf2bdfe9c1b2cfc4ca529f5496fce002de7b71be Mon Sep 17 00:00:00 2001 From: Dex Bleeker Date: Fri, 10 Sep 2021 11:35:02 +0200 Subject: [PATCH 4/7] Improve beer tests --- app/models/beer.rb | 5 -- config/environments/test.rb | 7 +- test/models/beer_test.rb | 138 ++++++++++++++++-------------------- test/test_helper.rb | 2 + 4 files changed, 70 insertions(+), 82 deletions(-) diff --git a/app/models/beer.rb b/app/models/beer.rb index a9c7ae7d0..0a3773793 100644 --- a/app/models/beer.rb +++ b/app/models/beer.rb @@ -8,11 +8,6 @@ class Beer < ActiveRecord::Base scope :random, -> { order('RAND()') } - # This method is (only) used in the tests - def add_review!(user, rating, description, proefdatum = Date.today) - self.reviews.create!(user_id: user.id, rating: rating.round, description: description, proefdatum: proefdatum) - end - def cijfer? grade = self.grade if grade diff --git a/config/environments/test.rb b/config/environments/test.rb index 1e90bd035..ebeec264a 100755 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -20,7 +20,7 @@ } # Show full error reports and disable caching. - config.consider_all_requests_local = true + config.consider_all_requests_local = true config.action_controller.perform_caching = false # Raise exceptions instead of rendering exception templates. @@ -41,5 +41,8 @@ # Raises error for missing translations # config.action_view.raise_on_missing_translations = true - PaperTrail.enabled = false + # Tests are disabled by default to speed up tests + config.after_initialize do + PaperTrail.enabled = false + end end diff --git a/test/models/beer_test.rb b/test/models/beer_test.rb index dc4138f71..f2c4b6ad1 100644 --- a/test/models/beer_test.rb +++ b/test/models/beer_test.rb @@ -1,94 +1,82 @@ require 'test_helper' class BeerTest < ActiveSupport::TestCase - test 'Add beer correctly' do - old_count = Beer.count - - b = Beer.new - assert !b.save - b = Beer.new(percentage: 'a') - assert !b.save - b = Beer.new(percentage: '') - assert !b.save - b = Beer.new(percentage: '5.0%') - assert b.save - b = Beer.new(percentage: '5') - assert b.save - b = Beer.new(percentage: '100.000') - assert b.save - - assert_equal Beer.count, old_count + 3 + setup do + @beer = Beer.new(name: "Leffe Blond", soort: "Blond", brewer: "Leffe", country: "Belgiƫ", percentage: "6.6 %") end - test 'Review grade correctly' do - # users 3 and 4 don't have reviews yet (thus don't skew the ratings) - u = users(:three) - u2 = users(:four) - - b = beers(:one) - # b.add_review(user, rating, description, proefdatum) - b.add_review!(u, 8.0, 'Heel lekker biertje') - - b.add_review!(u2, 7.0, 'Vrij lekker biertje') - + test 'valid beer' do + assert @beer.valid? end - test 'Review grade with weight' do - u = users(:one) - u2 = users(:two) - - b = beers(:one) - b2 = beers(:two) - b3 = beers(:three) - b4 = beers(:four) - b5 = beers(:five) - b6 = beers(:six) - # set weight of user u - b.add_review!(u, 9.0, '') - b2.add_review!(u, 9.0, '',) - b3.add_review!(u, 9.0, '') - b4.add_review!(u, 9.0, '') + test 'beer does have versions (papertrail)' do + assert_nothing_raised do + @beer.versions + end - # set weight of user u2 - b.add_review!(u2, 2.0, '') - b2.add_review!(u2, 2.0, '') - b3.add_review!(u2, 2.0, '') - b4.add_review!(u2, 2.0, '') + with_versioning do + @beer.save + assert_equal 1, @beer.versions.count + end + end - b5.add_review!(u, 8.0, '') - b5.add_review!(u2, 9.0, '') - b5.update_cijfer - # what todo with the rating: - # keep it weighted or just average - b6.add_review!(u, 8.0, '') - b6.add_review!(u2, 2.0, '') + test 'beer has new version when name changes' do + with_versioning do + @beer.save + assert_difference '@beer.versions.count', 1 do + @beer.name = "Changed name" + @beer.save + end + end end - test 'Review belongs to person' do - u = users(:one) - b = beers(:one) + test 'beer has new version when soort changes' do + with_versioning do + @beer.save + assert_difference '@beer.versions.count', 1 do + @beer.soort = "Changed soort" + @beer.save + end + end + end - b.add_review!(u, 5.0, '') - assert_equal b.reviews.last.user, u + test 'beer has new version when brewer changes' do + with_versioning do + @beer.save + assert_difference '@beer.versions.count', 1 do + @beer.brewer = "Changed brewer" + @beer.save + end + end end - test 'Rating should be correct' do - skip("This test does not work currently.") - u = users(:one) - b = beers(:one) + test 'beer has new version when country changes' do + with_versioning do + @beer.save + assert_difference '@beer.versions.count', 1 do + @beer.country = "Changed country" + @beer.save + end + end + end - assert_nothing_raised do - b.add_review!(u, 5, '') - b.add_review!(u, 1.0, '') - b.add_review!(u, 10.0, '') - b.add_review!(u, 5, '') + test 'beer has new version when percentage changes' do + with_versioning do + @beer.save + assert_difference '@beer.versions.count', 1 do + @beer.percentage = 1.0 + @beer.save + end end + end - assert_raise { b.add_review!(u, 50, '') } - assert_raise { b.add_review!(u, 0, '') } - assert_raise { b.add_review!(u, 'something', '') } - assert_raise { b.add_review!(u, '', '') } - assert_raise { b.add_review!(u, 7.6, '') } - assert_raise { b.add_review!(u, 7.62345445, '') } + test 'beer does not have a new version when grade changes' do + with_versioning do + @beer.save + assert_no_difference '@beer.versions.count' do + @beer.grade = 6 + @beer.save + end + end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 097578417..eb1abf218 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,6 +1,7 @@ ENV["RAILS_ENV"] ||= "test" require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help' + class ActionController::TestCase include Devise::Test::ControllerHelpers end @@ -14,6 +15,7 @@ class ActiveSupport::TestCase # -- they do not yet inherit this setting fixtures :all + # Allow manually enable versioning for specific tests def with_versioning was_enabled = PaperTrail.enabled? was_enabled_for_request = PaperTrail.request.enabled? From 963428f9fe0ba0bc859237b1f195a4316991dffa Mon Sep 17 00:00:00 2001 From: Dex Bleeker Date: Fri, 10 Sep 2021 11:57:17 +0200 Subject: [PATCH 5/7] Test versions for signups --- test/models/signup_test.rb | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb index 9ed9ee4c7..2408f046a 100644 --- a/test/models/signup_test.rb +++ b/test/models/signup_test.rb @@ -8,7 +8,7 @@ class SignupTest < ActiveSupport::TestCase end_time: DateTime.now + 1.day + 2.hours, deadline: DateTime.now + 5, user: @user) - @signup = Signup.new(event: @event, user: @user) + @signup = Signup.new(event: @event, user: @user) # Status is true by default end test 'valid signup' do @@ -61,4 +61,28 @@ class SignupTest < ActiveSupport::TestCase @signup.status = nil refute @signup.valid? end + + test 'signup has versions (papertrail)' do + assert_nothing_raised { @signup.versions } + end + + test 'signup has a new version when status changes' do + with_versioning do + @signup.save + assert_difference '@signup.versions.count' do + @signup.status = false + @signup.save + end + end + end + + test 'signup has a new version when reason changes' do + with_versioning do + @signup.save + assert_difference '@signup.versions.count' do + @signup.reason = "New reason" + @signup.save + end + end + end end From 153e340044486f5430a9fb4ccb3b1ed7c040d7a5 Mon Sep 17 00:00:00 2001 From: Dex Bleeker Date: Fri, 10 Sep 2021 12:27:42 +0200 Subject: [PATCH 6/7] Add tests for versions (reviews/signups) --- test/models/beer_test.rb | 4 +--- test/models/review_test.rb | 43 ++++++++++++++++++++++++++++++++++++++ test/models/signup_test.rb | 15 ++++++++++--- 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/test/models/beer_test.rb b/test/models/beer_test.rb index f2c4b6ad1..7ff29738c 100644 --- a/test/models/beer_test.rb +++ b/test/models/beer_test.rb @@ -10,9 +10,7 @@ class BeerTest < ActiveSupport::TestCase end test 'beer does have versions (papertrail)' do - assert_nothing_raised do - @beer.versions - end + assert_nothing_raised { @beer.versions } with_versioning do @beer.save diff --git a/test/models/review_test.rb b/test/models/review_test.rb index 6f077cc1f..937f18ffb 100644 --- a/test/models/review_test.rb +++ b/test/models/review_test.rb @@ -63,4 +63,47 @@ class ReviewTest < ActiveSupport::TestCase @review.beer_id = -1 refute @review.valid? end + + test 'has versions (papertrail)' do + assert_nothing_raised { @review.versions } + end + + test 'has a new version when rating changes' do + with_versioning do + @review.save + assert_difference '@review.versions.count' do + @review.rating = 6 + @review.save + end + end + end + + test 'has a new version when description changes' do + with_versioning do + @review.save + assert_difference '@review.versions.count' do + @review.actiontext_description = "New description" + @review.save + end + end + end + + test 'has a new version when proefdatum changes' do + with_versioning do + @review.save + assert_difference '@review.versions.count' do + @review.proefdatum = Date.today - 1.day + @review.save + end + end + end + + test 'has a new version when it is deleted' do + with_versioning do + @review.save + assert_difference 'PaperTrail::Version.count', 2 do + @review.destroy + end + end + end end diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb index 2408f046a..129fd6939 100644 --- a/test/models/signup_test.rb +++ b/test/models/signup_test.rb @@ -62,11 +62,11 @@ class SignupTest < ActiveSupport::TestCase refute @signup.valid? end - test 'signup has versions (papertrail)' do + test 'has versions (papertrail)' do assert_nothing_raised { @signup.versions } end - test 'signup has a new version when status changes' do + test 'has a new version when status changes' do with_versioning do @signup.save assert_difference '@signup.versions.count' do @@ -76,7 +76,7 @@ class SignupTest < ActiveSupport::TestCase end end - test 'signup has a new version when reason changes' do + test 'has a new version when reason changes' do with_versioning do @signup.save assert_difference '@signup.versions.count' do @@ -85,4 +85,13 @@ class SignupTest < ActiveSupport::TestCase end end end + + test 'has a new version when it is deleted' do + with_versioning do + @signup.save + assert_difference 'PaperTrail::Version.count', 2 do + @signup.destroy + end + end + end end From 73ea7ad1565d4d5e20fe9d69d3d9fc525da12ea2 Mon Sep 17 00:00:00 2001 From: Dex Bleeker Date: Fri, 10 Sep 2021 12:42:22 +0200 Subject: [PATCH 7/7] Add version tests for quotes --- test/models/quote_test.rb | 39 ++++++++++++++++++++++++++++++++++++++ test/models/review_test.rb | 1 + test/models/signup_test.rb | 2 +- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/test/models/quote_test.rb b/test/models/quote_test.rb index c335dfc94..fa4a99911 100755 --- a/test/models/quote_test.rb +++ b/test/models/quote_test.rb @@ -57,4 +57,43 @@ class QuoteTest < ActiveSupport::TestCase @quote.delete refute @quote.deleted_at.nil? end + + test 'has a new version when text changes' do + with_versioning do + @quote.save + assert_difference '@quote.versions.count' do + @quote.text = "New quote body" + @quote.save + end + end + end + + test 'has a new version when user changes' do + with_versioning do + @quote.save + assert_difference '@quote.versions.count' do + @quote.user = users(:two) + @quote.save + end + end + end + + test 'has a new version when reporter changes' do + with_versioning do + @quote.save + assert_difference '@quote.versions.count' do + @quote.reporter = users(:three) + @quote.save + end + end + end + + test 'has a new version when it is deleted' do + with_versioning do + @quote.save + assert_difference 'PaperTrail::Version.count' do + @quote.destroy + end + end + end end diff --git a/test/models/review_test.rb b/test/models/review_test.rb index 937f18ffb..0f505149f 100644 --- a/test/models/review_test.rb +++ b/test/models/review_test.rb @@ -101,6 +101,7 @@ class ReviewTest < ActiveSupport::TestCase test 'has a new version when it is deleted' do with_versioning do @review.save + # TODO: Why is this 2? assert_difference 'PaperTrail::Version.count', 2 do @review.destroy end diff --git a/test/models/signup_test.rb b/test/models/signup_test.rb index 129fd6939..ce96e4b95 100644 --- a/test/models/signup_test.rb +++ b/test/models/signup_test.rb @@ -89,7 +89,7 @@ class SignupTest < ActiveSupport::TestCase test 'has a new version when it is deleted' do with_versioning do @signup.save - assert_difference 'PaperTrail::Version.count', 2 do + assert_difference 'PaperTrail::Version.count' do @signup.destroy end end