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] merge since, until request parameter with time_range #6251
[Fix] merge since, until request parameter with time_range #6251
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6251 +/- ##
==========================================
+ Coverage 76.85% 77.28% +0.43%
==========================================
Files 47 47
Lines 9393 10645 +1252
==========================================
+ Hits 7219 8227 +1008
- Misses 2174 2418 +244
Continue to review full report at Codecov.
|
@@ -1032,11 +1032,16 @@ def get_form_data(self, slice_id=None, use_slice_data=False): | |||
slc = db.session.query(models.Slice).filter_by(id=slice_id).first() | |||
slice_form_data = slc.form_data.copy() | |||
# allow form_data in request override slice from_data | |||
if 'time_range' in form_data: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment like: we need to do this here so form_data has precedence over slice for time fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
@betodealmeida touched this last, might want an approval from him on this |
bbf71d9
to
edd6781
Compare
LGTM, thanks for fixing this @graceguo-supercat! |
(cherry picked from commit 7d8e321)
When exploring, Superset allow request parameter override slice parameter.
There is an issue that slice has time_range parameter, while request has since/until (or vise verse), they didn't merge correct. Example: annotation request like:
http://localhost:8080/superset/slice_json/240?form_data=%7B%22until%22%3A%222008-10-30T00%3A00%3A00%22%7D
@betodealmeida @fabianmenges @michellethomas @kristw