-
Notifications
You must be signed in to change notification settings - Fork 4k
fix: don't allow setting an invalid rating #22633
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
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #22633 +/- ##
===========================================
- Coverage 62.34% 62.34% -0.01%
===========================================
Files 768 768
Lines 73515 73519 +4
Branches 6332 6332
===========================================
+ Hits 45834 45836 +2
- Misses 24102 24104 +2
Partials 3579 3579
Flags with carried forward coverage won't be shown. Click here to find out more. |
@akhilnarang add one test case covering all 3 cases. Rest lgtm. |
111a80f
to
d6e64e3
Compare
frappe/model/base_document.py
Outdated
@@ -371,6 +371,13 @@ def get_valid_dict( | |||
elif df.fieldtype in float_like_fields and not isinstance(value, float): | |||
value = flt(value) | |||
|
|||
elif df.fieldtype == "Rating": |
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.
It's best to move this to document's _validate
method. get_valid_dict
is serializer function, so it shouldn't be applying business logic here.
Convert anything <0 to 0, and anything >1 to 1 Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
d6e64e3
to
4c93e60
Compare
Convert anything <0 to 0, and anything >1 to 1
Resolves #16245
Before this patch:
After this patch: