Merge pull request #589 from killbill/kaui_4.1

KAUI 4.1 Release

Author: tungleduyxyz

.rubocop.yml

@@ -1,3 +1,9 @@
+require:
+  - rubocop-performance
+  - rubocop-rails
+  - rubocop-rspec
+  - rubocop-thread_safety
+
 inherit_mode:
   merge:
     - Exclude
@@ -52,3 +58,48 @@ Style/Documentation:
 
 Style/EmptyElse:
   EnforcedStyle: empty
+
+# Rails cops
+Rails:
+  Enabled: true
+
+Rails/I18nLocaleTexts:
+  Enabled: false
+
+Rails/SkipsModelValidations:
+  Enabled: false
+
+# RSpec cops
+RSpec:
+  Enabled: true
+
+RSpec/MultipleExpectations:
+  Enabled: false
+
+RSpec/ExampleLength:
+  Enabled: false
+
+# Performance cops
+Performance:
+  Enabled: true
+
+# Thread Safety cops
+ThreadSafety:
+  Enabled: true
+
+ThreadSafety/ClassAndModuleAttributes:
+  Enabled: false
+
+ThreadSafety/DirChdir:
+  Exclude:
+    - 'script/sandbox'
+
+# CLI tool uses exit() for control flow
+Rails/Exit:
+  Exclude:
+    - 'lib/kaui/installer/installer.rb'
+
+# Legacy tests - migration to IntegrationTest is non-trivial
+Rails/ActionControllerTestCase:
+  Exclude:
+    - 'test/functional/**/*'

Gemfile

@@ -24,7 +24,6 @@ group :development do
   gem 'puma'
   gem 'rails-controller-testing'
   gem 'rake'
-  gem 'rubocop'
   gem 'simplecov'
 
   if defined?(JRUBY_VERSION)
@@ -43,6 +42,14 @@ group :development do
   end
 end
 
+group :development, :test do
+  gem 'rubocop', require: false
+  gem 'rubocop-performance', require: false
+  gem 'rubocop-rails', require: false
+  gem 'rubocop-rspec', require: false
+  gem 'rubocop-thread_safety', require: false
+end
+
 # gem 'killbill-assets-ui', github: 'killbill/killbill-assets-ui', ref: 'main'
 # gem 'killbill-assets-ui', path: '../killbill-assets-ui'
 gem 'killbill-assets-ui'

app/controllers/kaui/account_timelines_controller.rb

@@ -76,7 +76,7 @@ def download
         if %w[INVOICE ALL].include?(event_type)
           @invoices.each do |invoice_stub|
             invoice = invoice_stub.invoice_id.present? && @invoices_by_id.key?(invoice_stub.invoice_id) ? @invoices_by_id[invoice_stub.invoice_id] : invoice_stub
-            target_date = invoice.target_date.present? ? invoice.target_date : '[unknown]'
+            target_date = invoice.target_date.presence || '[unknown]'
             bundle_keys = invoice_stub.bundle_keys.present? ? invoice_stub.bundle_keys.split(',').map { |bundle_key| @bundle_names[bundle_key] }.join(', ') : ''
             invoice_details = []
             invoice_details << "Amount: #{invoice.amount_to_money} (#{@account.currency})"
@@ -133,7 +133,7 @@ def download
         end
       end
 
-      send_data csv_string, filename: "account-timelines-#{Date.today}.csv", type: 'text/csv'
+      send_data csv_string, filename: "account-timelines-#{Time.zone.today}.csv", type: 'text/csv'
     end
 
     private
@@ -154,7 +154,7 @@ def load_bundle_name_for_timeline(bundle_key)
     end
 
     def extract_invoices_by_id(all_invoices)
-      return {} if all_invoices.nil? || all_invoices.empty?
+      return {} if all_invoices.blank?
 
       # Convert into Kaui::Invoice to benefit from additional methods xxx_to_money
       @invoices_by_id = all_invoices.to_h do |invoice|

