1042591 - [Contacts][ICE] Display contacts from ICE group

Status: VERIFIED FIXED

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

Description

[Michał Budzyński (:michalbe)]

When there are contacts added to ICE group, and we click on the first element of the Contacts List (`ICE CONTACTS`), we should see only the contacts we defined as ICE. 
This could be solved using fake-search (searching for something only ICE contacts have, so all the others will be hidden), so we will not need to rebuild the Contacts List (we don't have support of the second view of the list for now, and it would be painful to implement one).

Comment 1

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

Attachment: Pointer to PR 23086

Comment 2

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

Comment on attachment 8475893 [details] [review]
Pointer to PR 23086

Wrong bug

Comment 3

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

Attachment: Pointer to PR 23209

Comment 4

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

Comment on attachment 8477404 [details] [review]
Pointer to PR 23209

Hi Sergi,

the PR is ready for review, if you want to see the ICE contacts you actually have to setup real contacts as ice, for doing that just:

ICEStore.setContacts(['<contact id1>','<contact id2>']);

where the contact ids must be real ;)

Thanks!

Comment 5

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

Comment on attachment 8477404 [details] [review]
Pointer to PR 23209

Hei Sergi,

I missed your comments on github, I'm using the flags in bugzilla to check if you reviewed the code.

I'm removing the r? from now and will apply the changes that you comment.

Will ask for r? again when ready. Thanks!

Comment 6

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

Comment on attachment 8477404 [details] [review]
Pointer to PR 23209

Hei Sergi,

ready for second round!

Comment 7

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

Comment on attachment 8477404 [details] [review]
Pointer to PR 23209

Hi Alberto, could you give feedback of this patch before the final review?

Thanks!

Comment 8

[Alberto Pastor [:albertopq]]

Comment on attachment 8477404 [details] [review]
Pointer to PR 23209

The code looks good to me. Just a couple of questions, but more regarding the spec that the implenetation:

- Shouldn't be a way of canceling the action of select a contact? Right now the only option you have is selecting a contact. Why not a inline activity?

- Should the ICE group be displayed when selecting a new ICE contact? Looks a little bit odd to me.

Nothing blocker, though.

Thanks!

Comment 9

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

Hi Alberto thanks for looking at this.

(In reply to Alberto Pastor from comment #8)
> Comment on attachment 8477404 [details] [review]
> Pointer to PR 23209
> 
> The code looks good to me. Just a couple of questions, but more regarding
> the spec that the implenetation:
> 
> - Shouldn't be a way of canceling the action of select a contact? Right now
> the only option you have is selecting a contact. Why not a inline activity?

We didn't receive any specification, but we could add a cancel button on the list I guess.
I would prefer not to use an activity cause don't want to spawn another process when we are already in contacts.

> 
> - Should the ICE group be displayed when selecting a new ICE contact? Looks
> a little bit odd to me.

Definitely sonds good.

> 
> Nothing blocker, though.
> 
> Thanks!


Really good feedback, sounds to me like things that we can do in follow ups ;)

Comment 10

[Alberto Pastor [:albertopq]]

Sorry, this feedback meant to go to bug 1042584. Already commented on the PR with my comments for this one.

Comment 11

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 8477404 [details] [review]
Pointer to PR 23209

Made another small comment to the PR, but is a r+ for me. Thanks!

Comment 12

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

Landed:

https://github.com/mozilla-b2g/gaia/commit/800bcfa418f8d4ad4eebdb3e015ba0f6cfbf65fa

Comment 13

[Eric Chang [:ericcc] [:echang]]

Verified User story is fixed. ICE can be set, view in Contacts, couple bugs found.

Gaia      2be78d83a760fa3b9638fe51c266b442d14597f1
Gecko     https://hg.mozilla.org/mozilla-central/rev/1db35d2c9a2f
BuildID   20140831160203
Version   34.0a1
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230

1061072 [Contacts] Back 'X' is missing from Set ICE page (Single select mode) 
1061069 [Contacts] ICE icon on top of the contact list is truncated
1061068 [Contacts] ICE missing from top of the list when toggling the 'Order by last name'

Comment 14

[Eric Chang [:ericcc] [:echang]]

Suite updated
[Comms][Contacts]Small set
[Comms]Contacts
https://moztrap.mozilla.org/manage/suites/?&pagenumber=1&pagesize=20&sortfield=created_on&sortdirection=desc&filter-name=%5Bcomms%5D&filter-name=contacts

ICE test cases
https://moztrap.mozilla.org/manage/cases/?&pagenumber=1&pagesize=20&sortfield=created_on&sortdirection=desc&filter-tag=2886&filter-tag=2884