Skip to content

Commit

Permalink
fix: don't allow setting an invalid rating (#22633) (#25432)
Browse files Browse the repository at this point in the history
* feat: don't allow setting an invalid rating

Convert anything <0 to 0, and anything >1 to 1

Signed-off-by: Akhil Narang <me@akhilnarang.dev>

* chore: add in tests for rating

Signed-off-by: Akhil Narang <me@akhilnarang.dev>

---------

Signed-off-by: Akhil Narang <me@akhilnarang.dev>
(cherry picked from commit b601131)

Co-authored-by: Akhil Narang <me@akhilnarang.dev>
  • Loading branch information
mergify[bot] and akhilnarang committed Mar 14, 2024
1 parent 0eaa472 commit 502b907
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 0 deletions.
10 changes: 10 additions & 0 deletions frappe/model/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ def _validate(self):
self._validate_selects()
self._validate_non_negative()
self._validate_length()
self._fix_rating_value()
self._validate_code_fields()
self._sync_autoname_field()
self._extract_images_from_text_editor()
Expand Down Expand Up @@ -586,6 +587,15 @@ def get_msg(df):
msg = get_msg(df)
frappe.throw(msg, frappe.NonNegativeError, title=_("Negative Value"))

def _fix_rating_value(self):
for field in self.meta.get("fields", {"fieldtype": "Rating"}):
value = self.get(field.fieldname)
if not isinstance(value, float):
value = flt(value)

# Make sure rating is between 0 and 1
self.set(field.fieldname, max(0, min(value, 1)))

def validate_workflow(self):
"""Validate if the workflow transition is valid"""
if frappe.flags.in_install == "frappe":
Expand Down
29 changes: 29 additions & 0 deletions frappe/tests/test_rating.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import frappe
from frappe.core.doctype.doctype.test_doctype import new_doctype
from frappe.tests.utils import FrappeTestCase


class TestRating(FrappeTestCase):
def setUp(self):
doc = new_doctype(
fields=[
{
"fieldname": "rating",
"fieldtype": "Rating",
"label": "rating",
"reqd": 1, # mandatory
},
],
)
doc.insert()
self.doctype_name = doc.name

def test_negative_rating(self):
doc = frappe.get_doc(doctype=self.doctype_name, rating=-1)
doc.insert()
self.assertEqual(doc.rating, 0)

def test_positive_rating(self):
doc = frappe.get_doc(doctype=self.doctype_name, rating=5)
doc.insert()
self.assertEqual(doc.rating, 1)

0 comments on commit 502b907

Please sign in to comment.