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

WIP: fix(time_picker): Fix incorrect time range display #17816

Closed
wants to merge 5 commits into from
Closed

WIP: fix(time_picker): Fix incorrect time range display #17816

wants to merge 5 commits into from

Conversation

CodeingBoy
Copy link
Contributor

@CodeingBoy CodeingBoy commented Dec 19, 2021

SUMMARY

This fixes #16462. It will correct time range display from (begin, end) to [begin, end)

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE(copied from issue):
image

AFTER:
image

TESTING INSTRUCTIONS

Just create a dashboard with a time range filter and confirm time range is [begin, end)

ADDITIONAL INFORMATION

  • Has associated issue: Fixes [time_picker]start date display is not inclusive #16462
  • 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

@CodeingBoy CodeingBoy changed the title [WIP]fix(time_picker): Fix incorrect time range display fix(time_picker): Fix incorrect time range display Dec 19, 2021
@codecov
Copy link

codecov bot commented Dec 19, 2021

Codecov Report

Merging #17816 (f575477) into master (8345eb4) will increase coverage by 0.06%.
The diff coverage is 66.66%.

❗ Current head f575477 differs from pull request most recent head e6ea1bb. Consider uploading reports for the commit e6ea1bb to get more accurate results

@@            Coverage Diff             @@
##           master   #17816      +/-   ##
==========================================
+ Coverage   66.67%   66.73%   +0.06%     
==========================================
  Files        1738     1587     -151     
  Lines       65078    63954    -1124     
  Branches     6885     6864      -21     
==========================================
- Hits        43391    42682     -709     
+ Misses      19939    19408     -531     
- Partials     1748     1864     +116     
Flag Coverage Δ
javascript 53.77% <66.66%> (+2.09%) ⬆️
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...nts/controls/DateFilterControl/DateFilterLabel.tsx 57.52% <50.00%> (+16.10%) ⬆️
superset-frontend/src/explore/constants.ts 92.15% <100.00%> (-7.85%) ⬇️
...t-frontend/src/dashboard/containers/SliceAdder.jsx 0.00% <0.00%> (-100.00%) ⬇️
...in-chart-echarts/src/MixedTimeseries/buildQuery.ts 0.00% <0.00%> (-100.00%) ⬇️
...ntend/src/explore/components/ExploreChartPanel.jsx 14.66% <0.00%> (-59.53%) ⬇️
...gins/legacy-plugin-chart-rose/src/controlPanel.tsx 50.00% <0.00%> (-50.00%) ⬇️
...ns/legacy-plugin-chart-horizon/src/controlPanel.ts 50.00% <0.00%> (-50.00%) ⬇️
...ns/legacy-plugin-chart-treemap/src/controlPanel.ts 50.00% <0.00%> (-50.00%) ⬇️
...egacy-plugin-chart-pivot-table/src/controlPanel.ts 50.00% <0.00%> (-50.00%) ⬇️
...acy-plugin-chart-paired-t-test/src/controlPanel.ts 50.00% <0.00%> (-50.00%) ⬇️
... and 1240 more

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 8345eb4...e6ea1bb. Read the comment docs.

@CodeingBoy
Copy link
Contributor Author

Hi @zhaoyongjie, I had marked last review as resolved since I had replied there. Please consider merging this commit.

@CodeingBoy CodeingBoy changed the title fix(time_picker): Fix incorrect time range display WIP: fix(time_picker): Fix incorrect time range display Mar 18, 2022
@CodeingBoy
Copy link
Contributor Author

UPDATE: master had modified superset-frontend/src/explore/components/controls/DateFilterControl/DateFilterLabel.tsx which causes conflicts, working on resolution.

@CodeingBoy CodeingBoy changed the title WIP: fix(time_picker): Fix incorrect time range display fix(time_picker): Fix incorrect time range display Jun 13, 2022
@CodeingBoy CodeingBoy changed the title fix(time_picker): Fix incorrect time range display WIP: fix(time_picker): Fix incorrect time range display Jun 13, 2022
@CodeingBoy
Copy link
Contributor Author

Since #18936 had removed SIP-15 related logics, including this endpoint config. I should recheck if this bug still exists and may create another issue.

@CodeingBoy
Copy link
Contributor Author

This bug was fixed in version 1.5.1, closing this PR.

@CodeingBoy CodeingBoy closed this Jun 13, 2022
@CodeingBoy CodeingBoy deleted the time-range-bugfix branch June 14, 2022 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[time_picker]start date display is not inclusive
2 participants