Closed
Bug 875159
Opened 12 years ago
Closed 10 years ago
[B2G] [Inari] [Contacts] Wrong message is displayed when no Gmail contacts are available to import
Categories
(Firefox OS Graveyard :: Gaia::Contacts, defect)
Tracking
(blocking-b2g:-, b2g-v1.3T affected, b2g-v2.0 affected, b2g-v2.1 verified)
People
(Reporter: dsubramanian, Assigned: robert.sajdok)
References
Details
(Whiteboard: permafail, c=,)
Attachments
(6 files)
Description: When the user has no Gmail contacts available to import, the user must be prompted with correct message "No contacts available to import". Prerequiste: No contacts in Gmail Repro Steps: 1) Updated to Unagi Build ID: 20130515070208 2) Go to Contacts App 3) Click on Settings 4) Click on Gmail button 5) Sign in with valid credentials Actual: Wrong message is displayed "All your friends are imported" Expected: Correct message is displayed to inform the user, that there are no contacts available to import Environmental Variables: Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/d06cfe7d67c2 Gaia: 0ddb515f15cbc6b74fc2742b7599d6ae74c6413f Version:18.0 Notes: Repro frequency: 100% Test Suite Name: Contacts UCID:OWD-23754 Link to failed test case: https://moztrap.mozilla.org/runtests/run/1301/env/315/?pagenumber=1&pagesize=20&sortfield=order&sortdirection=asc&filter-id=7073 Q Analysts Test Team Priority: Pri 3 See attached screenshot and logcat for more info.
Reporter | ||
Comment 1•12 years ago
|
||
Reporter | ||
Comment 2•12 years ago
|
||
Reporter | ||
Updated•12 years ago
|
Summary: [B2G] [Inari] [Contacts] Wrong message is displayed when no Gmail contacts are available to be import → [B2G] [Inari] [Contacts] Wrong message is displayed when no Gmail contacts are available to import
Updated•11 years ago
|
Assignee: nobody → jmcf
Comment 4•11 years ago
|
||
In that case putting in c= to get this on product backlog list.
blocking-b2g: leo? → -
Whiteboard: inarirun2 → inarirun2, c=
Having a similar issue After logged in to Hotmail account and imported all contacts, and then logged out, and signed in with a different account without contacts, the information about the previous account is shown Check the image attachment Environmental Variables: Build ID: 20130610070206 Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8e3f39363c54 Gaia: ce3b99781d182ad550a325206990c249b0dbcf0e Platform Version: 18.0
Whiteboard: inarirun2, c=,leorun3 → inarirun2, c=,leorun3, burirun3
Updated•11 years ago
|
Whiteboard: inarirun2, c=,leorun3, burirun3 → permafail, c=,
Updated•11 years ago
|
Assignee: jmcf → gtorodelvalle
Comment 6•11 years ago
|
||
Removing myself as the assignee since not already working on this bug ;-)
Assignee: gtorodelvalle → nobody
Updated•11 years ago
|
status-b2g-v1.3T:
--- → affected
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → robert.sajdok
Comment 8•11 years ago
|
||
Comment on attachment 8406889 [details] [review] Proposed fix. Thanks for looking at this Robert. I like the new string text, but I don't think we want to drop the old text. Ultimately, I think the code needs to distinguish between "no contacts at all" and "found some contacts, but they are imported already". So I think we want a new string defined in facebook.en-US.properties. Maybe something like |fbNoFriends = No contacts available to import|. Then in the importers |markExiting()| function you can check to see if |myFriends.length < 1| and then set fbNoFriends string immediately. https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/js/importer_ui.js#L462 How does that sound? Thanks again! Francisco, does this sound correct to you?
Attachment #8406889 -
Flags: review?(bkelly)
Attachment #8406889 -
Flags: review-
Attachment #8406889 -
Flags: feedback?(francisco.jordano)
Comment 9•11 years ago
|
||
Comment on attachment 8406889 [details] [review] Proposed fix. Hi, Thanks for the patch, as Ben comments, is better to differentiate when you consciously don't import any, or there are no contacts to import. A separate string and a bit of code and unit tests will be kindly appreciated :) Thanks!
Attachment #8406889 -
Flags: feedback?(francisco.jordano) → feedback-
Assignee | ||
Comment 10•11 years ago
|
||
Regarding this issue. If the contacts are imported the method "markExisting" is not called.
Flags: needinfo?(bkelly)
Comment 11•11 years ago
|
||
I'm not very familiar with the internals of the importer. Cristian, does comment 10 look like a problem>
Flags: needinfo?(bkelly) → needinfo?(crdlc)
Updated•11 years ago
|
Flags: needinfo?(crdlc) → needinfo?(jmcf)
Comment 12•11 years ago
|
||
(In reply to robert.sajdok from comment #10) > Regarding this issue. If the contacts are imported the method "markExisting" > is not called. Mark existing will mark those contacts that are already imported. At some point in the code (probably on the friendsReady function) you will need to detect when the number to import is equal to the number of already imported
Flags: needinfo?(jmcf)
Assignee | ||
Comment 14•11 years ago
|
||
I don't understand the idea of making tests for it. I can't see tests for markExisting functions where I've made changes. Is there a place for the test for markExisting function? Please, give me some advice or show me the place for tests of this code.
Flags: needinfo?(francisco.jordano)
Comment 15•11 years ago
|
||
Comment on attachment 8423186 [details] [review] Proposed fix. The code looks good here, but I do think we need a test. Perhaps you can expand this test case or another one in this file? https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/contacts/test/unit/import/import_test.js#L160 Basically, perform the import and then look for the particular message string in the resulting DOM. Sorry for the delay on the review. I have been traveling this week. Thanks again for your help with this Robert!
Attachment #8423186 -
Flags: review?(bkelly)
Flags: needinfo?(francisco.jordano) → needinfo?(robert.sajdok)
Assignee | ||
Comment 16•10 years ago
|
||
Ok, I will try. Should I make the different commit for this test or Should it be in the same commit as correction to this bug?
Flags: needinfo?(robert.sajdok) → needinfo?(bkelly)
Comment 17•10 years ago
|
||
(In reply to robert.sajdok from comment #16) > Ok, I will try. > Should I make the different commit for this test or Should it be in the same > commit as correction to this bug? Please put them in the same commit. You can use: git commit --amend To update your current commit. You'll need to force push to the github branch to update the PR, then. Thanks!
Flags: needinfo?(bkelly)
Assignee | ||
Comment 18•10 years ago
|
||
I am not sure how to write this test. I think the MockConnector.listDeviceContacts should be empty. Because my function concerns import without contacts. Please check my test function and if it is necessary show me the right direction.
Flags: needinfo?(bkelly)
Assignee | ||
Updated•10 years ago
|
Attachment #8423186 -
Flags: review?(bkelly)
Comment 19•10 years ago
|
||
Sorry for the delay in responding. I was on vacation and then busy trying to catch up last few days. I think you want something like: assert.isTrue(document.getElementById('friends-msg').textContent === 'No contacts available to import'); In your test to verify the correct message text is set.
Flags: needinfo?(bkelly)
Comment 20•10 years ago
|
||
Comment on attachment 8423186 [details] [review] Proposed fix. Please reflag once the test verifies the message string. Otherwise this looks good. Thanks!
Attachment #8423186 -
Flags: review?(bkelly)
Assignee | ||
Updated•10 years ago
|
Attachment #8423186 -
Flags: review?(bkelly)
Comment 21•10 years ago
|
||
Comment on attachment 8423186 [details] [review] Proposed fix. Thank you Robert! I really appreciate your hard work and persistence on this. r=me
Attachment #8423186 -
Flags: review?(bkelly) → review+
Comment 22•10 years ago
|
||
Robert, can you rebase this to most recent master and then add checkin-needed keyword to get it landed? Thanks again.
Flags: needinfo?(robert.sajdok)
Assignee | ||
Comment 23•10 years ago
|
||
> Robert, can you rebase this to most recent master
Done
Please explain me the second part of your comment "add check in-needed keyword"
Flags: needinfo?(robert.sajdok) → needinfo?(bkelly)
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(bkelly)
Comment 24•10 years ago
|
||
Thanks Robert. Just by way of explanation, I was going to merge your PR for you, but since it needed to be rebased I thought it would be better to use checkin-needed. That way you wouldn't need to wait for me if I wasn't around. The checkin-needed keyword is a flag for the sheriffs to land the patch for you once the tests turn green.
Comment 25•10 years ago
|
||
Master: https://github.com/mozilla-b2g/gaia/commit/07cb6457379308e1afe3cfc52af3bd633c159bfc
Status: NEW → RESOLVED
Closed: 10 years ago
status-b2g-v2.1:
--- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S4 (20june)
Comment 26•10 years ago
|
||
Putting a string between the plural form declaration and the actual plural forms is quite confusing. No actual harm done, just a reminder if you want to fix it here in a follow-up commit. fbFriendsFound = {[ plural(numFriends) ]} +fbNoFriends = No contacts available to import fbFriendsFound[zero] = All your friends are imported fbFriendsFound[one] = Found one friend to import
Comment 28•10 years ago
|
||
This issue is still occurring on the Flame 2.0 2.0 Environmental Variables: Device: Flame 2.0 Build ID: 20140625000201 Gaia: de77f794db22a45f9d575de2c6e266a30a50de3b Gecko: 79712bd7b60d Version: 32.0a2 (2.0) Firmware Version: v121-2 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Can you look at this Robert?
Flags: needinfo?(robert.sajdok)
Updated•10 years ago
|
status-b2g-v2.0:
--- → affected
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(robert.sajdok)
Assignee | ||
Comment 31•10 years ago
|
||
I have not got my own Flame. I bought one but I've been still waiting for a delivery. I tested this bug on today's version of gaia on local machine with firefox and I can't see the problem you described. I have gmail account without contacts and after importing from this account I get the message "No contacts available to import" In my opinion everything is ok.
Comment 32•10 years ago
|
||
(In reply to robert.sajdok from comment #31) Hello, I am still experiencing this on the latest 2.0 Flame build: Device: Flame 2.0 BuildID: 20140730000206 Gaia: 2e85678de2c8e13e585288d4cec7d6673cee17ee Gecko: 6e37ecf873da Version: 32.0 (2.0) Firmware: V122 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 When logging into an account without contacts, I am receving the prompt that "All your friends are imported"
status-b2g-v2.0:
--- → affected
Flags: needinfo?(robert.sajdok)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Comment 33•10 years ago
|
||
At this point let's write up a new issue.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage-]
Flags: needinfo?(ktucker) → needinfo?(rkunkel)
Comment 34•10 years ago
|
||
Issue is rewriten: https://bugzilla.mozilla.org/show_bug.cgi?id=1047710
Flags: needinfo?(rkunkel)
Comment 35•10 years ago
|
||
As far as I can tell this just needs to be uplifted, no? It only landed v2.1.
Assignee | ||
Comment 37•10 years ago
|
||
Yes I see the problem in v2.0 version of gaia. I do not know what exactly I should do with it? Should I make patch for v2.0 version or what?
Flags: needinfo?(bkelly)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(robert.sajdok)
Comment 38•10 years ago
|
||
(In reply to robert.sajdok from comment #37) > Yes I see the problem in v2.0 version of gaia. I do not know what exactly I > should do with it? Should I make patch for v2.0 version or what? No. As Francesco indicated in bug 1047710 comment 3, we can't uplift a bug with string changes to v2.0 at this point in the release cycle. There would not be enough time to do l10n translations, etc. So I don't think there is anything to be done for this right now. Thanks!
Flags: needinfo?(bkelly)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage-]
Comment 39•10 years ago
|
||
This issue has been verified successfully on Flame 2.1 See attachment: Verify_image.png Reproducing rate: 0/5 Flame 2.1 build: Gaia-Rev db2e84860f5a7cc334464618c6ea9e92ff82e9dd Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/211eae88f119 Build-ID 20141126001202 Version 34.0
You need to log in
before you can comment on or make changes to this bug.
Description
•