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: allows hard delete for feedbacks #6154
fix: allows hard delete for feedbacks #6154
Conversation
9cf17a1
to
e69c0a3
Compare
Codecov Report
@@ Coverage Diff @@
## development #6154 +/- ##
===============================================
- Coverage 66.29% 66.26% -0.04%
===============================================
Files 286 287 +1
Lines 14415 14430 +15
===============================================
+ Hits 9557 9562 +5
- Misses 4858 4868 +10
Continue to review full report at Codecov.
|
e69c0a3
to
0c81cd6
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.
This looks fine.
@@ -23,7 +23,7 @@ class Meta: | |||
inflect = dasherize | |||
|
|||
id = fields.Str(dump_only=True) | |||
rating = fields.Float(required=True, validate=Range(min=0, max=5)) | |||
rating = fields.Float(required=True, validate=Range(min=1, max=5)) |
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.
This behaviour of frontend deleting the record when setting the rating to 0 is wrong IMO. Delete action should be very explicit. Either show a dialog on frontend that setting rating to 0 will delete the feedback
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.
FE is using ui-rating from semantic UI. If a user selects rating 2 suppose and wants to clear out the rating, he/she needs to click on 2 stars again and the rating is cleared (set to 0). I think showing a dialog box will be better.
@CosmicCoder96 your opinion?
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.
@iamareebjamal please review this PR again. The FE PR is still WIP so I can include your suggestions there
f08f20c
f08f20c
to
a47970a
Compare
a47970a
to
bee7562
Compare
updates login and includes logger
bee7562
to
ad788ac
Compare
Fixes #6153
Short description of what this resolves:
Currently there is hard delete for feedback.
Changes proposed in this pull request:
0-5
to1-5
for ratingChecklist
development
branch.