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

🌵 Show search error if input is not a date #2453

Merged
merged 3 commits into from
Jun 25, 2019
Merged

Conversation

dodobas
Copy link

@dodobas dodobas commented Jun 24, 2019

No description provided.

@codecov
Copy link

codecov bot commented Jun 25, 2019

Codecov Report

Merging #2453 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2453   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         225     225           
  Lines       25426   25434    +8     
======================================
+ Hits        25426   25434    +8
Impacted Files Coverage Δ
temba/contacts/search/parser.py 100% <100%> (ø) ⬆️

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 4dac908...88ca621. Read the comment docs.

@@ -11,13 +11,12 @@
MAX_UTC_OFFSET = 14 * 60 * 60

# pattern for any date which should be parsed by the ISO8601 library (assumed to be not human-entered)
FULL_ISO8601_REGEX = regex.compile(r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.(\d{,9}))?([\+\-]\d{2}:\d{2}|Z)$")
FULL_ISO8601_REGEX = regex.compile(r"^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}(\.(\d{,9}))?([\+\-]\d{2}:\d{2}|Z)$")
Copy link
Author

Choose a reason for hiding this comment

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

added <space> as a delimiter between date and time

DD_MM_YYYY = regex.compile(r"\b([0-9]{1,2})[-.\\/_ ]([0-9]{1,2})[-.\\/_ ]([0-9]{4}|[0-9]{2})\b")
MM_DD_YYYY = regex.compile(r"\b([0-9]{1,2})[-.\\/_ ]([0-9]{1,2})[-.\\/_ ]([0-9]{4}|[0-9]{2})\b")
YYYY_MM_DD = regex.compile(r"\b([0-9]{4}|[0-9]{2})[-.\\/_ ]([0-9]{1,2})[-.\\/_ ]([0-9]{1,2})\b")
DD_MM_YYYY = regex.compile(r"\b([0-9]{1,2})[-.\\/_]([0-9]{1,2})[-.\\/_]([0-9]{4}|[0-9]{2})\b")
Copy link
Author

Choose a reason for hiding this comment

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

removed <space> as a delimiter, i've only seen spaces used when date has full/abbr month like: 20th June, 2019 or 13. Dec 2019, we don't support those

# maybe it is similar to iso date ?
if not parsed:
if dayfirst:
parsed = _date_from_formats(date_str, YYYY_MM_DD, 3, 2, 1)
Copy link
Author

Choose a reason for hiding this comment

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

YYYY_MM_DD is similar to ISO date, it supports single digit month/day but requires a 4 digit year, and works with multiple delimiters

this format was probably used in the past, but support was perhaps dropped because it was clashing with ISO_YYYY_MM_DD

@dodobas dodobas changed the title WIP Show search error if input is not a date 🌵 Show search error if input is not a date Jun 25, 2019
@dodobas
Copy link
Author

dodobas commented Jun 25, 2019

I've checked dynamic groups on both, RP and TX, that use a contactfield (value_type='D') for any weird date input strings, and there are no weird looking inputs. there is about 35 contactgroups in total

here is the SQL i've used to check the contactgroups:

select cf.key, cg.query

from contacts_contactgroup cg join contacts_contactgroup_query_fields cgqf ON cg.id = cgqf.contactgroup_id JOIN contacts_contactfield cf on cgqf.contactfield_id = cf.id

where cg.query is not null and cg.is_active=true and cf.is_active=True and cf.value_type='D';

@@ -41,6 +40,37 @@ def datetime_to_str(date_obj, format, tz):
return date_obj.strftime(format)


def str_to_date(date_str, dayfirst=True):
Copy link
Collaborator

Choose a reason for hiding this comment

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

All looks good, how about a verbose comment for this method that describes in prose what dates we accept and what our logic is there? Just to document for future us what we mean to support.

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.

3 participants