Ubuntu

[FFE] Add payment preview for music

Reported by Joshua Hoover on 2013-03-12
20
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ayatana Design
Medium
Patricia Panqueva
Music Lens
Undecided
Unassigned
Unity
Medium
Manuel de la Peña
unity (Ubuntu)
Wishlist
Unassigned
Declined for Raring by Scott Kitterman

Bug Description

In order to support payments in the dash, the payment preview for music needs to be included in Unity. While it will initially be used to support in-dash payments of Ubuntu One music store purchases, any developer can use this payment preview.

In order to provide means to test this new feature the unity-lens-scope will be updated so that it uses the PaymentPreview object exposed by libunity that later triggers the use of the PaymentPreview and MusicPaymentPreview within Unity. This means that once those two components are update users will be able to perform purchases within the dash

Ubuntu One QA has tested this feature and the in-dash payments as a whole.

See http://youtu.be/QPjaR9ADyh8 for a short demonstration of the feature.

Steps to test:

Assumption: Ubuntu One client setup on computer already.

 1. Open Dash and click on the Music icon
 2. Search for music you want to purchase and click on the album/track
 3. Click the "Download" button
 4. Click the "Choose Payment Method" link
 5. A browser Window will open and prompt for Payment Preferences
 6. Select "Allow automatic payments" and click the "Update" button
 7. Repeat steps 1-3
 8. Enter your Ubuntu One password when prompted and click the "Purchase" button
 9. Payment will process and eventually show a notification about the track/album downloading

To check that the purchase is on your account you can either:
A. Open the Ubuntu One Control Panel, click the "Explore" button for "Purchased Music" and see that the tracks/album are in that folder

B. Open https://one.ubuntu.com/files/#f=u%2F~%2F.ubuntuone%2FPurchased%2520from%2520Ubuntu%2520One and the tracks/album should be found there

Designed flow:

Assumption: UbuntuOne client setup on computer already.

1. Open Dash and click on the Music Icon
2. Search for music you want to purchase and click on the album/ track
3. Click the ‘Download’ button
4. Click the ‘Choose Payment Method’ link
5. A browser Window will open for ‘Select your preferred payment method’
https://sites.google.com/a/canonical.com/dash/6-checkout/compact-version/4-user-logged-in-valid-payment-method-stored-however-the-user-wishes-to-change-the-payment-method

6. Select the prefered payment method and fill out the form
https://docs.google.com/a/canonical.com/presentation/d/1KjJTXmkEIA1Y5I2sy3b-fWegfmPbBHdsgiMTkjU9qXk/edit#slide=id.gb83e9d0f_17

7. Click the ‘Review order’ Button at the end of the page
8. Review the order
9.Click the ‘Submitted order’ button at the end of the page
10. The ‘confirmation page’ will be presented with the button to ‘play now’ once the purchased music is available to listen to from the cloud player (aprox 30 seconds to download) and the automatic sync to the desktop is starting.
https://docs.google.com/a/canonical.com/presentation/d/1KjJTXmkEIA1Y5I2sy3b-fWegfmPbBHdsgiMTkjU9qXk/edit#slide=id.g6eaeaacf_1_7
11. An OSD notification is prompted in the desktop informing the purchase was successful and the syncing process is starting.
12. An email is sent to the user informing the purchase was successful and containing a link to the purchased music
13.OSD Notifications will show Ubuntu One is syncing the new purchased music and the sync menu will show the progress of the download.
14. Another notification is prompted in desktop once the song is locally available and the download is complete .
15. The user can click on the sync menu to open the song locally in the desktop in the prefered player or on the web player isf the song is not downloaded yet.

If the user is not login into Ubuntu One then they will need to login locally first as it is specify here:
https://docs.google.com/a/canonical.com/drawings/d/1hDcEUXfNaMBUvkiVmzeQ5VUzRE0I7HHABxwZBAp6Ylw/edit

Tags: udp Edit Tag help

Related branches

lp:~mandel/unity/generic-payment-preview
Didier Roche: Approve on 2013-03-22
PS Jenkins bot: Approve (continuous-integration) on 2013-03-21
Nick Dedekind: Approve on 2013-03-21
lp:~mandel/unity/error-preview
PS Jenkins bot: Approve (continuous-integration) on 2013-03-21
Nick Dedekind: Approve on 2013-03-21
lp:~alecu/unity-lens-music/musicstore-purchase
Didier Roche: Approve on 2013-03-22
PS Jenkins bot: Approve (continuous-integration) on 2013-03-22
Michal Hruby: Approve on 2013-03-21
Changed in unity (Ubuntu):
importance: Undecided → High

Why wasn't this landed before feature freeze?

Joshua Hoover (joshuahoover) wrote :

Scott, It wasn't quite ready prior to feature freeze.

Then I think it should wait. We moved feature freeze later with the view that we would be stricter about exceptions. What's the impact of waiting?

Joshua Hoover (joshuahoover) wrote :

Scott, If it doesn't go in Raring, we wait 6 months to get people making purchases via the Dash and lose out on a chance to make serious improvements based on the feedback we get from real world use.

Stefano Rivera (stefanor) wrote :

Against the risk of releasing with something that isn't ready and is very buggy and/or upsets our users.

If you land it early in the s-series, you'll get feedback from real world use, and a chance to get it ship-shape before S releases.

Adolfo Jayme (fitoschido) wrote :

@stefanor: This can't hurt much more than the last cycle's shopping lens fiasco, does it?

It should be clear that this code was ready before FF. It wasn't landed due to some needed code cleanup that couldn't occur before due to personal matters.

However, the code is well tested and really can't cause any regression (since it adds something wasn't possible to do before).

Em 14-03-2013 18:42, Marco Trevisan (Treviño) escreveu:
> However, the code is well tested and really can't cause any regression
> (since it adds something wasn't possible to do before).

Of course it adds a new feature, something that wasn't possible to do
before (that's why it needs a FFE). Anyway, this doesn't mean this new
feature don't have bugs (or, as in bug #1053470, another unity FFE,
privacy concerns).

I don't understand which benefits this new feature brings that overcome
the risk of introducing new bugs, with this 2000+ lines patch, and
justify a FFE?

2013/3/14 Marco Biscaro <email address hidden>:
> Em 14-03-2013 18:42, Marco Trevisan (Treviño) escreveu:
>> However, the code is well tested and really can't cause any regression
>> (since it adds something wasn't possible to do before).
>
> Of course it adds a new feature, something that wasn't possible to do
> before (that's why it needs a FFE).

What I mean is that is not changing code paths that we're currently
using, thus the possibility of having regressions is small.

Dimitri John Ledkov (xnox) wrote :

My understanding was that bug 1153731 ( [FFe] Ubuntu One in the installer step ) is desired together with this FFe.

Scott Kitterman (kitterman) wrote :

This too should wait. Feature freeze is really supposed to be the time this stuff is already landed.

Changed in unity (Ubuntu):
importance: High → Wishlist
milestone: ubuntu-13.04-beta-2 → none
status: New → Won't Fix
Roberto Alsina (ralsina) wrote :

There are a few reasons why this FFE is important.

1) This code, as mentioned before, was ready before FF but it was put on hold because of an urgent one-off project. It is fairly well tested both by my team in online services and by the unity team.

2) Because this code implements the payment previews it has to be part of unity for any other deliverables (via software center, or PPA) that may need it.

3) This can't be placed in a PPA, because it means putting all of unity in one, and makes a horribly invasive PPA

4) This code is not exposed at all. If we merge only this, and deliver the scopes that support purchasing separately, then this code is simply not visible to the user unless he installs that extra scope, and is effectively disabled.

Delivering in this way, with a FFE for this specific change, gives us the flexibility moving forward, while not endangering the user experience in any way.

Please reconsider.

Changed in unity (Ubuntu):
status: Won't Fix → New
Mark Shuttleworth (sabdfl) wrote :

Before considering this for a +1, can it go into the experimental unity7
PPA please?

Roberto Alsina (ralsina) wrote :

@sabdfl working on it immediately.

Didier Roche (didrocks) wrote :

Gave instructions to Robert to have this merged in the ppa and so that we can revert easily if things go wrong.

The good news is that the nux and libunity part were already merged weeks ago, so only Unity (the source) is impacted.

Iain Lane (laney) wrote :

fitoschido, please do not change the status on release team process bugs like this. I'm putting it back to Won't Fix until such a time as the decision is changed through proper process.

