1007588 - [Cost Control] Implement an IAC mechanism to warn other applications about sending a SMS for requiring balance or making a SMS top-up

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Cost Control, defect)

Description

[Salvador de la Puente González [:salva]]

Cost Control application can broadcast via IAC a message saying it is about to send a balance query or a top-up via SMS in order to allow SMS applications to react accordingly upon these special cases.

Comment 1

[Marina Rodríguez [:mai]]

Attachment: patch v1.0

Comment 2

[Salvador de la Puente González [:salva]]

Comment on attachment 8428551 [details] [review]
patch v1.0

You've my comments on GitHub.

Comment 3

[Salvador de la Puente González [:salva]]

Comment on attachment 8428551 [details] [review]
patch v1.0

I did not want to give you the r-, just ask for my review for the second version.

Comment 4

[Julien Wajsberg [:julienw]]

I found 2 issues in this patch while working on the SMS patch for this:

Issue 1.
* when sending the balance/topup SMS fails, we don't send an IAC to disable the blacklist

Issue 2.
* when receiving a message, we send the IAC request to enable the blacklist. The issue is that it prevents the notification to be displayed for this message if it's part of the "senders" array, while this patch was exactly to make this possible. Maybe I'm wrong, but I think the IAC request should be sent only when cost control send a message to topup or get the balance. If it needs to send a message at this moment, it should do it after a timeout to allow the SMS to be received correctly.

Comment 5

[Salvador de la Puente González [:salva]]

(In reply to Julien Wajsberg [:julienw] from comment #4)
> I found 2 issues in this patch while working on the SMS patch for this:
> 
> Issue 1.
> * when sending the balance/topup SMS fails, we don't send an IAC to disable
> the blacklist

You're right, we need to contemplate this case.

> 
> Issue 2.
> * when receiving a message, we send the IAC request to enable the blacklist.
> The issue is that it prevents the notification to be displayed for this
> message if it's part of the "senders" array, while this patch was exactly to
> make this possible. Maybe I'm wrong, but I think the IAC request should be
> sent only when cost control send a message to topup or get the balance. If
> it needs to send a message at this moment, it should do it after a timeout
> to allow the SMS to be received correctly.

Of course, indeed, the `costcontrolSMSQuery` should be notified only when starting a balance request or when starting a top up request. Can you point me where did you see we are sending the IAC request to enable the blacklist as soon as we receive a message?

Comment 6

[Julien Wajsberg [:julienw]]

I simply added logs in [1]:

  console.log('received message', JSON.stringify(data));

[1] https://github.com/mozilla-b2g/gaia/pull/19483/files#diff-758728f3b5e9878aca439efb22437178R21

Comment 7

[Marina Rodríguez [:mai]]

Comment on attachment 8428551 [details] [review]
patch v1.0

Salva, do you mind reviewing the path?
Regards

Comment 8

[Salvador de la Puente González [:salva]]

Comment on attachment 8428551 [details] [review]
patch v1.0

Please, address the comments on GitHub and ask for my review again.

Comment 9

[Marina Rodríguez [:mai]]

Comment on attachment 8428551 [details] [review]
patch v1.0

Updated the PR. Salva, would you mind reviewing again?

Comment 10

[Salvador de la Puente González [:salva]]

Comment on attachment 8428551 [details] [review]
patch v1.0

Much better. Let's land this. Thank you for all the changes.

Comment 11

[Marina Rodríguez [:mai]]

Master: a6d8405100c93da904282f6dacd9c66bb29f75e2

Comment 13

[Loli (:lolimartinezcr)]

I have tested sound and SMS in inbox folder with usage application and I have seen a bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1032095