app/controllers/kaui/accounts_controller.rb

@@ -57,7 +57,7 @@ def download
       query_string = params[:search]
 
       if all_fields_checked
-        columns = KillBillClient::Model::AccountAttributes.instance_variable_get('@json_attributes')
+        columns = KillBillClient::Model::AccountAttributes.instance_variable_get(:@json_attributes)
         csv_headers = columns.dup
         Kaui::Account::REMAPPING_FIELDS.each do |k, v|
           index = csv_headers.index(k)
@@ -97,36 +97,7 @@ def download
           csv << data
         end
       end
-      send_data csv_string, filename: "accounts-#{Date.today}.csv", type: 'text/csv'
-    end
-
-    def new
-      @account = Kaui::Account.new
-    end
-
-    def create
-      @account = Kaui::Account.new(params.require(:account).delete_if { |_key, value| value.blank? })
-
-      @account.errors.add(:phone, :invalid_phone) if !@account.phone.nil? && !@account.check_account_details_phone?
-
-      @account.errors.add(:check_account_details_bill_cycle_day_local, :invalid_bill_cycle_day_local) if !@account.bill_cycle_day_local.nil? && !@account.check_account_details_bill_cycle_day_local?
-
-      unless @account.errors.empty?
-        flash.now[:errors] = @account.errors.messages.values.flatten
-        render action: :new and return
-      end
-
-      # Transform "1" into boolean
-      @account.is_migrated = @account.is_migrated == '1'
-
-      begin
-        @account = @account.create(current_user.kb_username, params[:reason], params[:comment], options_for_klient)
-
-        redirect_to account_path(@account.account_id), notice: 'Account was successfully created'
-      rescue StandardError => e
-        flash.now[:error] = "Error while creating account: #{as_string(e)}"
-        render action: :new
-      end
+      send_data csv_string, filename: "accounts-#{Time.zone.today}.csv", type: 'text/csv'
     end
 
     # rubocop:disable Style/MultilineBlockChain
@@ -140,7 +111,7 @@ def show
       fetch_children = promise { @account.children(false, false, 'NONE', cached_options_for_klient) }
       fetch_parent = @account.parent_account_id.nil? ? nil : promise { Kaui::Account.find_by_id(@account.parent_account_id, false, false, cached_options_for_klient) }
       fetch_overdue_state = promise { @account.overdue(cached_options_for_klient) }
-      fetch_account_tags = promise { @account.tags(false, 'NONE', cached_options_for_klient).sort { |tag_a, tag_b| tag_a <=> tag_b } }
+      fetch_account_tags = promise { @account.tags(false, 'NONE', cached_options_for_klient).sort }
       fetch_account_fields = promise { @account.custom_fields('NONE', cached_options_for_klient).sort { |cf_a, cf_b| cf_a.name.downcase <=> cf_b.name.downcase } }
       fetch_account_emails = promise { Kaui::AccountEmail.find_all_sorted_by_account_id(@account.account_id, 'NONE', cached_options_for_klient) }
       fetch_payments = promise { @account.payments(cached_options_for_klient).map! { |payment| Kaui::Payment.build_from_raw_payment(payment) } }
@@ -180,7 +151,7 @@ def show
       @custom_fields = wait(fetch_account_fields)
       @account_emails = wait(fetch_account_emails)
       wait(fetch_payment_methods)
-      @payment_methods = wait(fetch_payment_methods_with_details).map { |pm_f| wait(pm_f) }.compact
+      @payment_methods = wait(fetch_payment_methods_with_details).filter_map { |pm_f| wait(pm_f) }
       @available_tags = wait(fetch_available_tags)
       @children = wait(fetch_children)
       @account_parent = @account.parent_account_id.nil? ? nil : wait(fetch_parent)
@@ -198,6 +169,52 @@ def show
 
       params.permit!
     end
