Skip to content

fix(rating): retain user-selected value (#24)#136

Merged
harshtandiya merged 1 commit into
developfrom
fix/rating-field-retain-values
May 16, 2026
Merged

fix(rating): retain user-selected value (#24)#136
harshtandiya merged 1 commit into
developfrom
fix/rating-field-retain-values

Conversation

@harshtandiya
Copy link
Copy Markdown
Collaborator

@harshtandiya harshtandiya commented May 16, 2026

Summary

  • frappe-ui Rating works in 1..N integer stars; Frappe stores Rating as a 0..1 fraction and clamps to [0, 1]. Without conversion, clicking N stars persisted 1.0 and on reload only the first star rendered, losing the real selection.
  • New Rating.vue wrapper converts 0..1 <-> 1..5 in both directions; readonly submission view scales the stored fraction to stars.

Closes #24

Test plan

  • New Playwright spec rating-field.spec.ts clicks the 3rd star on a published form and asserts the linked DocType stores 0.6 (red before fix, green after).
  • yarn typecheck
  • yarn lint
  • form-submission and submission-view specs still pass.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added rating field component for form submissions, allowing users to select star ratings.
  • Tests

    • Added end-to-end test suite for rating field functionality, verifying form submission and data storage validation.

Review Change Stack

frappe-ui Rating uses 1..N stars; Frappe stores Rating as a 0..1 fraction
and clamps to [0, 1]. Without conversion, clicking N stars stored 1.0 and
on reload only the first star rendered, losing the real selection.

Wrap frappe-ui Rating in a component that converts 0..1 <-> 1..5 in both
directions, and scale the value in the read-only submission view.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 16, 2026

📝 Walkthrough

Walkthrough

Introduces a custom Rating component wrapper that bridges Frappe's 0–1 fraction storage format with frappe-ui's 1–N integer star display format. Integrates the component into the field registry, updates submission display to apply the conversion, and validates the flow with an E2E test.

Changes

Rating field component and integration

Layer / File(s) Summary
Rating component with value conversion bridge
frontend/src/components/fields/Rating.vue
New Vue component that wraps frappe-ui's FrappeRating. Computes integer star values from a 0..1 modelValue and converts incoming star updates back to 0..1 fractions. Exposes readonly and disabled props and connects them to the underlying component.
Field registry and submission display integration
frontend/src/config/fieldTypes.ts, frontend/src/components/form/submissions/SubmissionFieldValue.vue
Updates field type registry to use the local Rating component. Adjusts submission display to convert stored fractions to integer stars using Math.round((Number(value) || 0) * 5) with rating_from set to 5.
End-to-end test for rating field flow
frontend/e2e/specs/rating-field.spec.ts
Adds Playwright E2E test that creates a form with a Rating field via REST, submits it as guest after clicking the 3rd star, fetches the submission via REST with form filter, and asserts the stored rating is approximately 0.6.

Sequence Diagram

sequenceDiagram
  participant Storage as Frappe Storage
  participant Component as Rating.vue
  participant FrappeUI as FrappeRating
  participant User as User
  
  Storage->>Component: 0.6 (0..1 fraction)
  Component->>FrappeUI: 3 stars (1..5 integer)
  FrappeUI->>User: Display 3 stars
  User->>FrappeUI: Click star
  FrappeUI->>Component: New star value
  Component->>Storage: 0.6 (0..1 fraction)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

⭐ A rating field once broken, now restored,
With fractions and stars in perfect accord,
Zero to one, or one through to five,
The bridge between formats keeps forms alive!
Implemented by BuildWithHussain's Rabbit

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(rating): retain user-selected value (#24)' clearly and concisely describes the main fix—ensuring rating field selections are persisted and retained—which aligns with the core changeset.
Description check ✅ Passed The PR description covers the root cause, solution approach, and test plan; however, it does not explicitly confirm completion of the template checklist items (pre-commit, typecheck, tests).
Linked Issues check ✅ Passed The PR fully addresses issue #24 by implementing bidirectional conversion between frappe-ui's 1..N star format and Frappe's 0..1 fractional storage, adding a Playwright test that verifies the fix, and ensuring values are retained on reload.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the rating field retention bug: a new Rating.vue wrapper component, field type configuration update, test suite, and submission view rendering update—with no unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/rating-field-retain-values

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@harshtandiya harshtandiya enabled auto-merge May 16, 2026 19:08
@harshtandiya harshtandiya added this pull request to the merge queue May 16, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
frontend/src/components/form/submissions/SubmissionFieldValue.vue (1)

103-108: ⚡ Quick win

Eliminate duplication by using the local Rating component.

The conversion logic Math.round((Number(value) || 0) * 5) duplicates the same logic in Rating.vue (line 20). This violates the DRY principle and creates a maintenance burden—if the conversion formula or MAX_STARS changes, both locations must be updated.

The local Rating component already handles the 0..1 → stars conversion internally and supports readonly mode, so you can simplify this code by importing and using it directly.

♻️ Proposed refactor to use the local Rating component

First, update the imports at the top of the file:

-import { Checkbox, Switch, Rating, TextEditor } from "frappe-ui";
+import { Checkbox, Switch, TextEditor } from "frappe-ui";
+import Rating from "`@/components/fields/Rating.vue`";

Then replace the manual conversion with the local component:

 <Rating
     v-else-if="fieldtype === Fieldtype.RATING"
-    :modelValue="Math.round((Number(value) || 0) * 5)"
-    :rating_from="5"
+    :modelValue="value"
     readonly
 />
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/form/submissions/SubmissionFieldValue.vue` around
lines 103 - 108, Replace the duplicated conversion in SubmissionFieldValue.vue
for Fieldtype.RATING by using the local Rating component’s built-in conversion:
import/use the local Rating component (the one whose conversion logic is on line
20 in Rating.vue) and remove the manual Math.round((Number(value) || 0) * 5)
expression; instead pass the raw value (or Number(value) if you prefer explicit
typing) to :modelValue and keep readonly, so the local Rating component handles
0..1 → stars conversion centrally.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@frontend/src/components/form/submissions/SubmissionFieldValue.vue`:
- Around line 103-108: Replace the duplicated conversion in
SubmissionFieldValue.vue for Fieldtype.RATING by using the local Rating
component’s built-in conversion: import/use the local Rating component (the one
whose conversion logic is on line 20 in Rating.vue) and remove the manual
Math.round((Number(value) || 0) * 5) expression; instead pass the raw value (or
Number(value) if you prefer explicit typing) to :modelValue and keep readonly,
so the local Rating component handles 0..1 → stars conversion centrally.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2c474f46-d6b2-4791-9de4-fa54e3e4622f

📥 Commits

Reviewing files that changed from the base of the PR and between 8d43df9 and 03d70c1.

📒 Files selected for processing (4)
  • frontend/e2e/specs/rating-field.spec.ts
  • frontend/src/components/fields/Rating.vue
  • frontend/src/components/form/submissions/SubmissionFieldValue.vue
  • frontend/src/config/fieldTypes.ts

Merged via the queue into develop with commit dd565e7 May 16, 2026
8 checks passed
@harshtandiya harshtandiya deleted the fix/rating-field-retain-values branch May 16, 2026 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: rating fields are broken - doesn't retain values

1 participant