1072180 - [GAIA][Contacts]if No Contacts on Gmail, while import select all and deselect all are enabled

Status: VERIFIED FIXED

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

Description

[ashayb2g]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20140922030204

Steps to reproduce:

Precondition:
No contacts on Gmail.

Steps:
1. Contacts->Settings->Import->Gmail->Login->Accept->See empty list.


Actual results:

It shows All friends are imported, and select All, deselect All buttons are active alternately.


Expected results:

It should not show this message or select All and deselect All buttons should be disabled.

Comment 1

[Johan Lorenzo [:jlorenzo]]

QA wanted for a branch check.

Comment 2

[Duane Dixon [:ddixon]]

Branch Check

Issue DOES occur in Flame 2.2, 2.1, 2.0, 2.0 base image, and Open_C 2.2 

Actual Results: 
The "Select All" and "Deselect All" buttons can be pressed despite having no contacts available to import. 

Device: Flame Master
Build ID: 20140923233152
Gaia: ff6dbb006e4e60edba697639aa2a2a199b07069b
Gecko: 1e2993c99323
Version: 35.0a1 (Master)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
------------------------------------------------------------------
Device: Flame 2.1
Build ID: 20140924061258
Gaia: 020e6283a033e8fbcf65e7ed81c5b75ba0095f22
Gecko: d6b762814638
Version: 34.0a2 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
------------------------------------------------------------------
Device: Flame 2.0
Build ID: 20140923195101
Gaia: 263e3b201dca967ec5346e35901aa981ca47dce7
Gecko: 35d791e16d31
Version: 32.0 (2.0)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
------------------------------------------------------------------
Device: Flame 2.0 (Base Image)
Build ID: 20140904160718
Gaia: 506da297098326c671523707caae6eaba7e718da
Gecko: 2b27becae85092d46bfadcd4fb5605e82e1e1093
Version: 32.0 (2.0)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
------------------------------------------------------------------
Device: Open_C Master
Build ID: 20140923065343
Gaia: 37b8a812c642ca616bf9457cb9b71e45261cdfa8
Gecko: 9e193395b912
Version: 35.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Comment 3

[ashayb2g]

Attachment: importer_selectAll_disable.patch

PFA patch to fix this issue.

Thanks
Ashay

Comment 4

[Johan Lorenzo [:jlorenzo]]

Comment on attachment 8494974 [details] [diff] [review]
importer_selectAll_disable.patch

I would say this tiny patch LGTM but I'm not a developer in the Comms Teams. Transfering to Francisco.

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8494974 [details] [diff] [review]
importer_selectAll_disable.patch

Nice one!

LGTM, could we have a unit test for this?

Please flag me again for review to have this patch into master!

Comment 6

[Brogan Zumwalt [Inactive]]

This affects Facebook importing screen as well.

Comment 7

[Jose Manuel Cantera]

assigning to Adrian to rescue the proposed patch and finish the work

Comment 8

[Adrián de la Rosa]

After discussing it with Jose Manuel, we would like to know UX opinion about which would be the best solution for this problem. The bug suggested to disable the "Select all" and "Deselect all" buttons, but Jose Manuel suggested that we just showed a confirmation dialog.

I've recorded a video with each implementation so we can decide:
 - Confirmation dialog: https://www.youtube.com/watch?v=mIgdF9OyCqc
 - Buttons disabled: https://www.youtube.com/watch?v=RiMVegc0c60

Comment 9

[Carrie Wang [New account: [e-mail]]]

I think a confirm window will make it simple and clear. Thanks for the suggestion. Great job guys!

Comment 10

[Adrián de la Rosa]

Attachment: Pull request #25457

I haven't been able to use the patch because it couldn't be applied right now on master. Anyway, since now we changed to a totally different approach to fix this, it doesn't matter.

I had to change two tests that were checking that import UI was fine when no contacts to import since now we won't see that UI if there are no contacts.

Tests are passing in local, and on device the fix work as expected and I can also import from an account with contacts as usual.

What do you think?

Comment 11

[Jose Manuel Cantera]

Comment on attachment 8510381 [details] [review]
Pull request #25457

the PR needs more work and the tests an approach change

thanks

Comment 12

[Adrián de la Rosa]

Comment on attachment 8510381 [details] [review]
Pull request #25457

PR updated. Using more mocks and less stubs on tests, so it is solved with less loc. Also, improvements in importer_ui.js as suggested.

Comment 13

[Jose Manuel Cantera]

Comment on attachment 8510381 [details] [review]
Pull request #25457

a return is missing. tests is incomplete as well

thanks

Comment 14

[Adrián de la Rosa]

Comment on attachment 8510381 [details] [review]
Pull request #25457

Added callback execution for tests when no contacts, just like when contacts are loaded. The test now also check the parameters of the call to the confirm window.

Comment 15

[Jose Manuel Cantera]

Comment on attachment 8510381 [details] [review]
Pull request #25457

thanks Adrian. Please check tests status

Comment 16

[Jose Manuel Cantera]

https://github.com/mozilla-b2g/gaia/commit/fbb610827cffcebe213385c3343ac2f693f96956

Comment 17

[John Thomas [:Johnt]]

This issue has been resolved fixed on the Flame 2.2 KK Shallow Flash (319mb)

Result: Instead of the selectable "select all" and "deselectall" buttons a message saying "No friends or contacts found in this account." with an OK message will appear.

Flame 2.2
Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat Base)(Shallow Flash)
Build ID: 20141125040209
Gaia: 824a61cccec4c69be9a86ad5cb629a1f61fa142f
Gecko: acde07cb4e4d
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0