Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
schwern opened this issue Jan 22, 2020 · 4 comments · Fixed by #67
Closed

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

schwern opened this issue Jan 22, 2020 · 4 comments · Fixed by #67
Assignees
Labels

Comments

@schwern
Copy link

schwern commented Jan 22, 2020

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
Copy link
Author

schwern commented Jan 22, 2020

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
Copy link
Author

schwern commented Jan 22, 2020

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 workgena added the bug label Jan 23, 2020
@workgena
Copy link
Contributor

workgena commented Jan 23, 2020

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants