search

activeadmin-plugins / active_admin_datetimepicker

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

Fix Input HTML Value Default and Non Date Time Inputs #58

Closed HoyaBoya closed 4 years ago

HoyaBoya commented 5 years ago

This PR does two things that have regressed in active_admin_datetimepicker in 0.7.2.

The first issue is that we could previously pass in integers, String, etc for a model attribute and the date picker will still render. With this change however https://github.com/activeadmin-plugins/active_admin_datetimepicker/blob/master/lib/active_admin_datetimepicker/base.rb#L34, we now get an error that min is not a method on the value. It may seem wonky that we would want to render a date picker for a non date field but that was our use case. We'd massage the value pack to the integer in the AA controller. Perhaps this should be a configuration with an explicit warning or raise, vs attempting to call min when it will fail.

The second seems like a straight up bug, with the input_html: { value: X } no longer being set in the date picker.

This PR attempts to fix both.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-9.7%) to 85.417% when pulling 94065509db41e67e162dd68840cce1d38e451a5f on HoyaBoya:fix_input_html_value into 9b4051bc56caa9ef893a2bd66669c0a50c41ea2d on activeadmin-plugins:master.

workgena commented 5 years ago

Hello :slightly_smiling_face:

The use-case is not clear for me. Can you, please, provide an usage example(AA code), what value do you pass to <input> and how DateTimePicker reacts to it (screenshot is welcome).

filter :starts_at, as: :date_time_range, html_options: { your_use_case_here }

Do you expect the same behavior in forms f.input :starts_at, as: :date_time_picker ?

Than we could collaborate together to write appropriate RSpec test-case, to make sure this regression won't happen again.