From a88f0cc2761464b9690e5ee43bee859a039d9cfb Mon Sep 17 00:00:00 2001 From: Niklas Sombert Date: Mon, 20 Jan 2020 08:38:06 +0100 Subject: [PATCH 1/4] :sparkles: Allow users to opt-out of overdrafting To achieve this, there's a new user attribute: can_overdraw. This is enabled by default for all new and existing users. If this is set to false, this user's balance can go below zero. As a first step this throws validation errors which somehow works but is not very user-friendly. Also, this is not yet fully exposed via the API. --- app/controllers/users_controller.rb | 2 +- app/models/user.rb | 1 + app/views/users/_form.html.haml | 1 + db/migrate/20200119134526_add_can_overdraw_to_users.rb | 10 ++++++++++ db/schema.rb | 4 ++-- 5 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20200119134526_add_can_overdraw_to_users.rb diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a7e6a587..a13aeb6b 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -130,7 +130,7 @@ def buy_drink end def user_params - params.require(:user).permit(:name, :email, :balance, :active, :audit, :redirect) + params.require(:user).permit(:name, :email, :balance, :active, :audit, :redirect, :can_overdraw) end def warn_user_if_audit diff --git a/app/models/user.rb b/app/models/user.rb index 564b4353..12f97403 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -2,6 +2,7 @@ class User < ActiveRecord::Base validates_presence_of :name + validates :balance, numericality: {greater_than_or_equal_to: 0}, unless: :can_overdraw scope :order_by_name_asc, -> { order(arel_table['name'].lower.asc) } diff --git a/app/views/users/_form.html.haml b/app/views/users/_form.html.haml index 4c4716b5..0d088742 100644 --- a/app/views/users/_form.html.haml +++ b/app/views/users/_form.html.haml @@ -7,6 +7,7 @@ = f.input :active = f.input :audit, hint: 'This will create detailed logs about what you buy and deposit. If you uncheck this box, all records will be deleted.' = f.input :redirect, hint: "Redirect after buying a drink?" + = f.input :can_overdraw, hint: "Allow the balance to go below zero?" = f.actions do = f.action :submit diff --git a/db/migrate/20200119134526_add_can_overdraw_to_users.rb b/db/migrate/20200119134526_add_can_overdraw_to_users.rb new file mode 100644 index 00000000..ecd05be9 --- /dev/null +++ b/db/migrate/20200119134526_add_can_overdraw_to_users.rb @@ -0,0 +1,10 @@ +class AddCanOverdrawToUsers < ActiveRecord::Migration[5.2] + def up + add_column :users, :can_overdraw, :boolean, default: true + User.all.each { |user| user.can_overdraw = true } + end + + def down + remove_column :users, :can_overdraw + end +end diff --git a/db/schema.rb b/db/schema.rb index 6848716f..f73821a2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170929201659) do +ActiveRecord::Schema.define(version: 2020_01_19_134526) do create_table "audits", force: :cascade do |t| t.datetime "created_at" @@ -21,7 +21,6 @@ create_table "barcodes", id: :string, force: :cascade do |t| t.integer "drink", null: false - t.index ["id"], name: "sqlite_autoindex_barcodes_1", unique: true end create_table "drinks", force: :cascade do |t| @@ -47,6 +46,7 @@ t.boolean "active", default: true t.boolean "audit", default: false t.boolean "redirect", default: true + t.boolean "can_overdraw", default: true end end From b45a43bdb205126a5deb6825bcab4b86fe16ba69 Mon Sep 17 00:00:00 2001 From: Niklas Sombert Date: Mon, 20 Jan 2020 09:00:08 +0100 Subject: [PATCH 2/4] :white_check_mark: Add tests for disabling overdrafting --- test/fixtures/users.yml | 1 + test/functional/users_controller_test.rb | 11 +++++++ test/unit/user_test.rb | 42 ++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 8a194adf..d36e0446 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -4,6 +4,7 @@ one: name: A email: mail@example.net balance: 100 + can_overdraw: false two: name: B diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 78d206b7..1ca69841 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -98,6 +98,17 @@ class UsersControllerTest < ActionController::TestCase assert_redirected_to users(:two) end + test "buy with overdrafting disabled succeed if balance is high enough" do + get :buy, params: {id: @user, drink: @drink} + assert_redirected_to redirect_path(@user) + end + + test "buy with overdrafting disabled fails if balance is not high enough" do + assert_raises ActiveRecord::RecordInvalid do + get :payment, params: {id: @user, amount: 200} + end + end + test "buy by barcode" do assert_difference('Audit.count') do post :buy_barcode, params: {id: @user, barcode: @barcode} diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 4a2463a9..1954a98a 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -6,12 +6,32 @@ class UserTest < ActiveSupport::TestCase assert_not user.save, "Saved the user without a name" end + test "should not save without overdraft if balance is below zero" do + user = User.new + user.can_overdraw = false + user.balance = -1 + assert_not user.save, "Saved the user with overdrafting disabled and balance below zero" + end + test "should save" do user = User.new user.name = "test" assert user.save, "Failed to save the user with a name" end + test "should not modify if overdrafting is disabled and balance is below zero" do + user = users(:one) + user.balance = -1 + assert_not user.save, "Modified the user with overdrafting disabled and balance below zero" + end + + test "should modify if overdrafting is enabled and balance is below zero" do + user = users(:one) + user.balance = -1 + user.can_overdraw = true + assert user.save, "Failed to modify the user with overdrafting enabled and balance below zero" + end + test "should deposit" do assert users(:one).deposit(rand(500)), "Failed to deposit money" end @@ -25,17 +45,25 @@ class UserTest < ActiveSupport::TestCase end test "should pay" do - assert users(:one).payment(rand(500)), "Failed to pay" + assert users(:one).payment(rand(99)), "Failed to pay" end test "payment should decrease balance" do user = users(:one) - amount = rand(500) + amount = rand(99) balance_was = user.balance user.payment amount assert_equal balance_was - amount, user.balance, "Payment didn't decrease the balance" end + test "payment should not lead to a negative balance if overdrafting is disabled" do + user = users(:one) + amount = 200 + assert_raises ActiveRecord::RecordInvalid do + user.payment amount + end + end + test "should buy a drink" do assert users(:one).buy(drinks(:one)), "Failed to buy" end @@ -48,6 +76,15 @@ class UserTest < ActiveSupport::TestCase assert_equal balance_was - drink.price, user.balance, "Buying didn't decrease the balance" end + test "buying should not lead to a negative balance if overdrafting is disabled" do + user = users(:one) + user.balance = 1 + drink = drinks(:one) + assert_raises ActiveRecord::RecordInvalid do + user.buy drink + end + end + test "should have attributes" do u = users(:one) assert_respond_to u, :id, "id missing" @@ -59,5 +96,6 @@ class UserTest < ActiveSupport::TestCase assert_respond_to u, :active, "active missing" assert_respond_to u, :audit, "audit missing" assert_respond_to u, :redirect, "redirect missing" + assert_respond_to u, :can_overdraw, "can_overdraw missing" end end From 954e0a588c455d71438737a7e7ecb185165f78de Mon Sep 17 00:00:00 2001 From: Niklas Sombert Date: Mon, 20 Jan 2020 09:25:08 +0100 Subject: [PATCH 3/4] :recycle: Just fade out alerts created by flash This way we can create new alerts in other places that don't disappear by themselves. --- app/assets/javascripts/application.js | 2 +- app/views/application/_flash.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index f72f46ba..519090ef 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -28,4 +28,4 @@ if (navigator.userAgent.match(/(ipod|iphone|ipad)/i)) { } //hide notification bars after a few seconds -$('.alert').delay(10000).fadeOut('slow'); +$('.flash').delay(10000).fadeOut('slow'); diff --git a/app/views/application/_flash.html.haml b/app/views/application/_flash.html.haml index fd85db4e..0b3e22fa 100644 --- a/app/views/application/_flash.html.haml +++ b/app/views/application/_flash.html.haml @@ -1,4 +1,4 @@ - flash.each do |level, message| - .alert(class="alert-#{level}") + .flash.alert(class="alert-#{level}") %strong= "#{level}!" = message From e00a593a53f0d581ff03b98c633189348ac00521 Mon Sep 17 00:00:00 2001 From: Niklas Sombert Date: Mon, 20 Jan 2020 09:27:02 +0100 Subject: [PATCH 4/4] :sparkles: Hide some drinks when overdrawing is disabled --- app/controllers/users_controller.rb | 5 +++++ app/views/users/show.html.haml | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index a13aeb6b..7dcfdcd5 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -12,6 +12,11 @@ def index def show @user = User.find(params[:id]) @drinks = Drink.order(active: :desc).order_by_name_asc + @drinks_missing = false + unless @user.can_overdraw + @drinks = @drinks.where(price: (0..@user.balance)) + @drinks_missing = Drink.where.not(price: (0..@user.balance)).exists? + end # show.html.haml end diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 8dd27886..38e4541b 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -36,6 +36,12 @@ %hr +- if @drinks_missing + .alert.alert-info + Some drinks are hidden because their price is higher than your balance + and you have disabled overdrawing. + = link_to 'Change?', edit_user_url(@user) + .row.drinks = render @drinks, :locals => {:user => @user}