Changed in unity (Ubuntu):
status: New → Won't Fix

Em 19-03-2013 07:44, Roberto Alsina escreveu:
> 4) This code is not exposed at all. If we merge only this, and deliver
> the scopes that support purchasing separately, then this code is simply
> not visible to the user unless he installs that extra scope, and is
> effectively disabled.

Does this mean that, even if the FFE is approved, the code will not be
largely tested by users until extra scopes are installed? It looks like
this may cause unnoticed regressions or bugs that will only be noticed
after the final release.

Didier Roche (didrocks) wrote :

FYI: there will be some unity-lens-music changes modified and both should be merged into this FFe so that we can see clearly what's the impact and so on.

I asked yesterday them to:
- give a clear status of how the code is ready
- what components are exactly impacted
- attach them to this bug report
- rebase some on the 100scopes branch (to be able to test in the ppa)
- change the description to tell exactly what behavior/UI is changing

Once this is done, we can merge at once all the necessary components into the ppa, so that if things go badly, we can revert them easily (just one commit).

Roberto Alsina (ralsina) wrote :

@Marco Biscaro: there is a choice here between a non-invasive, limited impact change, and a more extensive and visible one.

Talking with Didier Roche yesterday, we decided we'll propose the branches that expose the new functionality via the music scope. It should be up in a couple of hours or so.

Manuel de la Peña (mandel) wrote :

> FYI: there will be some unity-lens-music changes modified and both should be merged into this FFe so that we can see clearly
> what's the impact and so on.
>
> I asked yesterday them to:
> - give a clear status of how the code is ready

What status would you like, I don't think I fully understand the statement.

> - what components are exactly impacted

The components that are affected are:
 - unity -> Where a new preview is added to allow the scope work with payments.
 - unity-lens-music -> Where the code is update to use the new API found in libunity to perform payments.

> - attach them to this bug report

Already did.

> - rebase some on the 100scopes branch (to be able to test in the ppa)

At least the unity code has already done this, alecu is working on the scope code.

> - change the description to tell exactly what behavior/UI is changing
Already did.

> Once this is done, we can merge at once all the necessary components into the ppa, so that if things go badly, we can revert
> them easily (just one commit).

description: updated
Didier Roche (didrocks) wrote :

re status:
the question was: is the code ready to land for you on those 2 components? Now that both components are indeed proposed for merging against the 100 scopes branches, I'm assuming it's a yes and that you tested that combination. :)

re description:
I still find the description not clear enough, it's just a global "we add payment from the dash". Can you please provide screenshotd and a clear manual test plan about what people should see, what's the difference exactly we should see to ensure the feature is working.

Meanwhile you provide those infos, once we get a first version of 100scopes landing in the certified ppa (meaning tests are passing), I'm happy to merge your 2 branches after upstream reviewed them (poking them right now) and have a test run.

Didier Roche (didrocks) wrote :

Example of why we need a clear story and status on it:
Just saw that lp:~mandel/unity/error-preview was added to this bug. So it means:
- I don't know if the code is ready or not and tested against the 100scopes branches.
- we know have 3 branches instead of the 1 we had in the beginning. 2 being on the same component.

-> Please tell us if:
1. all this was tested against the 100 scopes branches
2. if everything is ready and we won't have another additional branch appearing in the next couple of days
3. merge both unity branches to just make one so that the in dash payment will just be one branch for reviewing per component and will make easier to revert if we don't have the +1 from sabfdl.
(there is still the 4: update the description, see my previous comment)

Until 1, 2, 3 are cleared, I can't mix the 100scopes testing with this one, not knowing if the feature is ready and tested.

Manuel de la Peña (mandel) wrote :

> Example of why we need a clear story and status on it:
> Just saw that lp:~mandel/unity/error-preview was added to this bug. So it means:
> - I don't know if the code is ready or not and tested against the 100scopes branches.
> - we know have 3 branches instead of the 1 we had in the beginning. 2 being on the same component.

