From f9f60cba17aa7e9730c42e5fb237ac09174f2e89 Mon Sep 17 00:00:00 2001 From: Sylvain Date: Mon, 24 Oct 2022 17:39:16 +0200 Subject: [PATCH] (wip) allow admin to reserve for himself --- .rubocop.yml | 4 +-- CHANGELOG.md | 1 + app/controllers/api/members_controller.rb | 2 +- .../src/javascript/controllers/projects.js | 2 +- .../src/javascript/directives/cart.js | 11 +++---- app/models/accounting_period.rb | 2 +- app/models/user.rb | 21 ++++++------- app/services/members/list_service.rb | 18 +++++------ app/workers/statistics_export_worker.rb | 26 ++++++++-------- config/locales/en.yml | 3 -- db/seeds.rb | 4 +-- lib/stripe/service.rb | 30 ++++++++++--------- lib/tasks/fablab/fix_invoices.rake | 23 +++++++------- lib/tasks/fablab/setup.rake | 25 ++++++++++++++++ 14 files changed, 97 insertions(+), 75 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index c48eca5a3..b99d77cae 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,9 +7,9 @@ Layout/LineLength: Metrics/MethodLength: Max: 35 Metrics/CyclomaticComplexity: - Max: 13 + Max: 14 Metrics/PerceivedComplexity: - Max: 11 + Max: 14 Metrics/AbcSize: Max: 45 Metrics/ClassLength: diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a427c6b6..9d3571826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Fix a bug: no statistics on trainings and spaces reservations - Fix a security issue: updated nokogiri to 1.13.9 to fix [GHSA-2qc6-mcvw-92cw](https://github.com/advisories/GHSA-2qc6-mcvw-92cw) - [TODO DEPLOY] `rails fablab:maintenance:regenerate_statistics[2021,6]` +- [TODO DEPLOY] `rails fablab:setup:set_admins_group` ## v5.4.25 2022 October 19 diff --git a/app/controllers/api/members_controller.rb b/app/controllers/api/members_controller.rb index 12cb86743..fa619106d 100644 --- a/app/controllers/api/members_controller.rb +++ b/app/controllers/api/members_controller.rb @@ -157,7 +157,7 @@ class API::MembersController < API::ApiController end def search - @members = Members::ListService.search(current_user, params[:query], params[:subscription], params[:include_admins]) + @members = Members::ListService.search(current_user, params[:query], params[:subscription]) end def mapping diff --git a/app/frontend/src/javascript/controllers/projects.js b/app/frontend/src/javascript/controllers/projects.js index fbc18796c..0cd5dc523 100644 --- a/app/frontend/src/javascript/controllers/projects.js +++ b/app/frontend/src/javascript/controllers/projects.js @@ -231,7 +231,7 @@ class ProjectsController { const asciiName = Diacritics.remove(nameLookup); Member.search( - { query: asciiName, include_admins: 'true' }, + { query: asciiName }, function (users) { $scope.matchingMembers = users; }, function (error) { console.error(error); } ); diff --git a/app/frontend/src/javascript/directives/cart.js b/app/frontend/src/javascript/directives/cart.js index d21b37618..4e98469cb 100644 --- a/app/frontend/src/javascript/directives/cart.js +++ b/app/frontend/src/javascript/directives/cart.js @@ -291,13 +291,11 @@ Application.Directives.directive('cart', ['$rootScope', '$uibModal', 'dialogs', }; /** - * Check if the currently logged user has the 'admin' role OR the 'manager' role, but is not taking reseravtion for himself + * Check if the currently logged user has the 'admin' OR 'manager' role, but is not taking reseravtion for himself * @returns {boolean} */ $scope.isAuthorized = function () { - if (AuthService.isAuthorized('admin')) return true; - - if (AuthService.isAuthorized('manager')) { + if (AuthService.isAuthorized(['admin', 'manager'])) { return ($rootScope.currentUser.id !== $scope.user.id); } @@ -823,11 +821,10 @@ Application.Directives.directive('cart', ['$rootScope', '$uibModal', 'dialogs', return Wallet.getWalletByUser({ user_id: $scope.user.id }, function (wallet) { const amountToPay = helpers.getAmountToPay($scope.amountTotal, wallet.amount); if ((AuthService.isAuthorized(['member']) && (amountToPay > 0 || (amountToPay === 0 && hasOtherDeadlines()))) || - (AuthService.isAuthorized('manager') && $scope.user.id === $rootScope.currentUser.id && amountToPay > 0)) { + ($scope.user.id === $rootScope.currentUser.id && amountToPay > 0)) { return payOnline(items); } else { - if (AuthService.isAuthorized(['admin']) || - (AuthService.isAuthorized('manager') && $scope.user.id !== $rootScope.currentUser.id) || + if (AuthService.isAuthorized(['admin', 'manager'] && $scope.user.id !== $rootScope.currentUser.id) || (amountToPay === 0 && !hasOtherDeadlines())) { return payOnSite(items); } diff --git a/app/models/accounting_period.rb b/app/models/accounting_period.rb index 0926bd8de..39746142f 100644 --- a/app/models/accounting_period.rb +++ b/app/models/accounting_period.rb @@ -19,7 +19,7 @@ class AccountingPeriod < ApplicationRecord validates_with PeriodOverlapValidator validates_with PeriodIntegrityValidator - belongs_to :user, class_name: 'User', foreign_key: 'closed_by' + belongs_to :user, class_name: 'User', foreign_key: 'closed_by', inverse_of: :accounting_periods def delete false diff --git a/app/models/user.rb b/app/models/user.rb index 94721c2e9..7a0d8ff09 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -43,9 +43,9 @@ class User < ApplicationRecord has_many :exports, dependent: :destroy has_many :imports, dependent: :nullify - has_one :payment_gateway_object, as: :item + has_one :payment_gateway_object, as: :item, dependent: :nullify - has_many :accounting_periods, foreign_key: 'closed_by', dependent: :nullify + has_many :accounting_periods, foreign_key: 'closed_by', dependent: :nullify, inverse_of: :user has_many :proof_of_identity_files, dependent: :destroy has_many :proof_of_identity_refusals, dependent: :destroy @@ -56,14 +56,15 @@ class User < ApplicationRecord end before_create :assign_default_role - after_commit :create_gateway_customer, on: [:create] - after_commit :notify_admin_when_user_is_created, on: :create after_create :init_dependencies after_update :update_invoicing_profile, if: :invoicing_data_was_modified? after_update :update_statistic_profile, if: :statistic_data_was_modified? before_destroy :remove_orphan_drafts + after_commit :create_gateway_customer, on: [:create] + after_commit :notify_admin_when_user_is_created, on: :create attr_accessor :cgu + delegate :first_name, to: :profile delegate :last_name, to: :profile delegate :subscriptions, to: :statistic_profile @@ -116,11 +117,11 @@ class User < ApplicationRecord end def self.online_payers - User.with_any_role(:manager, :member) + User.with_any_role(:admin, :manager, :member) end def self.adminsys - return unless Rails.application.secrets.adminsys_email.present? + return if Rails.application.secrets.adminsys_email.blank? User.find_by('lower(email) = ?', Rails.application.secrets.adminsys_email&.downcase) end @@ -225,7 +226,7 @@ class User < ApplicationRecord def update_statistic_profile raise NoProfileError if statistic_profile.nil? || statistic_profile.id.nil? - statistic_profile.update_attributes( + statistic_profile.update( group_id: group_id, role_id: roles.first.id ) @@ -255,7 +256,7 @@ class User < ApplicationRecord def remove_orphan_drafts orphans = my_projects .joins('LEFT JOIN project_users ON projects.id = project_users.project_id') - .where('project_users.project_id IS NULL') + .where(project_users: { project_id: nil }) .where(state: 'draft') orphans.map(&:destroy!) end @@ -342,7 +343,7 @@ class User < ApplicationRecord def update_invoicing_profile raise NoProfileError if invoicing_profile.nil? - invoicing_profile.update_attributes( + invoicing_profile.update( email: email, first_name: first_name, last_name: last_name @@ -351,7 +352,7 @@ class User < ApplicationRecord def password_complexity return if password.blank? || SecurePassword.is_secured?(password) - + errors.add I18n.t("app.public.common.password_is_too_weak"), I18n.t("app.public.common.password_is_too_weak_explanations") end end diff --git a/app/services/members/list_service.rb b/app/services/members/list_service.rb index 813b27e45..bbf6c5c9f 100644 --- a/app/services/members/list_service.rb +++ b/app/services/members/list_service.rb @@ -28,7 +28,7 @@ class Members::ListService if params[:search].size.positive? @query = @query.where('users.username ILIKE :search OR ' \ 'profiles.first_name ILIKE :search OR ' \ - 'profiles.last_name ILIKE :search OR ' \ + 'profiles.last_name ILIKE :search OR ' \ 'profiles.phone ILIKE :search OR ' \ 'email ILIKE :search OR ' \ 'groups.name ILIKE :search OR ' \ @@ -41,19 +41,19 @@ class Members::ListService @query end - def search(current_user, query, subscription, include_admins = 'false') + def search(current_user, query, subscription) members = User.includes(:profile, :statistic_profile) .joins(:profile, :statistic_profile, :roles, 'LEFT JOIN "subscriptions" ON "subscriptions"."statistic_profile_id" = "statistic_profiles"."id" AND ' \ '"subscriptions"."created_at" = ( ' \ - 'SELECT max("created_at") ' \ - 'FROM "subscriptions" ' \ - 'WHERE "statistic_profile_id" = "statistic_profiles"."id")') + 'SELECT max("created_at") ' \ + 'FROM "subscriptions" ' \ + 'WHERE "statistic_profile_id" = "statistic_profiles"."id")') .where("users.is_active = 'true'") .limit(50) - query.downcase.split(' ').each do |word| + query.downcase.split.each do |word| members = members.where('lower(f_unaccent(users.username)) ~ :search OR ' \ 'lower(f_unaccent(profiles.first_name)) ~ :search OR ' \ 'lower(f_unaccent(profiles.last_name)) ~ :search', @@ -66,13 +66,11 @@ class Members::ListService members = members.where("users.is_allow_contact = 'true'") elsif subscription == 'true' # only admins have the ability to filter by subscription - members = members.where('subscriptions.id IS NOT NULL AND subscriptions.expiration_date >= :now', now: Date.today.to_s) + members = members.where('subscriptions.id IS NOT NULL AND subscriptions.expiration_date >= :now', now: Time.zone.today.to_s) elsif subscription == 'false' - members = members.where('subscriptions.id IS NULL OR subscriptions.expiration_date < :now', now: Date.today.to_s) + members = members.where('subscriptions.id IS NULL OR subscriptions.expiration_date < :now', now: Time.zone.today.to_s) end - members = members.where("roles.name = 'member' OR roles.name = 'manager'") if include_admins == 'false' || include_admins.blank? - members.to_a.filter(&:valid?) end diff --git a/app/workers/statistics_export_worker.rb b/app/workers/statistics_export_worker.rb index f54a26207..724c13878 100644 --- a/app/workers/statistics_export_worker.rb +++ b/app/workers/statistics_export_worker.rb @@ -1,27 +1,29 @@ +# frozen_string_literal: true + +# asynchronously export the statistics to an excel file and send the result by email class StatisticsExportWorker include Sidekiq::Worker def perform(export_id) export = Export.find(export_id) - unless export.user.admin? - raise SecurityError, 'Not allowed to export' - end + raise SecurityError, 'Not allowed to export' unless export.user.admin? - unless export.category == 'statistics' - raise KeyError, 'Wrong worker called' - end + raise KeyError, 'Wrong worker called' unless export.category == 'statistics' service = StatisticsExportService.new method_name = "export_#{export.export_type}" - if %w(account event machine project subscription training space global).include?(export.export_type) and service.respond_to?(method_name) - service.public_send(method_name, export) - - NotificationCenter.call type: :notify_admin_export_complete, - receiver: export.user, - attached_object: export + unless %w[account event machine project subscription training space global].include?(export.export_type) && + service.respond_to?(method_name) + return end + service.public_send(method_name, export) + + NotificationCenter.call type: :notify_admin_export_complete, + receiver: export.user, + attached_object: export + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index d5d937dce..6756c46d7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -453,9 +453,6 @@ en: price_category: reduced_fare: "Reduced fare" reduced_fare_if_you_are_under_25_student_or_unemployed: "Reduced fare if you are under 25, student or unemployed." - group: - #name of the user's group for administrators - admins: 'Administrators' cart_items: free_extension: "Free extension of a subscription, until %{DATE}" statistic_profile: diff --git a/db/seeds.rb b/db/seeds.rb index fc2b7a9fc..2b05f4012 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -82,12 +82,10 @@ if Group.count.zero? ]) end -Group.create! name: I18n.t('group.admins'), slug: 'admins' unless Group.find_by(slug: 'admins') - # Create the default admin if none exists yet if Role.where(name: 'admin').joins(:users).count.zero? admin = User.new(username: 'admin', email: ENV['ADMIN_EMAIL'], password: ENV['ADMIN_PASSWORD'], - password_confirmation: Rails.application.secrets.admin_password, group_id: Group.find_by(slug: 'admins').id, + password_confirmation: Rails.application.secrets.admin_password, group_id: Group.first.id, profile_attributes: { first_name: 'admin', last_name: 'admin', phone: '0123456789' }, statistic_profile_attributes: { gender: true, birthday: Date.current }) admin.add_role 'admin' diff --git a/lib/stripe/service.rb b/lib/stripe/service.rb index d3536f646..fa48f97ef 100644 --- a/lib/stripe/service.rb +++ b/lib/stripe/service.rb @@ -53,15 +53,16 @@ class Stripe::Service < Payment::Service end def create_user(user_id) - StripeWorker.perform_async(:create_stripe_customer, user_id) + StripeWorker.perform_async('create_stripe_customer', user_id) end def create_coupon(coupon_id) coupon = Coupon.find(coupon_id) stp_coupon = { id: coupon.code } - if coupon.type == 'percent_off' + case coupon.type + when 'percent_off' stp_coupon[:percent_off] = coupon.percent_off - elsif coupon.type == stripe_amount('amount_off') + when stripe_amount('amount_off') stp_coupon[:amount_off] = coupon.amount_off stp_coupon[:currency] = Setting.get('stripe_currency') end @@ -97,7 +98,7 @@ class Stripe::Service < Payment::Service if stp_subscription.status == 'canceled' # the subscription was canceled by the gateway => notify & update the status notify_payment_schedule_gateway_canceled(payment_schedule_item) - payment_schedule_item.update_attributes(state: 'gateway_canceled') + payment_schedule_item.update(state: 'gateway_canceled') return end stp_invoice = Stripe::Invoice.retrieve(stp_subscription.latest_invoice, api_key: stripe_key) @@ -107,7 +108,7 @@ class Stripe::Service < Payment::Service payment_method: 'card', payment_id: stp_invoice.payment_intent, payment_type: 'Stripe::PaymentIntent') - payment_schedule_item.update_attributes(state: 'paid', payment_method: 'card') + payment_schedule_item.update(state: 'paid', payment_method: 'card') pgo = PaymentGatewayObject.find_or_initialize_by(item: payment_schedule_item) pgo.gateway_object = stp_invoice pgo.save! @@ -115,29 +116,30 @@ class Stripe::Service < Payment::Service ##### Payment error notify_payment_schedule_item_failed(payment_schedule_item) stp_payment_intent = Stripe::PaymentIntent.retrieve(stp_invoice.payment_intent, api_key: stripe_key) - payment_schedule_item.update_attributes(state: stp_payment_intent.status, - client_secret: stp_payment_intent.client_secret) + payment_schedule_item.update(state: stp_payment_intent.status, + client_secret: stp_payment_intent.client_secret) pgo = PaymentGatewayObject.find_or_initialize_by(item: payment_schedule_item) pgo.gateway_object = stp_invoice pgo.save! elsif stp_invoice.status == 'draft' - return # Could be that the stripe invoice does not yet reflect the payment made by the member, because we called that service just after payment is made. We call return here and PaymentScheduleItemWorker will anyway call that method every hour + # Could be that the stripe invoice does not yet reflect the payment made by the member, because we called that service + # just after payment is made. We just return here and PaymentScheduleItemWorker will anyway call that method every hour else notify_payment_schedule_item_error(payment_schedule_item) - payment_schedule_item.update_attributes(state: 'error') + payment_schedule_item.update(state: 'error') end end def pay_payment_schedule_item(payment_schedule_item) - stripe_key = Setting.get('stripe_secret_key') - stp_invoice = Stripe::Invoice.pay(@payment_schedule_item.payment_gateway_object.gateway_object_id, {}, { api_key: stripe_key }) - PaymentScheduleItemWorker.new.perform(@payment_schedule_item.id) + stripe_key = Setting.get('stripe_secret_key') # TODO, test me + stp_invoice = Stripe::Invoice.pay(payment_schedule_item.payment_gateway_object.gateway_object_id, {}, { api_key: stripe_key }) + PaymentScheduleItemWorker.new.perform(payment_schedule_item.id) { status: stp_invoice.status } rescue Stripe::StripeError => e stripe_key = Setting.get('stripe_secret_key') - stp_invoice = Stripe::Invoice.retrieve(@payment_schedule_item.payment_gateway_object.gateway_object_id, api_key: stripe_key) - PaymentScheduleItemWorker.new.perform(@payment_schedule_item.id) + stp_invoice = Stripe::Invoice.retrieve(payment_schedule_item.payment_gateway_object.gateway_object_id, api_key: stripe_key) + PaymentScheduleItemWorker.new.perform(payment_schedule_item.id) { status: stp_invoice.status, error: e } end diff --git a/lib/tasks/fablab/fix_invoices.rake b/lib/tasks/fablab/fix_invoices.rake index fd503f369..41420dee0 100644 --- a/lib/tasks/fablab/fix_invoices.rake +++ b/lib/tasks/fablab/fix_invoices.rake @@ -29,22 +29,23 @@ namespace :fablab do puts "Date: #{invoice.created_at}" print 'Delete [d], create the missing reservation [c] OR keep as error[e] ? > ' - confirm = STDIN.gets.chomp - if confirm == 'd' + confirm = $stdin.gets.chomp + case confirm + when 'd' puts "Destroying #{invoice.id}..." invoice.destroy - elsif confirm == 'c' + when 'c' if invoice.invoiced_type != 'Reservation' - STDERR.puts "WARNING: Invoice #{invoice.id} is about #{invoice.invoiced_type}. Please handle manually." - STDERR.puts 'Ignoring...' + warn "WARNING: Invoice #{invoice.id} is about #{invoice.invoiced_type}. Please handle manually." + warn 'Ignoring...' next end reservable = find_reservable(ii) if reservable if reservable.is_a? Event - STDERR.puts "WARNING: invoice #{invoice.id} is linked to Event #{reservable.id}. This is unsupported, please handle manually." - STDERR.puts 'Ignoring...' + warn "WARNING: invoice #{invoice.id} is linked to Event #{reservable.id}. This is unsupported, please handle manually." + warn 'Ignoring...' next end reservation = ::Reservation.create!( @@ -55,10 +56,10 @@ namespace :fablab do ) invoice.update_attributes(invoiced: reservation) else - STDERR.puts "WARNING: Unable to guess the reservable for invoice #{invoice.id}, please handle manually." - STDERR.puts 'Ignoring...' + warn "WARNING: Unable to guess the reservable for invoice #{invoice.id}, please handle manually." + warn 'Ignoring...' end - elsif confirm == 'e' + when 'e' invoice.update_attributes(invoiced_type: 'Error') else puts "Operation #{confirm} unknown. Ignoring invoice #{invoice.id}..." @@ -112,7 +113,7 @@ namespace :fablab do availability = reservable.availabilities.where('start_at <= ? AND end_at >= ?', slot[0], slot[1]).first unless availability - STDERR.puts "WARNING: Unable to find an availability for #{reservable.class.name} #{reservable.id}, at #{slot[0]}, creating..." + warn "WARNING: Unable to find an availability for #{reservable.class.name} #{reservable.id}, at #{slot[0]}, creating..." availability = reservable.availabilities.create!(start_at: slot[0], end_at: slot[1]) end availability diff --git a/lib/tasks/fablab/setup.rake b/lib/tasks/fablab/setup.rake index 2a07c1ff8..96889de16 100644 --- a/lib/tasks/fablab/setup.rake +++ b/lib/tasks/fablab/setup.rake @@ -107,5 +107,30 @@ namespace :fablab do setting.save end end + + desc 'migrate administrators to normal groups' + task set_admins_group: :environment do + groups = Group.where.not(slug: 'admins').where(disabled: [false, nil]).order(:id) + User.admins.each do |admin| + print "\e[91m::\e[0m \e[1mMove admin #{admin.profile} to group\e[0m:\n" + admin.update(group_id: select_group(groups)) + PaymentGatewayService.new.create_user(admin.id) + end + print "\e[32m✅\e[0m \e[1mDone\e[0m\n" + end + + def select_group(groups) + groups.each do |g| + print "#{g.id}) #{g.name}\n" + end + print '> ' + group_id = $stdin.gets.chomp + if groups.map(&:id).include?(group_id.to_i) + group_id + else + warn "\e[91m[ ❌ ] Please select a valid group number \e[39m" + select_group(groups) + end + end end end