+
+    def new
+      @account = Kaui::Account.new
+    end
+
+    def edit; end
+
+    def create
+      @account = Kaui::Account.new(params.require(:account).permit!.to_h.compact_blank)
+
+      @account.errors.add(:phone, :invalid_phone) if !@account.phone.nil? && !@account.check_account_details_phone?
+
+      @account.errors.add(:check_account_details_bill_cycle_day_local, :invalid_bill_cycle_day_local) if !@account.bill_cycle_day_local.nil? && !@account.check_account_details_bill_cycle_day_local?
+
+      unless @account.errors.empty?
+        flash.now[:errors] = @account.errors.messages.values.flatten
+        render action: :new and return
+      end
+
+      # Transform "1" into boolean
+      @account.is_migrated = @account.is_migrated == '1'
+
+      begin
+        @account = @account.create(current_user.kb_username, params[:reason], params[:comment], options_for_klient)
+
+        redirect_to account_path(@account.account_id), notice: 'Account was successfully created'
+      rescue StandardError => e
+        flash.now[:error] = "Error while creating account: #{as_string(e)}"
+        render action: :new
+      end
+    end
+
+    def update
+      @account = Kaui::Account.new(params.require(:account).permit!.to_h.compact_blank)
+      @account.account_id = params.require(:account_id)
+
+      # Transform "1" into boolean
+      @account.is_migrated = @account.is_migrated == '1'
+
+      @account.update(true, current_user.kb_username, params[:reason], params[:comment], options_for_klient)
+
+      redirect_to account_path(@account.account_id), notice: 'Account successfully updated'
+    rescue StandardError => e
+      flash.now[:error] = "Error while updating account: #{as_string(e)}"
+      render action: :edit
+    end
     # rubocop:enable Style/MultilineBlockChain
 
     def destroy
@@ -261,23 +278,6 @@ def next_invoice_date
       end
     end
 
-    def edit; end
-
-    def update
-      @account = Kaui::Account.new(params.require(:account).delete_if { |_key, value| value.blank? })
-      @account.account_id = params.require(:account_id)
-
-      # Transform "1" into boolean
-      @account.is_migrated = @account.is_migrated == '1'
-
-      @account.update(true, current_user.kb_username, params[:reason], params[:comment], options_for_klient)
-
-      redirect_to account_path(@account.account_id), notice: 'Account successfully updated'
-    rescue StandardError => e
-      flash.now[:error] = "Error while updating account: #{as_string(e)}"
-      render action: :edit
-    end
-
     def set_default_payment_method
       account_id = params.require(:account_id)
       payment_method_id = params.require(:payment_method_id)
@@ -309,7 +309,7 @@ def validate_external_key
     end
 
     def link_to_parent
-      @account = Kaui::Account.new(params.require(:account).delete_if { |_key, value| value.blank? })
+      @account = Kaui::Account.new(params.require(:account).permit!.to_h.compact_blank)
       @account.account_id = params.require(:account_id)
       @account.is_payment_delegated_to_parent = @account.is_payment_delegated_to_parent == '1'
 

app/controllers/kaui/admin_allowed_users_controller.rb

@@ -12,26 +12,50 @@ def index
       end
     end
 
+    def show
+      @allowed_user = Kaui::AllowedUser.find(params.require(:id))
+      raise ActiveRecord::RecordNotFound, "Could not find user #{@allowed_user.id}" unless current_user.root? || @allowed_user.kb_username == current_user.kb_username
+
+      @roles = roles_for_user(@allowed_user)
+
+      tenants_for_current_user = retrieve_tenants_for_current_user
+      @tenants = Kaui::Tenant.all.select { |tenant| tenants_for_current_user.include?(tenant.kb_tenant_id) }
+    end
+
     def new
       @allowed_user = Kaui::AllowedUser.new
       @is_killbill_managed = true
       @roles = []
 
       # Restore form state if returning from role creation
