Closed Bug 1039501 Opened 7 years ago Closed 6 years ago

[User Story] [FOTA] WiFi prioritized for downloading

Categories

(Firefox OS Graveyard :: Gaia, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-v2.2 fixed)

VERIFIED FIXED
2.1 S6 (10oct)
Tracking Status
b2g-v2.2 --- fixed

People

(Reporter: sonmarce, Assigned: mancas)

References

Details

(Whiteboard: [systemsfe])

User Story

As a user I want software update of the device preferably be done via WiFi, so that I save money not having to use 3G data

Acceptance criteria:

AC1: The OEM will be able to configure this parameter so they can customize it according to each country needs:
  - Whether downloads over WiFi are prioritized: Possible values are OFF (downloads over WiFI are NOT prioritized) or ON (downloads over WiFI are prioritised).
AC2: In case this parameter is not configured, the default value will be set to ON (downloads over WiFI are prioritised).
AC3: Before starting a download process, if the parameter is set to ON and the user is using 3G connection, in the confirmation dialog for download he will be informed that he should better switch for a WiFi connection to do the download. If despite that, user still wants to continue, a dialog informing that he might incur in additional costs by using 3G will be shown.
AC4: If the user is connected via WiFi (regardless of the value of the configuration parameter) or that parameter is set to OFF, the user will just need to confirm the download process and no information about the network technology used or extra charges will be shown to him.

Attachments

(7 files, 11 obsolete files)

58.17 KB, image/png
Details
29.10 KB, image/png
jelee
: ui-review+
Details
681.18 KB, application/pdf
Details
41.42 KB, image/png
Details
34.98 KB, image/png
Details
29.92 KB, image/png
Details
46 bytes, text/x-github-pull-request
etienne
: review+
Details | Review
See description of User Story field
User Story: (updated)
Whiteboard: [systemsfe]
Assignee: nobody → b.mcb
User Story: (updated)
User Story: (updated)
User Story: (updated)
User Story: (updated)
User Story: (updated)
User Story: (updated)
Summary: [User Story] [FOTA] WiFi required or prioritized for downloading (just 3G if not updated for some time) → [User Story] [FOTA] WiFi required or prioritized for downloading
Summary: [User Story] [FOTA] WiFi required or prioritized for downloading → [User Story] [FOTA] WiFi prioritized for downloading
Hi Bruce, can you review the AC of this US? Thanks a lot
Flags: needinfo?(bhuang)
Looks good to me, we should probably also add the case where downloaded is started and the device moves to 3G.  I'm guessing we trigger something similar to AC3.
Flags: needinfo?(bhuang)
Attached file Wifi prioritized (obsolete) —
This patch is a first approach. We still waiting for UX
Attachment #8475029 - Flags: review?(timdream)
Comment on attachment 8475029 [details] [review]
Wifi prioritized

After looking at the patch I think it's non-trivial so it's better we ask for the dedicated peer for review, unless Etienne have hands over that part of system app to someone else ...

I also think we should get the UX review done first before code review, since any behavior change will definitely introduce some code change.
Attachment #8475029 - Flags: review?(timdream) → review?(etienne)
Comment on attachment 8475029 [details] [review]
Wifi prioritized

We're still waiting for the UX so this is just general feedback.

But can we keep the logic in the UpdateManager (as opposed to the SystemUpdatable)?
Attachment #8475029 - Flags: review?(etienne)
Attached file UX Spec. OTA. 2G/Wi-Fi (obsolete) —
Hi Jenny, I upload here the ux spec for the "WiFi prioritized for downloading" and "Not allow updates via 2G" bugs. Could you please take a look to it and correct me if I have missed something?

On the other hand, Matej, could you please check the wording please?

Thanks!
Flags: needinfo?(jelee)
Flags: needinfo?(Mnovak)
Here are my revisions and comments:

> Unable to start the download. Please, connect via Wi-Fi or 3G and try to download it again.

Small changes here:

Unable to start download. Please connect via Wi-Fi or 3G and try again.


> You are currently connected via 3G. Do you want to switch to Wi-Fi to start the download?

This one looks good.


> Required system update (%SizeUnit) available. Download now?

It seems odd to tell users that this is a required update, but then still give them the option. "Required" makes me think my phone won't work unless I do it. If that's not the case, can we remove that word?


