830210 - [MUSIC]: not able to uncheck song rating in Music application

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Music, defect, P3)

Comment 1

[Leo]

1. Title : Not able to uncheck song ratings in music app 
2. Precondition : video with duration more than one hour should available in phone
3. Tester's Action : User want to give a song zero or no rating 
4. Detailed Symptom (ENG.) : When user wants to give song zero rating or when he wanted to remove rating for a song, user will not able to uncheck (or remove) the rating for a song in Music app. 
5. Detailed Symptom (KOR.) : 
6. Expected : Music player should support to unchecking the rating for a song when user wants to clear the rating of that song.
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
1)Model Comparing : 
9. Attached files: 
1)Log : 
2)Test Contents : 
3)Video file :

Comment 2

[Leo]

1. Title : Not able to uncheck song ratings in music app 
2. Precondition : Rating option should be available for all songs in music app.
3. Tester's Action : User want to give a song zero or no rating 
4. Detailed Symptom (ENG.) : When user wants to give song zero rating or when he wanted to remove rating for a song, user will not able to uncheck (or remove) the rating for a song in Music app. 
5. Detailed Symptom (KOR.) : 
6. Expected : Music player should support to unchecking the rating for a song when user wants to clear the rating of that song.
7.Reproducibility: Y
1)Frequency Rate : 100%
8.Comparison Results : 
1)Model Comparing : 
9. Attached files: 
1)Log : 
2)Test Contents : 
3)Video file :

Comment 3

[Dominic Kuo [:dkuo]]

Music app should be able to rate a song to 0 star... That's a bug, thanks!

Comment 4

[Leo]

Attachment: Pointer to pull request - https://github.com/mozilla-b2g/gaia/pull/8172

Please review the patch at pointer
https://github.com/mozilla-b2g/gaia/pull/8172

Comment 5

[Dominic Kuo [:dkuo]]

I noticed the behaviour of rating songs might need to change if we apply this patch, and I would like to ask some inputs from UX.

What this patch did is unchecking the all stars when the we tap on the first star(if the first star is already checked), it also means that by tapping the first star, the user will feel it just like a toggle, it can be turned on/off.

But for the other four stars, there is no such behaviour like this, we better apply this to all the five stars, and I hope since we are going to fix this, we can also add this behaviour to complete this patch.

Casey, can you please comment this? thanks.

Comment 6

[Casey Yee]

(In reply to Dominic Kuo [:dkuo] from comment #5)
> What this patch did is unchecking the all stars when the we tap on the first
> star(if the first star is already checked), it also means that by tapping
> the first star, the user will feel it just like a toggle, it can be turned
> on/off.

Sounds like a good solution to me.

> 
> But for the other four stars, there is no such behaviour like this, we
> better apply this to all the five stars, and I hope since we are going to
> fix this, we can also add this behaviour to complete this patch.

I think adding the same toggle to all the stars also makes sense to me.

Comment 7

[Casey Yee]

CC Rob and Neo for additional suggestions if they have them.

Comment 8

[Dominic Kuo [:dkuo]]

[e-mail], sorry for the late response, can you update your patch base on comment 5 and comment 6, which are comfirmed by our ux Casey? thanks.

Comment 9

[Dominic Kuo [:dkuo]]

Comment on attachment 715127 [details]
Pointer to pull request - https://github.com/mozilla-b2g/gaia/pull/8172

[e-mail], I am cancelling this review request first to notify you, after this patch is updated, please re-assign to me again, thanks.

Comment 10

[Joe Cheng [:jcheng] (please needinfo)]

triage: leo+ as user cannot make any further changes to rating. seem like a broken features

Comment 11

[Leo]

Attachment: Music rating toggle and check/Uncheck

As per the comments on github, uploaded the patch.
1. Patch also fixes the first star check and uncheck
2. Patch toggles all individual ratings

Comment 12

[Leo]

Attachment: added patch for Music rating along with toggling of each rate

added patch for Music rating along with toggling of each rate

Comment 13

[Leo]

Attachment: patch for Music rating along with toggling of each rate

obsolete

Comment 14

[Dominic Kuo [:dkuo]]

Comment on attachment 735814 [details]
patch for Music rating along with toggling of each rate

[e-mail] The patch looks okay but need to revise, please see my comments on github.

I am cancelling this review request first to notify you, after this patch is updated, please re-assign to me again, thanks.

Comment 15

[Leo]

Hi dkuo,

Refer my comments in github and please suggest .

Thanks,

Comment 16

[Dominic Kuo [:dkuo]]

[e-mail], I have commented my suggestion on github, thanks.

Comment 17

[Leo]

Attachment: Patch for music rating, includes toggling of individual rate icon

Comment 18

[Dominic Kuo [:dkuo]]

Comment on attachment 738916 [details]
Patch for music rating, includes toggling of individual rate icon

The patch looks good to me now. Leo, please see the comments on github and fix the issues I have mentioned before landing it, thanks.

Comment 19

[Leo]

Hi dkuo,

Fixed as per your comments on github.
patch uploaded, Setting need a info to intimate you .

Thanks,

Comment 20

[Dominic Kuo [:dkuo]]

Leo, thanks for updating your patch and you should also rebase you branch then I can merge for you.

Comment 21

[Leo]

Hi Dkuo,

Currently the patch what i have uploaded is on GAIA Master's, Can you please tell me to which branch i should rebase.

Thanks,

Comment 22

[Dominic Kuo [:dkuo]]

Your PR is from leob2g:Bug_830210_Music_rating_fix and it's out-of-date, so you can just rebase that branch, like pull the latest upstream then rebase your local branch, and push back to leob2g:Bug_830210_Music_rating_fix, after that your PR should be automatically updated.

Comment 23

[Leo]

Hi dkuo,

I have rebased, as per your comment.
Please confirm .

Thanks

Comment 24

[Dominic Kuo [:dkuo]]

Comment on attachment 738916 [details]
Patch for music rating, includes toggling of individual rate icon

><!DOCTYPE html><meta charset="utf-8"><meta http-equiv="refresh" content="5;https://github.com/mozilla-b2g/gaia/pull/9480"><title>Bugzilla Code Review</title><p>You can review this patch at <a href="https://github.com/mozilla-b2g/gaia/pull/9480">https://github.com/mozilla-b2g/gaia/pull/9480</a>, or wait 5 seconds to be redirected there automatically.</p>

Comment 25

[Dominic Kuo [:dkuo]]

Attachment: Cherry pick PR from leob2g

Comment 26

[Dominic Kuo [:dkuo]]

Leo, the commit message of attachment 738916 [details] didn't follow the gaia policy, so I cherry picked your commit, refine it and opened a new PR, in attachment 743527 [details].

Comment 27

[Dominic Kuo [:dkuo]]

Landed on master: de39459d28922fe4e6e8385eddf3104d124a72d6

Comment 28

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted de39459d28922fe4e6e8385eddf3104d124a72d6 to:
v1-train: 874217478cd16eb7430fc73cd442735b7c3b726e

Comment 29

[Parul Mathur]

Added test case in MozTrap: 
https://moztrap.mozilla.org/manage/case/8518/