812369 - [Apps] Check for sufficient storage before starting app download

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P1)

Description

[Josh Carpenter [:jcarpenter]]

AppInstall-020
As a user, once I confirm an app's installation, if that app requires a download in order to complete installation, I want the system to check that there is sufficient storage to download the necessary assets before initiating download, and warn me if there is not, so that I am informed of the failure to install, understand why, and know how to fix the problem. 

Enabled by: https://bugzilla.mozilla.org/show_bug.cgi?id=809948

This bug covers creating a prompt to notify user once the storage check fails. As per page 8 of the spec, here: https://www.dropbox.com/sh/b0kyykhzcfkpm8b/ReFTX_E72X

Comment 1

[Ben Francis [:benfrancis]]

Fernando, are you working on this?

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

Only on the Gecko side, so far.

Comment 3

[Alex Keybl [:akeybl]]

Marking for C2, given this meets the criteria of known P1/P2 blocking-basecamp+ bugs at the end of C1.

Comment 4

[Julien Wajsberg [:julienw]]

in Bug 809948, it's said there is no need for a gaia patch.

However, as I understand it, we should still display a notice to the user. Or are we already ?

We'll receive a downloaderror event with an error "INSUFFICIENT_STORAGE".

Ben, what do you think about this ?

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Julien Wajsberg [:julienw] [:everlong] from comment #4)
> in Bug 809948, it's said there is no need for a gaia patch.

I meant that there was no need for this patch -> https://bug809948.bugzilla.mozilla.org/attachment.cgi?id=681457

> However, as I understand it, we should still display a notice to the user.

Indeed. We need to display the prompt that Josh mentions in comment 0

Comment 6

[Julien Wajsberg [:julienw]]

Attachment: patch v1

Comment 7

[Julien Wajsberg [:julienw]]

Comment on attachment 685099 [details] [diff] [review]
patch v1

added stas for review as well because of locales changes

Comment 8

[Julien Wajsberg [:julienw]]

Attachment: patch v2

sorry, fixed a very small gjslint issue

Comment 9

[Ben Francis [:benfrancis]]

Comment on attachment 685142 [details] [diff] [review]
patch v2

Review of attachment 685142 [details] [diff] [review]:
-----------------------------------------------------------------

::: apps/system/locales/system.en-US.properties
@@ +76,5 @@
>  stopDownloading=Stop downloading {{ app }}?
>  app-download-can-be-restarted=The download can be restarted later.
>  app-download-stop-button=Stop Download
> +not-enough-space=Not enough space
> +# this is not exactly like the design asked (they wanted two paragraph)

nit: Please remove this comment

::: apps/system/test/unit/mock_modal_dialog.js
@@ +5,5 @@
> +  ],
> +
> +  mPopulate: function mmd_mPopulate() {
> +    this.mMethods.forEach(function(methodName) {
> +      // we could probably put this method outside if we had a closure

nit: please remove this comment

Comment 10

[Ben Francis [:benfrancis]]

r+me with these nits

We may need a follow up to handle other types of errors.

Comment 11

[Julien Wajsberg [:julienw]]

Attachment: patch v3

follow-up to ben's review

Comment 12

[Ben Francis [:benfrancis]]

Comment on attachment 685568 [details] [diff] [review]
patch v3

r+me

Comment 13

[Staś Małolepszy :stas]

Comment on attachment 685568 [details] [diff] [review]
patch v3

Review of attachment 685568 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thanks.

Comment 14

[Julien Wajsberg [:julienw]]

merged https://github.com/mozilla-b2g/gaia/pull/6660

(sorry Stas, I mispelled your first name...)

Comment 15

[Staś Małolepszy :stas]

(In reply to Julien Wajsberg [:julienw] from comment #14)
> (sorry Stas, I mispelled your first name...)

No worries at all.  Maybe I should implement a 'git stas' command ;)  (Also, just to confuse everyone, staś is actually pronounced almost the same way as stash)

Comment 16

[Jason Smith [:jsmith]]

Verified through today's smoke + P1 test case run for app install.