activeadmin-plugins / active_admin_datetimepicker

:calendar: active_admin_datetimepicker gem
MIT License
72 stars 48 forks source link

"To" being set to "From" after submit. #65

Closed schwern closed 4 years ago

schwern commented 4 years ago

We encountered a problem where the date time range filter would show up with both fields having the placeholder "From". After submission the the lteq field would be set to the gteq field.

Here's the filter. start is a datetime column.

  filter :start,
    as: :date_time_range,
    label: "Date range"

Showing the From/From.

Screen Shot 2020-01-22 at 13 12 39

Different values filled in.

Screen Shot 2020-01-22 at 13 12 54

After submission, To is set to From, but the original inputs were understood.

Screen Shot 2020-01-22 at 13 13 09

Upon investigation we noticed the IDs are q_field_gteq and q_field_lteq for a Date rather than q_field_gteq_datetimepicker and q_field_lteq_datetimepicker for a DateTime.

I believe this is because ActiveAdmin::Inputs::Filters::DateTimeRangeInput inherits ActiveAdmin::Inputs::Filters::DateRangeInput#input_html_options which casts the value to a Date. This was added a few years ago.

This results in DateTimeRangeInput#gt_input_name and DateTimeRangeInput#lt_input_name both using the name from their superclass DateRangeInput.

The following monkey patch fixes the issue for us.

class ActiveAdmin::Inputs::Filters::DateTimeRangeInput
  def input_html_options_for(input_name, placeholder)
    current_value = begin
                      #cast value to Time object before rendering input
                      @object.public_send(input_name).to_s.to_datetime
                    rescue
                      nil
                    end

    return input_html_options.merge(
      placeholder: placeholder,
      value: current_value&.strftime("%Y-%m-%d %H:%M:%S")
    )
  end
end

Here's our relevant Gemfile.lock.

    active_admin_datetimepicker (0.7.3)
      activeadmin (>= 1.1, < 3.a)
      coffee-rails
      xdan-datetimepicker-rails (~> 2.5.4)
    active_admin_scoped_collection_actions (0.4.0)
      activeadmin (>= 1.1, < 3.a)
    activeadmin (2.6.0)
      arbre (~> 1.2, >= 1.2.1)
      formtastic (~> 3.1)
      formtastic_i18n (~> 0.4)
      inherited_resources (~> 1.7)
      jquery-rails (~> 4.2)
      kaminari (~> 1.0, >= 1.0.1)
      railties (>= 5.2, < 6.1)
      ransack (~> 2.1, >= 2.1.1)
      sassc-rails (~> 2.1)
      sprockets (>= 3.0, < 4.1)
    activeadmin-ajax_filter (0.4.4)
      activeadmin (>= 1.0)
      coffee-rails (>= 4.1.0)
      has_scope (>= 0.6.0)
      rails (>= 4.2.11)
      selectize-rails (>= 0.12.6)
schwern commented 4 years ago

In addition, DateRangeInput#input_html_options_for was merging with input_html_options backwards. If input_html_options had a value or placeholder (even if nil), the merge would wipe out the the placeholder and value from input_html_options_for.

So it's input_html_options.merge(new_options) not new_options.merge(input_html_options) to ensure the new placeholder and value are set.

schwern commented 4 years ago

And finally, it has to be converted to a format which includes the time. current_value&.strftime("%Y-%m-%d %H:%M") vs current_value.strftime("%Y-%m-%d").

workgena commented 4 years ago

Hello @schwern

Thanks 🙏 for detailed 🔬 explanation of an issue. Also your config/initializers/init_datetimepicker.rb might be useful.

I'll take a look at this issue this/next weak.

PS: Since you use DateTime, I would like to mention about timezone. The plugin does not know about difference of user(Web) and database time-zones. So you might check(logs/development.log) if the SQL query is what you actually expect.

workgena commented 4 years ago

Hi @schwern the problem has been fixed with new gem release 0.7.4

The problem arose because ActiveAdmin 2.6 changed the superclass DateRangeInput.