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

Fix for week_start_sunday and week_ending_saturday #4911

Merged
merged 4 commits into from
May 1, 2018

Conversation

betodealmeida
Copy link
Member

@betodealmeida betodealmeida commented May 1, 2018

The granularity options week_ending_saturday ("truncate forward til next Saturday") and week_start_sunday ("truncate back til last Sunday") are not working correctly. Currently the granularity dropdown has the same key (P1W, one week) for week, week_ending_saturday and week_start_sunday, and only the first one can be selected.

I fixed this bug by making the key unique. The ISO 8601 spec supports time intervals, so I used 1969-12-28T00:00:00Z/P1W to represent week_start_sunday (since 1969-12-28 was a Sunday), and P1W/1970-01-03T00:00:00Z to represent week_ending_saturday (since 1970-01-03 was a Saturday). This required modifying the getPlaySliderParams function, since it parses the ISO duration in order to get its props.

I also added these two granularities to the SQLite backend, in order to test it.

@codecov-io
Copy link

codecov-io commented May 1, 2018

Codecov Report

Merging #4911 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4911   +/-   ##
=======================================
  Coverage   77.14%   77.14%           
=======================================
  Files          44       44           
  Lines        8539     8539           
=======================================
  Hits         6587     6587           
  Misses       1952     1952
Impacted Files Coverage Δ
superset/db_engine_specs.py 52.69% <ø> (ø) ⬆️

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 e1d2150...158e02e. Read the comment docs.

@mistercrunch mistercrunch merged commit 13da5a8 into apache:master May 1, 2018
@mistercrunch mistercrunch deleted the DPTOOLS-559_proper_weeks branch May 1, 2018 20:28
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request May 2, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
hughhhh pushed a commit to lyft/incubator-superset that referenced this pull request May 16, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request May 30, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request May 30, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
timifasubaa pushed a commit to timifasubaa/incubator-superset that referenced this pull request May 31, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jun 4, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
hughhhh pushed a commit to lyft/incubator-superset that referenced this pull request Jun 7, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
hughhhh pushed a commit to lyft/incubator-superset that referenced this pull request Jun 7, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
hughhhh pushed a commit to lyft/incubator-superset that referenced this pull request Jun 7, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jun 7, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
hughhhh pushed a commit to lyft/incubator-superset that referenced this pull request Jun 27, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests

(cherry picked from commit 13da5a8)
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Handle locked weeks

* Fix spelling

* Fix druid

* Clean unit tests
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.25.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.25.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants