Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow users to opt-out of overdrafting #87

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/assets/javascripts/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to #88? General UI changes shouldn't happen in this PR.

7 changes: 6 additions & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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: ([email protected]))
@drinks_missing = Drink.where.not(price: ([email protected])).exists?
end
# show.html.haml
end

Expand Down Expand Up @@ -130,7 +135,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
Expand Down
1 change: 1 addition & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion app/views/application/_flash.html.haml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
- flash.each do |level, message|
.alert(class="alert-#{level}")
.flash.alert(class="alert-#{level}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

%strong= "#{level}!"
= message
1 change: 1 addition & 0 deletions app/views/users/_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 6 additions & 0 deletions app/views/users/show.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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}

Expand Down
10 changes: 10 additions & 0 deletions db/migrate/20200119134526_add_can_overdraw_to_users.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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|
Expand All @@ -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
1 change: 1 addition & 0 deletions test/fixtures/users.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ one:
name: A
email: [email protected]
balance: 100
can_overdraw: false

two:
name: B
Expand Down
11 changes: 11 additions & 0 deletions test/functional/users_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
42 changes: 40 additions & 2 deletions test/unit/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the change to rand(99)?

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
Expand All @@ -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"
Expand All @@ -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