-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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(api): return total count on related endpoint #16397
Conversation
superset/views/base_api.py
Outdated
ids = args.get("include_ids") | ||
if page and ids: | ||
# pagination with forced ids is not supported | ||
return self.response_400() |
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.
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.
That's a good idea - will update 👍
Codecov Report
@@ Coverage Diff @@
## master #16397 +/- ##
==========================================
- Coverage 76.64% 76.33% -0.31%
==========================================
Files 1000 1000
Lines 53489 53491 +2
Branches 6816 6818 +2
==========================================
- Hits 40996 40834 -162
- Misses 12257 12419 +162
- Partials 236 238 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Tested in my local environment, LGTM. Thanks for the improvement.
* fix(api): return total count on related endpoint * update response code from 400 to 422
* fix(api): return total count on related endpoint * update response code from 400 to 422
SUMMARY
While reviewing/testing #16144 , I came across a TODO related to the
/related
endpoints not returning total object count, but rather the count of results on the requested page.This PR makes the following changes:
include_ids
is undefined, always return the total number of objects as thecount
parameter as opposed to the actual number of objects inresult
.include_ids
is defined, behavior is unchanged, i.e.count
equals the length ofresult
.include_ids
is defined andpage > 0
, a 400 is returned due to pagination of mixed results not being possible.BEFORE
Currently the
count
parameter in the response is equal to the length of theresult
array (in this example, the total number of objects is in fact two, not just one):AFTER
Now the
count
parameter shows the total number of objects, despite the page size being less:When sending the request with
include_ids
andpage=0
, the result is the same as before (notice howpage_size
only affects the values not included ininclude_ids
):Now requesting other than the first page with defined
include_ids
will result in a 400:BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION