Skip to content

Support date and date_time to clear the value#1108

Merged
adrianthedev merged 2 commits into
avo-hq:mainfrom
rickychilcott:bugfix/allow-setting-datepicker-to-empty-value
Jul 29, 2022
Merged

Support date and date_time to clear the value#1108
adrianthedev merged 2 commits into
avo-hq:mainfrom
rickychilcott:bugfix/allow-setting-datepicker-to-empty-value

Conversation

@rickychilcott

Copy link
Copy Markdown
Contributor

Description

While updating the date/date_time picking implementation, we lost the ability to set the value to empty.

Previously, there was an avo date_field_controller.js exception that was raise that would bail out of the onChange when the input field was empty.

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Manual review steps

  1. Utilize a date or date_time field
  2. Manually clear the input field
  3. Check it works :)

Manual reviewer: please leave a comment with output from the test if that's the case.

@qlty-cloud-legacy

qlty-cloud-legacy Bot commented Jul 28, 2022

Copy link
Copy Markdown

Code Climate has analyzed commit 036ff6c and detected 0 issues on this pull request.

View more on Code Climate.

@codecov

codecov Bot commented Jul 28, 2022

Copy link
Copy Markdown

Codecov Report

Merging #1108 (036ff6c) into main (52b0568) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1108   +/-   ##
=======================================
  Coverage   94.18%   94.18%           
=======================================
  Files         523      523           
  Lines       10388    10388           
=======================================
  Hits         9784     9784           
  Misses        604      604           
Impacted Files Coverage Δ
app/controllers/avo/base_controller.rb 93.33% <100.00%> (ø)
lib/avo/fields/date_time_field.rb 100.00% <100.00%> (ø)
spec/system/avo/default_field_spec.rb 95.12% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52b0568...036ff6c. Read the comment docs.

Comment on lines +257 to +269
nullable_fields = fields
.filter do |field|
field.nullable
end
.map do |field|
[field.id, field.null_values]
end
[field.id, field.null_values]
end
.to_h

params.each do |key, value|
nullable = nullable_fields[key.to_sym]
nullable_values = nullable_fields[key.to_sym]

if nullable.present? && value.in?(nullable)
if nullable_values.present? && value.in?(nullable_values)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor formatting here

@adrianthedev adrianthedev merged commit 433a66a into avo-hq:main Jul 29, 2022
@adrianthedev

Copy link
Copy Markdown
Collaborator

Thank you for the fix @rickychilcott

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 this pull request may close these issues.

2 participants