849766 - [music] implement songs picker to support "pick" activity

Status: RESOLVED FIXED

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

Description

[Dominic Kuo [:dkuo]]

For v1.1 features, some apps like email and sms/mms need to pick songs to complete the user stories.

email: bug 838007
mms: bug 840044

so we need songs picker to support both email and mms to attach audio files.

Comment 1

[Dylan Oliver [:doliver]]

Duplicate of bug 836497?

Comment 2

[Dominic Kuo [:dkuo]]

Bug 836497 is for email user story only, and this one is for music app to support general pick activity for every app that needs pick, like sms/mms, and maybe more in the future.

Comment 3

[Maria Angeles Oteo (:oteo)]

Nominating as leo+, as this is blocking a MMS P1 US bug 840044.
Adding [NO-UPLIFT] whiteboard, as it can not be uplifted until the complete MMS functionality is ready.

Comment 4

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

remove no_uplift as this is does not have a major dependency with MMS

Comment 5

[Dominic Kuo [:dkuo]]

Attachment: implement song picker

Steve,

To speed up the developing of MMS features, I have implemented a first version of song picker for you to test when you are working on MMS, this patch should be enough for you to get the file/blob from the song picker via pick activity, let me know if there have any problems, thanks.

Comment 6

[Steve Chung [:steveck]]

This patch works well. CC other mms developers. Thanks

Comment 7

[Julien Wajsberg [:julienw]]

how to move this forward ?

Comment 8

[Dominic Kuo [:dkuo]]

I haven't get a chance to finish this patch and there are still some works to do. The major problem is it take few seconds to display all the songs in music picker, I am trying to make it faster.

Comment 9

[Dietrich Ayala (:dietrich)]

Can you land first, and file a followup bug for perf improvement on it?

Comment 10

[Dominic Kuo [:dkuo]]

Okay, the sounds good to me, I will cleanup that patch and file a perf bug as a followup.

Comment 11

[Tony Chung [:tchung]]

Marcia, this will be landing soon.  can you add a 1.1 test for this in moztrap?  (i dont know where the spec is for this).  thanks!

Comment 12

[Dietrich Ayala (:dietrich)]

Dominic can you please make this a priority to land the final patch today?

Comment 13

[Dominic Kuo [:dkuo]]

Comment on attachment 735670 [details]
implement song picker

David,

I have cleaned up this patch and tried to make it looks better as it is, we need this land to support picking files from MMS since gallery and video were already enabled. You may found that the features of MMS are not completed yet so I have also modified email for you to test music picker, after this patch get a r+, I will remove it before we land it.

Comment 14

[Dominic Kuo [:dkuo]]

(In reply to Dietrich Ayala (:dietrich) from comment #12)
> Dominic can you please make this a priority to land the final patch today?

Just finished cleanup my patch and hope it will not too buggy...

Comment 15

[David Flanagan [:djf]]

Comment on attachment 735670 [details]
implement song picker

Dominic,

I'm giving a tentative r+ because the MMS team really want this landed, even without the perf improvements.

See my comments on github. I think you should at least change the enumerateAll to enumerate() and avoid enumerating the TilesView at the same time as enumerating all song titles.

If you fix those things, go ahead and land, but then please file a followup bug to continue to work on the performance issues.

Comment 16

[Dominic Kuo [:dkuo]]

David,

Thanks for the review, I have revised my patch as your suggestions on github, I will file a new bug to record the performance issue, and I am landing it.

Comment 17

[Dominic Kuo [:dkuo]]

Landed on master: 4617db5dcc927c2fe0e05769db42bab18b59efb5

Comment 18

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

I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 4617db5dcc927c2fe0e05769db42bab18b59efb5
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 19

[Julien Wajsberg [:julienw]]

Dominic, could you please try to uplift yourself ?

Comment 20

[Dominic Kuo [:dkuo]]

This bug needs some changes from the patch of bug 831265(The dependency is already set), which are the conflicts that John has encountered, I have re-request the approval-gaia-v1 flag for bug 831265, if we cannot get approval, I will uplift this myself, thanks.

Comment 22

[Dominic Kuo [:dkuo]]

Conflict resolved.

Uplifted commit 4617db5dcc927c2fe0e05769db42bab18b59efb5 as:
v1-train: fcaa90923894211c19b3544b5cb16eb0db5a6286

Comment 23

[:CSchmoeckel]

Added Music Suite Test Case #8910 - Test that a Music picker activity functions for creating MMS messages.

Comment 24

[Jason Smith [:jsmith]]

The flags here are entirely incorrect. Please contact your QA lead to figure out how bugs should be filed in test runs.

Comment 25

[Jason Smith [:jsmith]]

Switching needinfo to Kevin since I am not getting a response from bmorgan.

Comment 26

[KTucker [:KTucker][Inactive 3/4/2016]]

I spoke to bmorgan and the lead over in SLC to go over the proper procedure once again to ensure this doesn't happen again. They will also be receiving even more training this week.