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

always pass valid date format #14296

Merged
merged 2 commits into from
Mar 20, 2017

Conversation

durandom
Copy link
Member

It could be, for whatever reason, that the filter timestamp is in an invalid format (see issue)

This makes sure, we parse the date and always convert it to the valid format of to_s(:db)

fixes #14279

@miq-bot assign @mkanoor
@miq-bot add_labels bug, providers/ansible_tower

@mkanoor
Copy link
Contributor

mkanoor commented Mar 17, 2017

@blomquisg @jameswnl
Please review and merge

@mkanoor mkanoor assigned blomquisg and unassigned mkanoor Mar 17, 2017
@mkanoor
Copy link
Contributor

mkanoor commented Mar 17, 2017

@durandom
Can we add a spec with bogus dates to see if it discards it

@jameswnl
Copy link
Contributor

LGTM. yeah, a test would be nice

@durandom durandom force-pushed the ansible_events_timestamp branch from d3f8c55 to ee7f781 Compare March 17, 2017 18:45
@durandom
Copy link
Member Author

@mkanoor @jameswnl test added

@miq-bot
Copy link
Member

miq-bot commented Mar 17, 2017

This pull request is not mergeable. Please rebase and repush.

@durandom durandom force-pushed the ansible_events_timestamp branch from ee7f781 to f068c83 Compare March 18, 2017 06:34
@miq-bot
Copy link
Member

miq-bot commented Mar 18, 2017

Checked commits durandom/manageiq@8e15432~...f068c83 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. ⭐

@blomquisg blomquisg merged commit bd25694 into ManageIQ:master Mar 20, 2017
@blomquisg blomquisg added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 20, 2017
@durandom durandom deleted the ansible_events_timestamp branch March 21, 2017 10:06
carbonin added a commit to carbonin/manageiq-appliance that referenced this pull request Mar 22, 2017
Because we are proxying requests we don't want to re-encode already
URI encoded values.

Before this change, queries to the API like this:
`api.activity_stream.all(timestamp__gt: '2000-01-01 12:12')`

ended up with URLs like this:
`/ansibleapi/v1/activity_stream?timestamp__gt=2000-01-01+12%3A12`

which were then redirected to a URL which re-encodes the `%` character:
`/ansibleapi/v1/activity_stream/?timestamp__gt=2000-01-01+12%253A12`

This resulted in an invalid date format when un-encoded: `2000-01-01 12%3A12`

ref:
ManageIQ/manageiq#14279
ManageIQ/manageiq#14296
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Embedded Ansible Event Catcher date/time parsing issue
6 participants