> By using 3G, you might incur in additional costs. Do you want to continue?

Change to:

When using 3G, additional costs may apply. Do you want to continue?
Flags: needinfo?(Mnovak)
(In reply to Matej Novak [:matej] from comment #7)

> > Required system update (%SizeUnit) available. Download now?
 
> It seems odd to tell users that this is a required update, but then still
> give them the option. "Required" makes me think my phone won't work unless I
> do it. If that's not the case, can we remove that word?

Jenny, I took it from your ux spec. What do you think about that?
Hi all, just want to clarify before moving on to review the spec, as far as I know, we are already showing warning message whenever user is trying to download update via data connection (see attached), so maybe what we need here is the flexibility to set "WiFi prioritized" on or off? When set to on, simply follow the current behavior, when set to off, don't show warning message.
Flags: needinfo?(jelee)
Now I know there's a message that covers that, I'm ok with it. I'll make the changes on the spec to match that, then.

On the other hand, what about the wording regarding the "Download confirm" string Matej said in comment#7?
Flags: needinfo?(jelee)
(In reply to Pau Masiá [:Pau] from comment #10)
> Now I know there's a message that covers that, I'm ok with it. I'll make the
> changes on the spec to match that, then.
> 
Does that mean you will also follow the current behavior: show warning dialog (as attached in comment 9) whenever user is trying to download update via data connection (3G, 2G etc)? As far as I know, user can still download update via 2G, so the condition for dialog on p.6 should be "when the device is not able to access network". 
Also, the follow up action that allows user to switch to Wi-Fi seems unnecessary to me. On p.7, when user taps on "yes (switch to Wi-Fi)", what happens next? It is not possible to ensure the device can automatically connect to Wi-Fi successfully every time. Ex. user can be at a new environment that he has to set up a connection from scratch manually. I'd say we can simply warn user about data connection is currently in use, and stop there (like our current behavior). Hope that makes sense :)       

> On the other hand, what about the wording regarding the "Download confirm"
> string Matej said in comment#7?

Yes I'm ok with removing "Required". Thanks!
Flags: needinfo?(jelee)
Attached file UX Spec. OTA. 2G/Wi-Fi (obsolete) —
Attachment #8478974 - Attachment is obsolete: true
(In reply to Jenny Lee from comment #11)
> (In reply to Pau Masiá [:Pau] from comment #10)
> > Now I know there's a message that covers that, I'm ok with it. I'll make the
> > changes on the spec to match that, then.
> > 
> Does that mean you will also follow the current behavior: show warning
> dialog (as attached in comment 9) whenever user is trying to download update
> via data connection (3G, 2G etc)? As far as I know, user can still download
> update via 2G, so the condition for dialog on p.6 should be "when the device
> is not able to access network". 
> Also, the follow up action that allows user to switch to Wi-Fi seems
> unnecessary to me. On p.7, when user taps on "yes (switch to Wi-Fi)", what
> happens next? It is not possible to ensure the device can automatically
> connect to Wi-Fi successfully every time. Ex. user can be at a new
> environment that he has to set up a connection from scratch manually. I'd
> say we can simply warn user about data connection is currently in use, and
> stop there (like our current behavior). Hope that makes sense :)       
> 
> > On the other hand, what about the wording regarding the "Download confirm"
> > string Matej said in comment#7?
> 
> Yes I'm ok with removing "Required". Thanks!

Hi Jenny, I think there has been a misunderstanding regarding this issue. What I said was that I would apply the changes to the spec for sake of consistency grouping similar scenarios to make it easier.

That is why I have updated the spec. Take a look at it, please :)

Thanks!
Flags: needinfo?(jelee)
Attached file UX Spec. OTA. 2G/Wi-Fi (obsolete) —
Sorry, this is the good one :)
Attachment #8481190 - Attachment is obsolete: true
(In reply to Pau Masiá [:Pau] from comment #14)
> Created attachment 8481198 [details]
> UX Spec. OTA. 2G/Wi-Fi
> 
> Sorry, this is the good one :)

Hi Pau,
p.6 - As mentioned in comment 11, user can still download update via 2G, so I'm not sure what is the purpose here. Do you have special requirement that forbids user from downloading via 2G?
p.7, p.8 - are IMO basically the same thing, can consider combining them and change the title. One thing I want to clarify though, in current implementation, the data connection warning dialog will appear *whenever* user tries to download via data connection, no use of parameter to decide whether or not to show the dialog. So, is the parameter something you want to add to current design?
p.9 - Looks good!

Thanks!
Flags: needinfo?(jelee)
(In reply to Jenny Lee from comment #15)

> Hi Pau,
> p.6 - As mentioned in comment 11, user can still download update via 2G, so
> I'm not sure what is the purpose here. Do you have special requirement that
> forbids user from downloading via 2G?

Hi Jenny, the requirements for that are included in the Acceptance Criteria of the User Story in bug 1039509,  [User Story] [FOTA] Not allow updates via 2G. This parameter for allowing or not the 2G download is configured by the OEM in build time. Although it's already implemented, I asked Pau to include this corresponding interaction design in the spec so we can have a complete picture of both functionalities.


> p.7, p.8 - are IMO basically the same thing, can consider combining them and
> change the title. One thing I want to clarify though, in current
> implementation, the data connection warning dialog will appear *whenever*
> user tries to download via data connection, no use of parameter to decide
> whether or not to show the dialog. So, is the parameter something you want
> to add to current design?

As I told before the WIFI and 2G parameters are parameters configured in build time so we are including them in these implementations but they can not be accessed and changed directly by the users.
Is it more clear now?

Thanks a lot, Jenny!
Flags: needinfo?(jelee)
(In reply to Maria Angeles Oteo (:oteo) from comment #16)
> (In reply to Jenny Lee from comment #15)
> 
> > Hi Pau,
> > p.6 - As mentioned in comment 11, user can still download update via 2G, so
> > I'm not sure what is the purpose here. Do you have special requirement that
> > forbids user from downloading via 2G?
> 
> Hi Jenny, the requirements for that are included in the Acceptance Criteria
> of the User Story in bug 1039509,  [User Story] [FOTA] Not allow updates via
> 2G. This parameter for allowing or not the 2G download is configured by the
> OEM in build time. Although it's already implemented, I asked Pau to include
> this corresponding interaction design in the spec so we can have a complete
> picture of both functionalities.
> 
> 
ok! So we have two sets of parameter, one for Wi-Fi prioritize and one for allowing 2G download? In that case there should be 4 outcomes: 
1. Wi-Fi prioritize ON + 2G update ok (covered in spec)
2. Wi-Fi prioritize OFF + 2G update ok (covered in spec)
3. Wi-Fi prioritize ON + 2G update Not ok and device is connected to 2G (needs more specification, first prompt user the Wi-Fi prioritize dialog, if user choose to download, prompt user the 2G not allowed dialog)
4. Wi-Fi prioritize OFF + 2G update Not ok and device is connected to 2G (covered in spec)

Thanks Maria!
Flags: needinfo?(jelee)
Attached file UX Spec. OTA. 2G/Wi-Fi (obsolete) —
Here I attach the new version of the spec already updated according to the comments above.

Thanks.
Attachment #8481198 - Attachment is obsolete: true
Flags: needinfo?(jelee)
Hi Pau, just one thing, your index is missing two pages. Thanks!
Flags: needinfo?(jelee)
Attached file UX Spec. OTA. 2G/Wi-Fi (obsolete) —
Little mistake fixed ;)! Thanks.
Attachment #8489275 - Attachment is obsolete: true
Comment on attachment 8475029 [details] [review]
Wifi prioritized

Hey etienne! Take a look at the PR please.

Thanks!
Attachment #8475029 - Flags: review?(etienne)
Comment on attachment 8475029 [details] [review]
Wifi prioritized

Comments on github, sorry to be picky but the logic here is pretty complex so we need to take great care of the readability.
Attachment #8475029 - Flags: review?(etienne)
Comment on attachment 8475029 [details] [review]
Wifi prioritized

I've taken care of your comments in github

Thanks!
Attachment #8475029 - Flags: review?(etienne)
Comment on attachment 8475029 [details] [review]
Wifi prioritized

the code change is a-okay! Thanks!
(a few comments on github but nothing blocking).

Please attach screenshots of the new dialogs so they can be ui-reviewed and we might also want to get QA to give this a spin before landing since it's a rather critical part of the system.
Attachment #8475029 - Flags: review?(etienne) → review+
Comment on attachment 8475029 [details] [review]
Wifi prioritized

Hey etienne! I've made some changes to retrieve the value from the update2g setting as you said in github, so take a quick look at the pr.

Thanks!
Flags: needinfo?(etienne)
Attached image wifi_prioritized.png (obsolete) —
Attachment #8492875 - Flags: ui-review?(jelee)
Attached image roaming.png
Attachment #8492876 - Flags: ui-review?(jelee)
Attached image forbidden.png (obsolete) —
Attachment #8492897 - Flags: ui-review?(jelee)
Attached file UX Spec. OTA. 2G/Wi-Fi
Spec updated. Page 9. Thanks!
Attachment #8489376 - Attachment is obsolete: true
(In reply to Manuel Casas Barrado [:mancas] from comment #25)
> Comment on attachment 8475029 [details] [review]
> Wifi prioritized
> 
> Hey etienne! I've made some changes to retrieve the value from the update2g
> setting as you said in github, so take a quick look at the pr.
> 
> Thanks!

Made a small comment, but it's already much more readable than before your patch, thank you for that ! :)

Johan, before you get overloaded with a *lot* of new stuffs, could you help us make sure everything looks good QA-wise here? ;)
Flags: needinfo?(etienne) → needinfo?(jlorenzo)
Attached image wifi_prioritized_using_3g.png (obsolete) —
Attachment #8493020 - Flags: ui-review?(jelee)
Sorry for this but the spec has changed so I've had to make some changes in the code. Please take a look when possible.

Thanks!
Flags: needinfo?(etienne)
(In reply to Manuel Casas Barrado [:mancas] from comment #32)
> Sorry for this but the spec has changed so I've had to make some changes in
> the code. Please take a look when possible.
> 

Looks good, please use the review flag next time I'll get to it quicker :)
Flags: needinfo?(etienne)
Attachment #8493020 - Flags: ui-review?(b.pmm)
Attachment #8492897 - Flags: ui-review?(b.pmm)
Attachment #8492876 - Flags: ui-review?(b.pmm)
Attachment #8492875 - Flags: ui-review?(b.pmm)
Attachment #8492876 - Flags: ui-review?(jelee) → ui-review+
Comment on attachment 8493020 [details]
wifi_prioritized_using_3g.png

Missing a period "." at the end, otherwise looks good, tks
Attachment #8493020 - Flags: ui-review?(jelee) → ui-review+
Comment on attachment 8492897 [details]
forbidden.png

Missing a period "." at the end, otherwise looks good, tks
Attachment #8492897 - Flags: ui-review?(jelee) → ui-review+
Comment on attachment 8492875 [details]
wifi_prioritized.png

Missing a period "." at the end, otherwise looks good, tks
Attachment #8492875 - Flags: ui-review?(jelee) → ui-review+
Comment on attachment 8493020 [details]
wifi_prioritized_using_3g.png

As Jenny says, there's a period "." missing at the end. Otherwise, ui+
Attachment #8493020 - Flags: ui-review?(b.pmm) → ui-review+
Comment on attachment 8492897 [details]
forbidden.png

Remember the missing period "." at the end.
Attachment #8492897 - Flags: ui-review?(b.pmm) → ui-review+
Attachment #8492876 - Flags: ui-review?(b.pmm) → ui-review+
Attached image forbidden.png (obsolete) —
Attachment #8492897 - Attachment is obsolete: true
Attachment #8495102 - Flags: ui-review?(b.pmm)
Attached image wifi_prioritized.png
Attachment #8492875 - Attachment is obsolete: true
Attachment #8492875 - Flags: ui-review?(b.pmm)
Attachment #8495103 - Flags: ui-review?(b.pmm)
Attachment #8493020 - Attachment is obsolete: true
Attachment #8495104 - Flags: ui-review?(b.pmm)
Comment on attachment 8495102 [details]
forbidden.png

The button shape should be blue.
Attachment #8495102 - Flags: ui-review?(b.pmm) → ui-review-
Attachment #8495103 - Flags: ui-review?(b.pmm) → ui-review+
Attachment #8495104 - Flags: ui-review?(b.pmm) → ui-review+
Attached image forbidden.png (obsolete) —
Attachment #8495102 - Attachment is obsolete: true
Attachment #8495143 - Flags: ui-review?(b.pmm)
Attachment #8495143 - Flags: ui-review?(b.pmm) → ui-review+
Comment on attachment 8475029 [details] [review]
Wifi prioritized

Hey Etienne!

This is embarrassing. One of the buttons of the UI should be blue so I've made some changes in the CustomDialog to make it work as expected. Please take a look at the PR since it's a shared script.
Attachment #8475029 - Flags: review+ → review?(etienne)
(In reply to Manuel Casas Barrado [:mancas] from comment #44)
> Comment on attachment 8475029 [details] [review]
> Wifi prioritized
> 
> Hey Etienne!
> 
> This is embarrassing. One of the buttons of the UI should be blue so I've
> made some changes in the CustomDialog to make it work as expected. Please
> take a look at the PR since it's a shared script.

Here are the lines you need to check:
https://github.com/mozilla-b2g/gaia/pull/23025/files#diff-100b2adc2865565a4857427242f33fb9R158

Thanks!
(In reply to Etienne Segonzac (:etienne) from comment #30)
I'm not sure to have time to focus on new features now. Let's say if I haven't looked at it by Tuesday, I'll transfer the ni to Marcia (the current QA contact for SystemsFE).
(In reply to Johan Lorenzo [:jlorenzo] from comment #46)
> (In reply to Etienne Segonzac (:etienne) from comment #30)
> I'm not sure to have time to focus on new features now. Let's say if I
> haven't looked at it by Tuesday, I'll transfer the ni to Marcia (the current
> QA contact for SystemsFE).

Johan, Loli can do the testing of the feature (as this was requested by Telefonica team), we were going to do it anyway as it raised in the certification process.
(In reply to Maria Angeles Oteo (:oteo) from comment #47)
> Johan, Loli can do the testing of the feature (as this was requested by
> Telefonica team), we were going to do it anyway as it raised in the
> certification process.

Let's ask Loli then.
Flags: needinfo?(jlorenzo) → needinfo?(lolimartinezcr)
(In reply to Johan Lorenzo [:jlorenzo] from comment #48)
> (In reply to Maria Angeles Oteo (:oteo) from comment #47)
> > Johan, Loli can do the testing of the feature (as this was requested by
> > Telefonica team), we were going to do it anyway as it raised in the
> > certification process.
> 
> Let's ask Loli then.

Yes I test it!
Flags: needinfo?(lolimartinezcr)
Attachment #8475029 - Flags: review?(etienne) → review+
Attached image forbidden.png
Attachment #8495143 - Attachment is obsolete: true
Attachment #8495887 - Flags: ui-review?(b.pmm)
Attachment #8495887 - Flags: ui-review?(b.pmm) → ui-review+
Attached file Wifi prioritized
Etienne, the PR has been closed due to tree stability issues as you should know. Please take a quick look at this new PR in order to give it r+.

Thanks!
Attachment #8475029 - Attachment is obsolete: true
Attachment #8498708 - Flags: review?(etienne)
Comment on attachment 8498708 [details] [review]
Wifi prioritized

for cases like this one where the patch doesn't change but you're just creating a new attachment you can "carry the r+" and mark the new attachment as r+ yourself (mentioning that you're carrying it over)
Attachment #8498708 - Flags: review?(etienne) → review+
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/e58fe34a63a85f51da0d7e35d3b6af290ba0662b
Status: NEW → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S6 (10oct)
Blocks: 1079215
No longer blocks: 1079215
Depends on: 1079215
Depends on: 1079218
No longer depends on: 1079215
Depends on: 1079215, 1079244
I have tested all scenarios with conection 3G, 2G and wifi with 'update.2g.enabled' with true or false and with 'app.update.wifi-prioritized' with true or false values.

I *can't* tested roaming scenarios, because I haven't SIM card with data connection and coverage.
I have tested it with Flame master (Gecko 437abd2 Gaia a5b7208). 

I have tested with a spanish SiM card (no roaming) and with a german SIM card (roaming scenario), in both scenarios all the cases (3G and 2G connection, wifi or without any data connection) have worked properly.
(In reply to Juanjo Iglesias from comment #55)
> I have tested it with Flame master (Gecko 437abd2 Gaia a5b7208). 
> 
> I have tested with a spanish SiM card (no roaming) and with a german SIM
> card (roaming scenario), in both scenarios all the cases (3G and 2G
> connection, wifi or without any data connection) have worked properly.

For this reason, I change status to Verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.