-      return unless params[:user_context].present?
+      return if params[:user_context].blank?
 
       context = params[:user_context]
       @allowed_user.kb_username = context[:kb_username]
       @allowed_user.description = context[:description]
-      @roles = context[:roles].to_s.split(',').reject(&:blank?)
+      @roles = context[:roles].to_s.split(',').compact_blank
       @external_checked = context[:external] == '1'
     end
 
+    def edit
+      @allowed_user = Kaui::AllowedUser.find(params.require(:id))
+      @is_killbill_managed = killbill_managed?(@allowed_user, options_for_klient)
+
+      # Use roles from context if returning from role creation, otherwise fetch from KB
+      if params[:user_context].present?
+        context = params[:user_context]
+        @roles = context[:roles].to_s.split(',').compact_blank
+        @allowed_user.description = context[:description] if context[:description].present?
+      else
+        @roles = roles_for_user(@allowed_user)
+      end
+    end
+
     def create
       @is_killbill_managed = nil
       @allowed_user = Kaui::AllowedUser.new(allowed_user_params)
 
-      existing_user = Kaui::AllowedUser.find_by_kb_username(@allowed_user.kb_username)
+      existing_user = Kaui::AllowedUser.find_by(kb_username: @allowed_user.kb_username)
       if existing_user.blank?
         if params[:external] == '1'
           # Create locally only
@@ -53,37 +77,13 @@ def create
       end
     end
 
-    def show
-      @allowed_user = Kaui::AllowedUser.find(params.require(:id))
-      raise ActiveRecord::RecordNotFound, "Could not find user #{@allowed_user.id}" unless current_user.root? || @allowed_user.kb_username == current_user.kb_username
-
-      @roles = roles_for_user(@allowed_user)
-
-      tenants_for_current_user = retrieve_tenants_for_current_user
-      @tenants = Kaui::Tenant.all.select { |tenant| tenants_for_current_user.include?(tenant.kb_tenant_id) }
-    end
-
-    def edit
-      @allowed_user = Kaui::AllowedUser.find(params.require(:id))
-      @is_killbill_managed = killbill_managed?(@allowed_user, options_for_klient)
-
-      # Use roles from context if returning from role creation, otherwise fetch from KB
-      if params[:user_context].present?
-        context = params[:user_context]
-        @roles = context[:roles].to_s.split(',').reject(&:blank?)
-        @allowed_user.description = context[:description] if context[:description].present?
-      else
-        @roles = roles_for_user(@allowed_user)
-      end
-    end
-
     def update
       @allowed_user = Kaui::AllowedUser.find(params.require(:id))
 
       @allowed_user.description = params[:allowed_user][:description].presence
 
       @allowed_user.update_in_kb!(params[:password].presence,
-                                  params[:roles].blank? ? nil : params[:roles].split(','),
+                                  params[:roles].presence&.split(','),
                                   current_user.kb_username,
                                   params[:reason],
                                   params[:comment],
@@ -149,7 +149,7 @@ def allowed_user_params
     end
 
     def roles_for_user(allowed_user)
-      Kaui::UserRole.find_roles_by_username(allowed_user.kb_username, options_for_klient).map(&:presence).compact
+      Kaui::UserRole.find_roles_by_username(allowed_user.kb_username, options_for_klient).filter_map(&:presence)
     rescue StandardError
       []
     end

app/controllers/kaui/admin_controller.rb

@@ -23,10 +23,10 @@ def index
     def set_clock
       if params[:commit] == 'Submit'
         date = Date.parse(params[:new_date]).strftime('%Y-%m-%d')
-        msg = I18n.translate('flashes.notices.clock_updated_successfully', new_date: date)
+        msg = I18n.t('flashes.notices.clock_updated_successfully', new_date: date)
       else
         date = nil
-        msg = I18n.translate('flashes.notices.clock_reset_successfully')
+        msg = I18n.t('flashes.notices.clock_reset_successfully')
       end
       begin
         Kaui::Admin.set_clock(date, nil, options_for_klient)