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

feat(time_comparison): Support all date formats when computing custom and inherit offsets #30002

Merged
merged 9 commits into from
Aug 23, 2024

Conversation

Antonio-RiveroMartnez
Copy link
Member

SUMMARY

The time comparison feature that's being used in Table and Big Number charts does not support Human Readable dates and it's very limited on what date formats it supports.

Now the API supports sending inherit or a YYYY-MM-DD date as offset value so we can rely on the existing API to parse any date before computing the custom or inherit offset.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before
error

Now
test

TESTING INSTRUCTIONS

  1. Set up a Table Chart with a date filter with value: Aug 2024 until `tomorrow``
  2. Select inherit time shift
  3. The chart must display the correct data and the comparison label

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot bot added change:frontend Requires changing the frontend viz:charts:bignumber Related to BigNumber charts viz:charts:table Related to the Table chart labels Aug 23, 2024
- Support Human readable dates in the label and data call
- Handling inherit and dates as time_offsets in the backend
- Today date must be set to 00:00 so round doesnt affect calculation of the shift
- Use different cache keys based on new offsets supported
- Render label when coming from inherit to custom without error
- Custom date must not set hour to 00:00
- Pylint fix
    - Support other formats when entering dates
- Stop processing offsets in the frontend for custom or inherit and instead use the backend so we avoid limitations on date formats and human readable
- If offset is a predefined one (not custom nor inherit) do not compute the offset sine it's the same value
- No need to add extra date formats when computing the offset now since it is always calle dafter parsing the date
- Mypy fix
ensureIsArray(customOrInheritShifts).includes('inherit')
) {
return fetchTimeRange(filter.comparator, filter.subject).then(res => {
const datePattern = /\d{4}-\d{2}-\d{2}/g;
Copy link
Member

Choose a reason for hiding this comment

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

Can we move it to a const, since we’re using it in many files?

- Add constant we can reuse for our date pattern
Copy link
Member

@kgabryje kgabryje left a comment

Choose a reason for hiding this comment

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

LGTM

@Antonio-RiveroMartnez Antonio-RiveroMartnez merged commit bc6d2db into master Aug 23, 2024
34 checks passed
@rusackas rusackas deleted the time_comparison_zeros branch August 23, 2024 16:38
@michael-s-molina michael-s-molina added review:checkpoint Last PR reviewed during the daily review standup and removed review:checkpoint Last PR reviewed during the daily review standup labels Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change:frontend Requires changing the frontend packages plugins size/L viz:charts:bignumber Related to BigNumber charts viz:charts:table Related to the Table chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants