Closed Bug 849766 Opened 8 years ago Closed 7 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, b2g18 fixed)

RESOLVED FIXED
1.1 CS (11may)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed

People

(Reporter: dkuo, Assigned: dkuo)

References

Details

Attachments

(1 file)

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.
Blocks: 838007, 840044
Duplicate of bug 836497?
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.
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.
blocking-b2g: --- → leo?
Whiteboard: [NO-UPLIFT]
blocking-b2g: leo? → leo+
remove no_uplift as this is does not have a major dependency with MMS
Whiteboard: [NO-UPLIFT]
Depends on: 831265
Attached file 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.
Attachment #735670 - Flags: feedback?(schung)
This patch works well. CC other mms developers. Thanks
Attachment #735670 - Flags: feedback?(schung) → feedback+
how to move this forward ?
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.
Can you land first, and file a followup bug for perf improvement on it?
Okay, the sounds good to me, I will cleanup that patch and file a perf bug as a followup.
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!
Flags: in-moztrap?(mozillamarcia.knous)
Dominic can you please make this a priority to land the final patch today?
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.
Attachment #735670 - Attachment description: WIP - implement song picker → implement song picker
Attachment #735670 - Flags: review?(dflanagan)
(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...
Target Milestone: --- → 1.1 CS (11may)
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.
Attachment #735670 - Flags: review?(dflanagan) → review+
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.
Landed on master: 4617db5dcc927c2fe0e05769db42bab18b59efb5
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Blocks: 868190
Blocks: 868225
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
Dominic, could you please try to uplift yourself ?
Flags: needinfo?(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.
Flags: needinfo?(dkuo)
Duplicate of this bug: 868945
Whiteboard: [NO_UPLIFT]
Conflict resolved.

Uplifted commit 4617db5dcc927c2fe0e05769db42bab18b59efb5 as:
v1-train: fcaa90923894211c19b3544b5cb16eb0db5a6286
Whiteboard: [NO_UPLIFT]
Added Music Suite Test Case #8910 - Test that a Music picker activity functions for creating MMS messages.
Flags: in-moztrap?(mozillamarcia.knous) → in-moztrap+
Whiteboard: qawanted, [2.0-flame-test-run-1]
The flags here are entirely incorrect. Please contact your QA lead to figure out how bugs should be filed in test runs.
Flags: needinfo?(bmorgan)
Whiteboard: qawanted, [2.0-flame-test-run-1]
Switching needinfo to Kevin since I am not getting a response from bmorgan.
Flags: needinfo?(bmorgan) → needinfo?(ktucker)
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.
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.