The reason to have two different branches against unity is to improve the quality of the reviews. As you can see lp:~mandel/unity/generic-payment-preview is a dependency of lp:~mandel/unity/error-preview . The reason for this course of action is to provide smaller code diff so that reviewers can focus better. I understand that the optimum thing to do is just have a single merge so that can be easily revert. I recommend to do the following, review lp:~mandel/unity/generic-payment-preview approve it when done and DO NOT MERGE it, then review lp:~mandel/unity/error-preview and merge it witch will bring the changes of the previous branch with it, that way we have a win win situation, better reviews and a single merge (once the later branch is merged lp will state that the previous one was too).

> -> Please tell us if:
> 1. all this was tested against the 100 scopes branches

We have create a ppa for the u1 QA team to test all this code with the 100 scopes code, we expect to have everything tested by EOD (time of the comment).

> 2. if everything is ready and we won't have another additional branch appearing in the next couple of days

Yes, all code present in those branches contains all the required changes.

> 3. merge both unity branches to just make one so that the in dash payment will just be one branch for reviewing per
> component and will make easier to revert if we don't have the +1 from sabfdl. (there is still the 4: update the description,
> see my previous comment)

I have answered at the top of the comment.

> Until 1, 2, 3 are cleared, I can't mix the 100scopes testing with this one, not knowing if the feature is ready and tested.

Understandable, we are going as fast as we can regarding the QA.

Didier Roche (didrocks) wrote :

sounds like a perfect plan and matches what we discussed on IRC. Thanks! Keep me posted when we can have the green light (this doesn't block the review though) :)

PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:~unity-team/unity/libunity-7.0-breakage at revision None, scheduled for release in unity, milestone Unknown

Changed in unity:
status: New → Fix Committed
PS Jenkins bot (ps-jenkins) wrote :

Fix committed into lp:~unity-team/unity/libunity-7.0-breakage at revision 3091, scheduled for release in unity, milestone Unknown

description: updated
Didier Roche (didrocks) wrote :

So, FYI, the feature is merged in one commit per project with the 100 scopes feature. It will be easy to revert if we don't get the sabdfl +1.
https://launchpad.net/~ubuntu-unity/+archive/experimental-prevalidation

I ran the integration tests before merging those and then just after. Comparing, it seems we didn't regress anything (but the test coverage for the preview is really limited). Online Services QA did some testing on their branch + 100 scopes to confirm the feature is working.

Running the ppa here didn't seem to trigger regression.

What remains so that this feature can land in raring:
- some explanations on what happens when there is a payment transaction error, like credit card not being valid, network issue…
- sabdfl +1

Manuel de la Peña (mandel) wrote :

> - some explanations on what happens when there is a payment transaction error, like credit card not being valid, network issue…

There are diff ways in witch the error will be shown to the user:

1. A preview with the error that occur will be shown in the dash.
2. A notification will be shown.

The reason to have both is to ensure that even if the user closed the dash (something that he can do at any time) he will be know about the error via the notification. If he does not close the dash both are shown but we considered that is better to be redundant than not being clear.

Joshua Hoover (joshuahoover) wrote :

Update: Manuel is currently taking the branches for in-dash purchases out of the Unity branch (for bug #1154229) and putting it in a PPA where we can do further testing. We've already received some early testing feedback and found some issues that need to be fixed, so Manuel is doing that as well. Once the new PPA is up, we'll update this bug description with the details. We have QA lined up to test the new PPA. It's only after the new PPA is up and tested, that we'll present this to sabdfl.

John Lea (johnlea) on 2013-04-02
Changed in ayatana-design:
assignee: nobody → Patricia Panqueva (pattoin)
description: updated
John Lea (johnlea) on 2013-04-02
description: updated
Changed in ayatana-design:
status: New → Triaged
importance: Undecided → Medium
tags: added: udp
description: updated
Olli Ries (ories) wrote :

after reviewing this feature further and discussing with Mark, we have decided to not push to land it for 13.04 as we ran into an authentication issue and it couldn't be determined in time whether this is a client or server issue. As this feature is dealing with sensitive information (credit card) we are taking our responsibility to not putting users at risk.
The feature will be further QA'd and will land in S as soon as it is deemed ready, we are still discussing whether to provide it via a PPA. An update will be available soon.

Stephen M. Webb (bregma) wrote :

Fix released in Unity 7.1.0

Changed in unity:
status: Fix Committed → Fix Released
Changed in unity:
milestone: none → 7.1.0
assignee: nobody → Manuel de la Peña (mandel)
importance: Undecided → Medium
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers