[User Story][CONTACTS] Share a contact from contact details screen

VERIFIED FIXED in 2.2 S4 (23jan)

Status

defect
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: noemi, Assigned: jmcf)

Tracking

unspecified
2.2 S4 (23jan)
ARM
Gonk (Firefox OS)
Dependency tree / graph
Bug Flags:
in-moztrap +

Firefox Tracking Flags

(feature-b2g:2.2+, tracking-b2g:backlog, b2g-v2.2 verified, b2g-master verified)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

5 years ago
As a user I want to be able to share a contact from contacts details screen

(please refer to "[2.0 Contacts] SharevCard v1.pdf" spec attached)
Reporter

Comment 1

5 years ago
Adding to backlog to be properly prioritized. Thanks!
blocking-b2g: --- → backlog
See Also: → 1007932
Assignee

Updated

5 years ago
QA Contact: jmcf
Assignee

Updated

5 years ago
Assignee: nobody → jmcf
Assignee

Updated

5 years ago
QA Contact: jmcf
Assignee

Updated

5 years ago
Depends on: 1007932
Target Milestone: --- → 2.2 S3 (9jan)
Assignee

Comment 2

5 years ago
We would need the 'share' icon in the Comms theme, as well.
Flags: needinfo?(fshih)
Flags: needinfo?(b.pmm)
Assignee

Comment 3

5 years ago
Posted file Patch v1 (obsolete) —
The proposed patch does not contain tests for the Contacts app, as we would need an integration test and I'm awaiting for the landing of bug 982260. Once that bug lands, I will add the corresponding integration test for contacts app.

thanks
Attachment #8545249 - Flags: review?(schung)
Attachment #8545249 - Flags: review?(francisco)
Assignee

Comment 5

5 years ago
Posted file Final Pull Request
Attachment #8545249 - Attachment is obsolete: true
Attachment #8545249 - Flags: review?(schung)
Attachment #8545249 - Flags: review?(francisco)
Attachment #8545861 - Flags: review?(schung)
Attachment #8545861 - Flags: review?(francisco)

Updated

5 years ago
feature-b2g: --- → 2.2?
IMO, this shouldn't be a 2.2 blocker as it's a new feature that wasn't planned for 2.2
Comment on attachment 8545861 [details] [review]
Final Pull Request

Hi Jose, great work here.

Code wise looking good, just a couple of tiny comments on github.

r- cause saw the following:
- icon is wrong, as it's white while the rest of icons for contacts are green
- since bug 982260 has a r+, please add tests to this bug too, we should not land any code, specially a whole feature without tests.
- I manage to reproduce a weird behavior: share a contact and when in sms click on the attachment and view, that will go back to contact, and I'm able to edit it, canceling the whole sharing end up with the contact edited via a weird flow.

I think we can do a follow up for the 3 point anyway.

Thanks!
Attachment #8545861 - Flags: review?(francisco) → review-
Forgot to mention, in the gaia try build there are lots of errors in contacts unit tests too
feature-b2g: 2.2? → ---
feature-b2g: --- → 2.2?
Just a reminder, 1/12 will be the branch date.
As this is not defined in 2.2 scope at the first place, approval can only be given if it's at very good state and low risk.
I'll keep the feature-b2g nomination for now.
Comment on attachment 8545861 [details] [review]
Final Pull Request

The message part looks fine, but just like Fransisco said there are lots of failed test that might related to this patch.

Another concern is about the contact/message circular activity and we might have some problem when pointing back to original app. I think we already have a bug for this issue.
Attachment #8545861 - Flags: review?(schung) → review+
Assignee

Updated

5 years ago
Attachment #8545861 - Flags: review- → review?(francisco)
Assignee

Comment 11

5 years ago
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #7)
> Comment on attachment 8545861 [details] [review]
> Final Pull Request
> 
> Hi Jose, great work here.
> 
> Code wise looking good, just a couple of tiny comments on github.

Fixed!

> 
> r- cause saw the following:
> - icon is wrong, as it's white while the rest of icons for contacts are green

Yes, that's why we have needinfo-ed Fang 

> - since bug 982260 has a r+, please add tests to this bug too, we should not
> land any code, specially a whole feature without tests.

done. I've also updated unit tests as well ;)

The failing unit tests have been fixed, too. 

> - I manage to reproduce a weird behavior: share a contact and when in sms
> click on the attachment and view, that will go back to contact, and I'm able
> to edit it, canceling the whole sharing end up with the contact edited via a
> weird flow.

The flow you describe is the normal flow as we always open vcards in edit mode ... 

> 
> I think we can do a follow up for the 3 point anyway.
> 
> Thanks!

Thanks to you!
Assignee

Comment 12

5 years ago
(In reply to Steve Chung [:steveck] from comment #10)
> Comment on attachment 8545861 [details] [review]
> Final Pull Request
> 
> The message part looks fine, but just like Fransisco said there are lots of
> failed test that might related to this patch.

Yep, a missing dependency on the mock DOM tree. Thanks for spotting! 

> 
> Another concern is about the contact/message circular activity and we might
> have some problem when pointing back to original app. I think we already
> have a bug for this issue.

Oh, absolutely, do you have a pointer to the bug?
Setting ni to Steve, according to the question asked by Jose Manuel in comment 12
Flags: needinfo?(schung)
Firstly I thought it might related to bug 931339, but the it seems a default view contact behavior that editing contact is allowed. After discussion with Jenny, she said the the edit button should be disabled if the view contact panel is brought by clicking on the contact attachment(and she already confirmed with Paul).

Hi Jose, I think we will need another view contact activity with proper parameter to avoid contact editing. Do you prefer to address right in this patch or create a follow up for it?
Flags: needinfo?(schung) → needinfo?(jmcf)
Assignee

Comment 15

5 years ago
(In reply to Steve Chung [:steveck] from comment #14)
> Firstly I thought it might related to bug 931339, but the it seems a default
> view contact behavior that editing contact is allowed. After discussion with
> Jenny, she said the the edit button should be disabled if the view contact
> panel is brought by clicking on the contact attachment(and she already
> confirmed with Paul).
> 
> Hi Jose, I think we will need another view contact activity with proper
> parameter to avoid contact editing. Do you prefer to address right in this
> patch or create a follow up for it?

I think it would be better to create a follow-up as we would need to land this bug before the branching to be done today
Flags: needinfo?(jmcf)
Comment on attachment 8545861 [details] [review]
Final Pull Request

Just left a nit on github, really tiny thing.

Code wise looking good to me, now with test (thanks a lot for adding them!), as you commented, waiting for the final resources.

Please merge once you have them.
Attachment #8545861 - Flags: review?(francisco) → review+
Assignee

Updated

5 years ago
See Also: → 1120392
Assignee

Comment 17

5 years ago
https://github.com/mozilla-b2g/gaia/commit/25b310a10360a6063cfe06e7ef3840696314c564

in bug 1120392 we will land the correct icons for the 'share' action
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
I'm sorry for the delay in replying, Just attached the image assets in bug 1120392. Thanks!
Flags: needinfo?(fshih)
Assignee

Updated

5 years ago
See Also: → 1120878

Updated

5 years ago
Flags: needinfo?(b.pmm)
Tested with today build in Flame( Gecko-7a1f501.Gaia-9946a49) there is an option to share contact info in Contact details panel. Thank you everyone for making it happen!
Assignee

Comment 20

5 years ago
Comment on attachment 8545861 [details] [review]
Final Pull Request

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Share Contact Feature
[User impact] if declined: High, brand new 2.2 feature will be missing, allowing to share contacts from the Contacts App
[Testing completed]: Yes, unit tests and marionnette tests are provided. 
[Risk to taking this patch] (and alternatives if risky): No-risk patch
[String changes made]: new literal string 'shareContact' 

(Aside note)

We have to request the approval because for some strange reason this bug did not land on the v2.2 branch despite We had agreed to do so. It seems that the branching left it out for some unknown reason. So the approval is just a formality as it was already agreed to have this feature in v2.2
Attachment #8545861 - Flags: approval-gaia-v2.2?
Reporter

Updated

5 years ago
Eric,

Can we assign a QA to help test this enhancement ?
Flags: needinfo?(echang)
Keywords: verifyme
:brg/:oteo, any testing help we can get more your end on this will be super helpful! Just want to make sure we identify any fallouts sooner than later.
Flags: needinfo?(oteo)
Flags: needinfo?(beatriz.rodriguezgomez)
(In reply to bhavana bajaj [:bajaj] from comment #22)
> :brg/:oteo, any testing help we can get more your end on this will be super
> helpful! Just want to make sure we identify any fallouts sooner than late

Sure, we'll testing on our side and we'll let you know any issue discovered.

Eric, Johan, let me know if you want me to change the QA Contact to someone in Telefonica's team although I think Eric was already doing some testing of the feature and raising an issue (https://bugzilla.mozilla.org/show_bug.cgi?id=1007932#c46)
Flags: needinfo?(oteo)
Flags: needinfo?(jlorenzo)
Flags: needinfo?(beatriz.rodriguezgomez)
Assignee

Updated

5 years ago
Blocks: 1120392
The Taiwan QA team is responsible for the validation of the 2.2 features on Mozilla's side. I prefer to let the decision in Eric's hands.
Flags: needinfo?(jlorenzo)
Reporter

Updated

5 years ago
See Also: → 1121917
Reporter

Updated

5 years ago
Reporter

Updated

5 years ago
Depends on: 1111724
Yes, I am working on another related case bug 1007932, so it looks like this one will be granted to 2.2+, right.
QA Whiteboard: [COM=Gaia::Contacts]
Flags: needinfo?(echang) → in-moztrap?(echang)
feature-b2g: 2.2? → 2.2+
Assignee

Updated

5 years ago
Duplicate of this bug: 1031651
Attachment #8545861 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Attachment #8555094 - Attachment mime type: text/plain → text/html
Flags: in-moztrap?(echang) → in-moztrap+
Per Comment 21 & Comment 25,and this bug had verified in https://bugzilla.mozilla.org/show_bug.cgi?id=1007932#c51 ,clear "verifyme" and modify as "VERIFIED".
Status: RESOLVED → VERIFIED
Keywords: verifyme
Depends on: 1140247
blocking-b2g: backlog → ---
Depends on: 1149938
Depends on: 1150827
Depends on: 1186108
No longer depends on: 1186108
You need to log in before you can comment on or make changes to this bug.