From d45d9d3ac53c2af974732733b65322ddc1df5385 Mon Sep 17 00:00:00 2001
From: Wilco <wilcovanbeijnum@gmail.com>
Date: Thu, 29 Sep 2022 14:21:34 +0200
Subject: [PATCH] Block negative credit orders (#779)

---
 app/controllers/activities_controller.rb      |  9 ++--
 .../credit_mutations_controller.rb            |  4 +-
 app/controllers/orders_controller.rb          | 10 ++--
 app/javascript/packs/order_screen.js          | 23 +++++----
 app/models/order.rb                           |  5 ++
 app/models/user.rb                            | 13 ++++-
 app/views/activities/order_screen.html.erb    | 22 +++++----
 spec/models/order_spec.rb                     | 48 +++++++++++++++++++
 8 files changed, 105 insertions(+), 29 deletions(-)

diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb
index 4ba6d4323..3d9e69308 100644
--- a/app/controllers/activities_controller.rb
+++ b/app/controllers/activities_controller.rb
@@ -155,12 +155,9 @@ def create_invoices
   def users_hash
     users_credits = User.calculate_credits
     User.active.order(:name).map do |user|
-      hash = user.attributes
-      hash[:minor] = user.minor
-      hash[:insufficient_credit] = user.insufficient_credit
-      hash[:avatar_thumb_or_default_url] = user.avatar_thumb_or_default_url
-      hash[:credit] = users_credits.fetch(user.id, 0)
-      hash
+      user.current_activity = @activity
+      user.as_json(methods: %i[avatar_thumb_or_default_url minor insufficient_credit can_order])
+          .merge(credit: users_credits.fetch(user.id, 0))
     end
   end
 
diff --git a/app/controllers/credit_mutations_controller.rb b/app/controllers/credit_mutations_controller.rb
index bb5b77b2a..7c13db775 100644
--- a/app/controllers/credit_mutations_controller.rb
+++ b/app/controllers/credit_mutations_controller.rb
@@ -19,7 +19,9 @@ def create # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
       if @mutation.save
         NewCreditMutationNotificationJob.perform_later(@mutation) if Rails.env.production? || Rails.env.staging?
         format.html { redirect_to which_redirect?, flash: { success: 'Inleg of mutatie aangemaakt' } }
-        format.json { render json: @mutation, include: { user: { methods: %i[credit avatar_thumb_or_default_url] } } }
+        format.json do
+          render json: @mutation, include: { user: { methods: %i[credit avatar_thumb_or_default_url minor insufficient_credit can_order] } }
+        end
 
       else
         format.html { redirect_to which_redirect?, flash: { error: @mutation.errors.full_messages.join(', ') } }
diff --git a/app/controllers/orders_controller.rb b/app/controllers/orders_controller.rb
index 668de4a60..05aa8c0d3 100644
--- a/app/controllers/orders_controller.rb
+++ b/app/controllers/orders_controller.rb
@@ -15,14 +15,16 @@ def index
     render json: @orders.to_json(proper_json)
   end
 
-  def create
+  def create # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
     @order = Order.new(permitted_attributes.merge(created_by: current_user))
     authorize @order
 
     if @order.save
-      render json: Order.includes(
+      order_data = Order.includes(
         :order_rows, user: { orders: :order_rows }
-      ).find(@order.id).to_json(include: json_includes)
+      ).find(@order.id)
+      order_data.user.current_activity = order_data.activity unless order_data.user.nil?
+      render json: order_data.as_json(include: json_includes)
     else
       render json: @order.errors, status: :unprocessable_entity
     end
@@ -69,7 +71,7 @@ def proper_json
   end
 
   def json_includes
-    { user: { methods: %i[credit avatar_thumb_or_default_url minor insufficient_credit] },
+    { user: { methods: %i[credit avatar_thumb_or_default_url minor insufficient_credit can_order] },
       activity: { only: %i[id title] },
       order_rows: { only: [:id, :product_count, { product: { only: %i[id name credit] } }] } }
   end
diff --git a/app/javascript/packs/order_screen.js b/app/javascript/packs/order_screen.js
index f93367f88..aa5fa5b9e 100644
--- a/app/javascript/packs/order_screen.js
+++ b/app/javascript/packs/order_screen.js
@@ -63,7 +63,10 @@ document.addEventListener('turbolinks:load', () => {
         },
 
         setUser(user = null) {
-          this.orderRows = [];
+          if (this.selectedUser === null || user === null || this.selectedUser.id != user.id) {
+            this.orderRows = [];
+          }
+
           this.payWithCash = false;
           this.payWithPin = false;
           this.selectedUser = user;
@@ -103,11 +106,11 @@ document.addEventListener('turbolinks:load', () => {
           orderRow.amount++;
         },
 
-        maybeConfirmOrder(e) {
-          if (this.selectedUser && this.selectedUser.insufficient_credit) {
-            this.$root.$emit('bv::show::modal', 'insufficient-credit-modal', e.target);
-          } else {
+        maybeConfirmOrder() {
+          if (!this.selectedUser || this.selectedUser.can_order) {
             this.confirmOrder();
+          } else {
+            this.$refs.cannotOrderModal.show();
           }
         },
 
@@ -160,7 +163,6 @@ document.addEventListener('turbolinks:load', () => {
 
             if (user) {
               this.$set(this.users, this.users.indexOf(this.selectedUser), response.body.user);
-              this.$emit('updateusers');
             }
 
             this.sendFlash('Bestelling geplaatst.', additionalInfo, 'success');
@@ -200,9 +202,8 @@ document.addEventListener('turbolinks:load', () => {
             }
           }).then((response) => {
             this.$set(this.users, this.users.indexOf(this.selectedUser), response.body.user);
-            this.$emit('updateusers');
 
-            if(!this.keepUserSelected) {
+            if(!this.keepUserSelected && this.orderRows.length === 0){
               this.setUser(null);
             } else {
               // re-set user to update credit
@@ -266,7 +267,11 @@ document.addEventListener('turbolinks:load', () => {
         },
 
         showOrderWarning() {
-          return this.showInsufficientCreditWarning || this.showAgeWarning;
+          return this.showCannotOrderWarning || this.showInsufficientCreditWarning || this.showAgeWarning;
+        },
+
+        showCannotOrderWarning() {
+          return this.selectedUser && !this.selectedUser.can_order;
         },
 
         showInsufficientCreditWarning() {
diff --git a/app/models/order.rb b/app/models/order.rb
index 7fd7cf47b..9a4318f78 100644
--- a/app/models/order.rb
+++ b/app/models/order.rb
@@ -12,10 +12,15 @@ class Order < ApplicationRecord
   validate :user_or_cash_or_pin
   validate :activity_not_locked
 
+  before_create :can_user_create_order?
   before_destroy -> { throw(:abort) }
 
   scope :orders_for, (->(user) { where(user: user) })
 
+  def can_user_create_order?
+    throw(:abort) unless user.nil? || user.can_order(activity)
+  end
+
   def order_total
     @sum ||= order_rows.sum('product_count * price_per_product')
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 718e20995..a848ea273 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -18,6 +18,8 @@ class User < ApplicationRecord
   scope :inactive, (-> { where(deactivated: true) })
   scope :treasurer, (-> { joins(:roles).merge(Role.treasurer) })
 
+  attr_accessor :current_activity
+
   def credit
     credit_mutations.sum('amount') - order_rows.sum('product_count * price_per_product')
   end
@@ -43,7 +45,16 @@ def minor
   end
 
   def insufficient_credit
-    provider == 'amber_oauth2' and credit < -5
+    provider == 'amber_oauth2' and credit.negative?
+  end
+
+  def can_order(activity = nil)
+    activity ||= current_activity
+    if activity.nil?
+      !insufficient_credit
+    else
+      !insufficient_credit or activity.orders.select { |order| order.user_id == id }.any?
+    end
   end
 
   def treasurer?
diff --git a/app/views/activities/order_screen.html.erb b/app/views/activities/order_screen.html.erb
index 6684431f2..15ef049e8 100644
--- a/app/views/activities/order_screen.html.erb
+++ b/app/views/activities/order_screen.html.erb
@@ -62,16 +62,14 @@
     </template>
   </b-modal>
 
-  <b-modal id="insufficient-credit-modal" ref="creditMutationModal" title="Negatief saldo" @ok="confirmOrder">
+  <b-modal id="cannot-order-modal" ref="cannotOrderModal" title="Negatief saldo" @ok="confirmOrder">
     <div class="modal-body" v-if="selectedUser">
-      {{selectedUser.name}} staat <b class="text-danger">{{doubleToCurrency(-selectedUser.credit)}}</b> negatief! Vraag de gebruiker om zijn of haar saldo op te waarderen via iDEAL of contante inleg.
+      {{selectedUser.name}} staat <b class="text-danger">{{doubleToCurrency(-selectedUser.credit)}}</b> negatief en moet eerst inleggen om te kunnen bestellen!
+      Vraag de gebruiker om zijn of haar saldo op te waarderen via iDEAL of contante inleg.
     </div>
     <template #modal-footer="{ ok, cancel }">
       <button aria-label="Close" class="btn btn-secondary mr-1" @click="cancel()" type="button">
-        Annuleren
-      </button>
-      <button class="btn btn-warning" :disabled="orderConfirmButtonDisabled" @click="ok()" type="button">
-        Bestelling toch plaatsen
+        Oké
       </button>
     </template>
   </b-modal>
@@ -184,7 +182,7 @@
         </div>
 
         <button @click="maybeConfirmOrder" class="btn btn-block btn-lg btn-success"
-              :class="{ 'btn-warning' : showOrderWarning }"
+              :class="{ 'btn-warning' : showOrderWarning && !showCannotOrderWarning, 'btn-danger' : showCannotOrderWarning }"
               :disabled="orderConfirmButtonDisabled"
               v-if="!payWithPin || !<%= @sumup_enabled.to_s %> || !isMobile">
         <span class="text-uppercase">
@@ -192,7 +190,15 @@
         </span>
         <br/>
         <small v-if="showOrderWarning">
-          <span v-if="showInsufficientCreditWarning" class="d-block">
+          <span v-if="showCannotOrderWarning" class="d-block">
+            <span>
+              <%= fa_icon 'exclamation-triangle ' %>
+            </span>
+            <span>
+              {{selectedUser.name}} staat <b>{{doubleToCurrency(-selectedUser.credit)}}</b> negatief en moet eerst inleggen!
+            </span>
+          </span>
+          <span v-else-if="showInsufficientCreditWarning" class="d-block">
             <span>
               <%= fa_icon 'exclamation-triangle ' %>
             </span>
diff --git a/spec/models/order_spec.rb b/spec/models/order_spec.rb
index adc148f39..3a913153b 100644
--- a/spec/models/order_spec.rb
+++ b/spec/models/order_spec.rb
@@ -102,6 +102,54 @@
     it { expect(count.find { |item| item[:category] == 'wine' }[:amount]).to eq 9 }
   end
 
+  describe '#create' do
+    let(:activity) { create(:activity) }
+
+    context 'when no user' do
+      let(:order) { build(:order, paid_with_cash: true, activity: activity) }
+
+      it { expect(order.save).to be true }
+    end
+
+    context 'when non-amber user without credit' do
+      let(:user) { create(:user) }
+      let(:order) { build(:order, user: user, activity: activity) }
+
+      before do
+        create(:credit_mutation, user: user, amount: -1)
+      end
+
+      it { expect(order.save).to be true }
+    end
+
+    context 'when amber user' do
+      let(:user) { create(:user, provider: 'amber_oauth2') }
+      let(:order) { build(:order, user: user, activity: activity) }
+
+      context 'with credit' do
+        # Note that a credit of 0 counts as non-negative credit
+        it { expect(order.save).to be true }
+      end
+
+      context 'without credit with activity order' do
+        before do
+          create(:order, user: user, activity: activity)
+          create(:credit_mutation, user: user, amount: -1)
+        end
+
+        it { expect(order.save).to be true }
+      end
+
+      context 'without credit without activity order' do
+        before do
+          create(:credit_mutation, user: user, amount: -1)
+        end
+
+        it { expect(order.save).to be false }
+      end
+    end
+  end
+
   describe '#destroy' do
     let(:order) { create(:order) }