From 0834a4f5fc7687464a8a63982106cca53f77885e Mon Sep 17 00:00:00 2001 From: Matt Levy Date: Fri, 19 Apr 2024 12:26:36 -0700 Subject: [PATCH] Fix create event issues due to empty times in params (#91) * Fix create event issues due to empty times in params Fix create event issue with empty times due to invalid check on hash elements Refactor to put convert for db times functionality into times helper Added test cases and Time validations * rubocop cleanup * Cleanup spec * Refactor test_helper_spec * spec fixes --- app/controllers/events_controller.rb | 12 +-- app/helpers/time_helper.rb | 20 +++- .../controllers/flatpickr_controller.js | 1 + config/initializers/stripe.rb | 17 ++-- spec/controllers/events_controller_spec.rb | 5 +- spec/helpers/time_helper_spec.rb | 93 +++++++++++++++++++ spec/models/user_spec.rb | 2 +- 7 files changed, 126 insertions(+), 24 deletions(-) create mode 100644 spec/helpers/time_helper_spec.rb diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 58c5b740..de4e01a1 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -35,7 +35,7 @@ def edit def create create_params = params_symbolized_hash[:event].dup - convert_event_times_for_db(create_params) + TimeHelper.convert_times_for_db(create_params) @event = Event.new(create_params) @@ -50,7 +50,7 @@ def create def update update_params = params_symbolized_hash[:event].dup - convert_event_times_for_db(update_params) + TimeHelper.convert_times_for_db(update_params) if @event.update(update_params) redirect_to @event, notice: 'The event has been updated.' @@ -124,14 +124,6 @@ def params_symbolized_hash @params_symbolized_hash ||= permitted_params.to_h.tap(&:symbolize_keys!) end - def convert_event_times_for_db(event_hash) - converted_times = {} - event_hash.keys.grep(/_time$/).each do |key| - converted_times[key] = TimeHelper.to_datetime_from_picker(event_hash[key]) - end - event_hash.merge!(converted_times) - end - def completed_ticket_requests TicketRequest .includes(:payment, :user) diff --git a/app/helpers/time_helper.rb b/app/helpers/time_helper.rb index d85f433a..b060da65 100644 --- a/app/helpers/time_helper.rb +++ b/app/helpers/time_helper.rb @@ -5,7 +5,7 @@ module TimeHelper TIME_FORMAT = '%m/%d/%Y, %H:%M %p %Z' - DISPLAY_FORMAT = '%A, %d %B %Y, %h:%M %p' + DISPLAY_FORMAT = '%A, %d %B %Y, %H:%M %p' class << self def for_display(datetime) @@ -15,7 +15,7 @@ def for_display(datetime) end def to_string_for_flatpickr(datetime) - return nil if datetime.nil? + return nil if datetime.blank? datetime.strftime(TIME_FORMAT) if [Date, DateTime, Time, ActiveSupport::TimeWithZone].include?(datetime.class) end @@ -25,5 +25,21 @@ def to_datetime_from_picker(datetime_string) ::DateTime.strptime("#{datetime_string.upcase} #{Time.current.strftime('%z')}Z", TIME_FORMAT).to_time end + + # converts all values in hash to Time where key match '_time' + def convert_times_for_db(in_hash) + return in_hash if in_hash.blank? + + unless in_hash.is_a?(Hash) + raise ArgumentError, "convert_times_for_db expects a Hash argument, got #{in_hash.class.name}" + end + + converted_times = {} + in_hash.keys.grep(/_time$/).each do |key| + converted_times[key] = TimeHelper.to_datetime_from_picker(in_hash[key]) if in_hash[key].present? + end + + in_hash.merge!(converted_times) + end end end diff --git a/app/javascript/controllers/flatpickr_controller.js b/app/javascript/controllers/flatpickr_controller.js index 46f8ba28..0ccc1850 100644 --- a/app/javascript/controllers/flatpickr_controller.js +++ b/app/javascript/controllers/flatpickr_controller.js @@ -12,6 +12,7 @@ export default class flatpickrController extends Controller { } ); + // this isn't in use at the moment flatpickr(".flatpickr-date", { altInput: false, enableTime: false, diff --git a/config/initializers/stripe.rb b/config/initializers/stripe.rb index 1a91c244..48f67054 100644 --- a/config/initializers/stripe.rb +++ b/config/initializers/stripe.rb @@ -1,10 +1,11 @@ # frozen_string_literal: true -require 'stripe' - -TicketBooth::Application.config.x.stripe.secret_key = ENV.fetch('STRIPE_SECRET_KEY', - Rails.application.credentials[Rails.env.to_sym].stripe.secret_api_key) -TicketBooth::Application.config.x.stripe.public_key = ENV.fetch('STRIPE_PUBLIC_KEY', - Rails.application.credentials[Rails.env.to_sym].stripe.publishable_api_key) - -Stripe.api_key = TicketBooth::Application.config.x.stripe.secret_key +# +# require 'stripe' +# +# TicketBooth::Application.config.x.stripe.secret_key = ENV.fetch('STRIPE_SECRET_KEY', +# Rails.application.credentials[Rails.env.to_sym].stripe.secret_api_key) +# TicketBooth::Application.config.x.stripe.public_key = ENV.fetch('STRIPE_PUBLIC_KEY', +# Rails.application.credentials[Rails.env.to_sym].stripe.publishable_api_key) +# +# Stripe.api_key = TicketBooth::Application.config.x.stripe.secret_key diff --git a/spec/controllers/events_controller_spec.rb b/spec/controllers/events_controller_spec.rb index 88b620bd..744c5d1e 100644 --- a/spec/controllers/events_controller_spec.rb +++ b/spec/controllers/events_controller_spec.rb @@ -54,13 +54,12 @@ describe 'POST #create' do let(:new_event) { build(:event) } - let(:new_event_params) do time_hash = {} hash = new_event.attributes hash.keys.grep(/_time/).each do |time_key| - time_hash[time_key.to_sym] = TimeHelper.to_string_for_flatpickr(hash[time_key].to_time) if hash[time_key] + time_hash[time_key.to_sym] = TimeHelper.to_string_for_flatpickr(hash[time_key].to_time) if hash[time_key].present? end hash.merge(time_hash) @@ -77,7 +76,7 @@ expect(new_event.start_time).not_to be_nil end - it 'has a start time thats a proper datetime' do + it 'has a start time that is a proper datetime' do expect(new_event.start_time.to_datetime).to be_a(DateTime) end diff --git a/spec/helpers/time_helper_spec.rb b/spec/helpers/time_helper_spec.rb new file mode 100644 index 00000000..c360b881 --- /dev/null +++ b/spec/helpers/time_helper_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe TimeHelper do + let(:valid_input) { '04/20/2024, 4:20 PM' } + + describe '.to_string_for_flatpickr' do + let(:date) { DateTime.new(2024, 4, 20, 8, 37, 48, -7) } + + it 'convert datetime to a string' do + expect(described_class.to_string_for_flatpickr(date)).to be_a(String) + end + + it 'handles nil' do + expect(described_class.to_string_for_flatpickr(nil)).to be_nil + end + end + + describe '.to_datetime_from_picker' do + let(:out) { described_class.to_datetime_from_picker(valid_input) } + + it 'converts string to Time' do + expect(out).to be_a(Time) + end + + it 'raises date error on empty' do + expect { described_class.to_datetime_from_picker('') }.to raise_error(Date::Error) + end + + it 'raises date error on bad format' do + str = '04/17/2024' + expect { described_class.to_datetime_from_picker(str) }.to raise_error(Date::Error) + end + + it 'returns nil on nil' do + expect(described_class.to_datetime_from_picker(nil)).to be_nil + end + end + + describe '.convert_times_for_db' do + let(:test_hash) { { start_time: valid_input } } + let(:date_epoch_string) { DateTime.parse('2024-04-20T16:20:00 -0700').to_time.to_i } + + describe 'empty aguments' do + subject { described_class.convert_times_for_db(input) } + + describe 'nil' do + let(:input) { nil } + + it { is_expected.to be_nil } + end + + describe 'empty hash' do + let(:input) { {} } + + it { is_expected.to be_empty } + end + end + + describe 'invalid non-empty arguments' do + let(:invalid_input) { '04.17.2024' } + + it 'raises type error with bad type arg' do + expect { described_class.convert_times_for_db(invalid_input) }.to raise_error(ArgumentError) + end + + it 'raises date error on bad format' do + expect { described_class.convert_times_for_db({ start_time: invalid_input }) }.to raise_error(Date::Error) + end + + describe 'empty string for time' do + let(:test_hash) { { start_time: '' } } + + it 'does not convert empty string for time key' do + expect(described_class.convert_times_for_db(test_hash)).to eq(test_hash) + end + end + end + + describe 'valid arguments' do + let(:out_time) { described_class.convert_times_for_db(test_hash)[:start_time] } + + it 'converts time string to Time' do + expect(out_time).to be_a(Time) + end + + it 'converts to a correct time' do + expect(out_time.to_i).to eq(date_epoch_string) + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b2b48ff1..dd78b8cd 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -17,7 +17,7 @@ # first :text # last :text # last_sign_in_at :datetime -# last_sign_in_ip :string`` +# last_sign_in_ip :string # locked_at :datetime # name :string(70) not null # remember_created_at :datetime