-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add date and datetime conversions #56
base: main
Are you sure you want to change the base?
Conversation
I'll fix the reeks. |
We'll need this in query, too: https://github.com/knowndecimal/fulfil/blob/main/lib/fulfil/query.rb#L49-L118 |
Actually, let's talk through this. We're already doing some of this in another spot. |
end | ||
|
||
def disable_escape_html_entities | ||
return unless defined?(ActiveSupport) && ActiveSupport.respond_to?(:escape_html_entities_in_json=) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::DomainParser#disable_escape_html_entities manually dispatches method call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cdmwebs rake test
was erroring out when my guard clause was only return unless defined?(ActiveSupport)
not sure exactly what to do to fix this codeclimate reek.
end | ||
|
||
def enable_escape_html_entities | ||
return unless defined?(ActiveSupport) && ActiveSupport.respond_to?(:escape_html_entities_in_json=) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::DomainParser#enable_escape_html_entities manually dispatches method call
|
||
private | ||
|
||
def date_as_object(date) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::DomainParser#date_as_object doesn't depend on instance state (maybe move it to another class?)
Converter.date_as_object(date) | ||
end | ||
|
||
def datetime_as_object(datetime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::DomainParser#datetime_as_object doesn't depend on instance state (maybe move it to another class?)
Converter.datetime_as_object(datetime) | ||
end | ||
|
||
def disable_escape_html_entities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::DomainParser#disable_escape_html_entities doesn't depend on instance state (maybe move it to another class?)
ActiveSupport.escape_html_entities_in_json = false | ||
end | ||
|
||
def enable_escape_html_entities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::DomainParser#enable_escape_html_entities doesn't depend on instance state (maybe move it to another class?)
end | ||
|
||
def handle_hash(field, key, value, options) | ||
if %i[gte gt lte lt].any? { |op| value.key?(op) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::Query#handle_hash refers to 'value' more than self (maybe move it to another class?)
end | ||
end | ||
|
||
def handle_hash(field, key, value, options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::Query#handle_hash has 4 parameters
end | ||
end | ||
|
||
def handle_hash(field, key, value, options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fulfil::Query#handle_hash has approx 7 statements
Code Climate has analyzed commit 4944ec8 and detected 6 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 94.5% (50% is the threshold). This pull request will bring the total coverage in the repository to 88.8% (1.0% change). View more on Code Climate. |
Figuring out how to fix these codeclimate reeks. I've got dates in the queries. Do those look right @cdmwebs? |
Fulfil requires dates and datetimes to be formatted in an object that contains a iso8601 datetime. This PR adds a conversion class and integrates it into the model to update the domains automatically.