Skip to content
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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Add date and datetime conversions #56

wants to merge 15 commits into from

Conversation

swanny85
Copy link
Contributor

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.

@swanny85 swanny85 self-assigned this Dec 21, 2023
lib/fulfil/conversions.rb Outdated Show resolved Hide resolved
lib/fulfil/conversions.rb Outdated Show resolved Hide resolved
lib/fulfil/model.rb Outdated Show resolved Hide resolved
@swanny85
Copy link
Contributor Author

I'll fix the reeks.

@cdmwebs
Copy link
Member

cdmwebs commented Dec 21, 2023

@cdmwebs
Copy link
Member

cdmwebs commented Dec 21, 2023

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=)
Copy link

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

Copy link
Contributor Author

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=)
Copy link

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

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

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

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

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) }
Copy link

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

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

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

Copy link

codeclimate bot commented Jan 4, 2024

Code Climate has analyzed commit 4944ec8 and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 6

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.

@swanny85
Copy link
Contributor Author

swanny85 commented Jan 4, 2024

Figuring out how to fix these codeclimate reeks. I've got dates in the queries. Do those look right @cdmwebs?

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

Successfully merging this pull request may close these issues.

2 participants