Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 319289

Summary: PersonProposalProvider should display real name when available
Product: z_Archived Reporter: David Shepherd <david.shepherd>
Component: MylynAssignee: David Shepherd <david.shepherd>
Status: RESOLVED FIXED QA Contact:
Severity: enhancement    
Priority: P3 CC: david.shepherd, steffen.pingel
Version: unspecifiedKeywords: contributed
Target Milestone: 3.5   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
Patch to enable PersonProposalProvider reuse
none
mylyn/context/zip
none
Updated Patch
none
updated patch
steffen.pingel: iplog+
mylyn/context/zip none

Description David Shepherd CLA 2010-07-08 13:08:35 EDT 
Often the real name of a person is available and should be preferred for displaying in the content assist instead of the user id.
Comment 1 David Shepherd CLA 2010-07-08 13:14:32 EDT 
Not sure that the real names are actually available, noticed that TaskAttribute.USER_ASSIGNED_NAME is depreciated.
Comment 2 Steffen Pingel CLA 2010-07-09 22:32:09 EDT 
Makes sense. As a first step you could extend PersonProposalProvider to work with IRepositoryPerson objects.
Comment 3 David Shepherd CLA 2010-07-13 14:43:55 EDT 
Created attachment 174200 [details]
Patch to enable PersonProposalProvider reuse

This patch should allow clients to reuse the PersonProposalProvider by subclassing it and implementing the createPersonProposal method and/or the getPrettyName method.  This would allow clients that have access to display names (i.e., pretty names) to show those names in the content assist while still providing the normal values as content for inserting into the field.  

Also in this patch: updates to how the address set is being populated, including adding users that commented and added attachments.

Please provide feedback on what needs to be updated on this patch.  Thanks!
Comment 4 David Shepherd CLA 2010-07-13 14:43:59 EDT 
Created attachment 174201 [details]
mylyn/context/zip
Comment 5 Steffen Pingel CLA 2010-07-13 15:11:35 EDT 
Looks good. Two thoughts:

* I don't think it's worth introducing the factory at this point since clients need to subclass anyways. Can you merge the factory into PersonProposalProvider?
* All creation of repository person objects should go through the addPerson() method. You probably want to pass in the TaskAttribute. Look at TaskAttachmentMapper to get the right one for attachment authors.
Comment 6 Steffen Pingel CLA 2010-08-18 01:57:38 EDT 
David, are you planning on addressing the points in comment #5?
Comment 7 David Shepherd CLA 2010-08-18 18:47:24 EDT 
Created attachment 176947 [details]
Updated Patch

(In reply to comment #5)
> * I don't think it's worth introducing the factory at this point since clients
> need to subclass anyways. Can you merge the factory into PersonProposalProvider?
Merged into PersonProposalProvider.
> * All creation of repository person objects should go through the addPerson()
> method. You probably want to pass in the TaskAttribute. Look at
> TaskAttachmentMapper to get the right one for attachment authors.
All creation of repository person objects moved to addPerson.  I'm not sure I understood this comment exactly right so please check out the patch.
Comment 8 Steffen Pingel CLA 2010-08-18 19:07:13 EDT 
It seems like the addAddresses() method is not actually transferring data from personsOfInterest to addressSet. Can you take a look?
Comment 9 David Shepherd CLA 2010-08-18 19:24:13 EDT 
Created attachment 176951 [details]
updated patch

Updated patch to transfer data.  The check for user_cc is a bit odd, please review.
Comment 10 Steffen Pingel CLA 2010-08-18 20:41:48 EDT 
Thanks. I have applied the patch with a few modifications.
Comment 11 Steffen Pingel CLA 2010-08-18 20:41:51 EDT 
Created attachment 176953 [details]
mylyn/context/zip