-
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
feat: Backend Validation for Creation Method #16375
feat: Backend Validation for Creation Method #16375
Conversation
0fa774e
to
cac0d0e
Compare
ff19f19
to
183aeb5
Compare
183aeb5
to
9826aa2
Compare
Codecov Report
@@ Coverage Diff @@
## master #16375 +/- ##
==========================================
- Coverage 76.55% 76.32% -0.24%
==========================================
Files 1000 1000
Lines 53483 53518 +35
Branches 6818 6814 -4
==========================================
- Hits 40945 40846 -99
- Misses 12300 12434 +134
Partials 238 238
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
e0f1436
to
95a7207
Compare
uri = "api/v1/report/" | ||
rv = self.client.post(uri, json=report_schedule_data) | ||
data = json.loads(rv.data.decode("utf-8")) | ||
assert rv.status_code == 422 |
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.
I think returning a 409 (https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/409) makes more sense here. All you need to do is to set the attribute status = 409
in your new exception.
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.
changed
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
131f7db
to
64b2096
Compare
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.
Looks great!
""" | ||
|
||
def __init__(self) -> None: | ||
status = 409 |
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.
Sorry, this should be on the class, not inside __init__
:
class ReportScheduleCreationMethodUniquenessValidationError(ValidationError):
status = 409
def __init__(self) -> None:
super().__init__(...)
64b2096
to
1f4a6f2
Compare
1f4a6f2
to
148c09c
Compare
* backend creation method validation * added tests * Update superset/reports/dao.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/dao.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update tests/integration_tests/reports/api_tests.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update tests/integration_tests/reports/api_tests.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/dao.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/dao.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/commands/create.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/commands/exceptions.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * revisions Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
* backend creation method validation * added tests * Update superset/reports/dao.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/dao.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update tests/integration_tests/reports/api_tests.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update tests/integration_tests/reports/api_tests.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/dao.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/dao.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/commands/create.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * Update superset/reports/commands/exceptions.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> * revisions Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
SUMMARY
This creates backend validation to make sure that a user cannot have multiple reports attached to one chart or dashboard with the respective creation method.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION