Bug 230977 - [api][breaking][nls] Service property page should not show subsystem configuration IDs in the Configuration section
Summary: [api][breaking][nls] Service property page should not show subsystem configur...
Status: NEW
Alias: None
Product: Target Management
Classification: Tools
Component: RSE (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: Future   Edit
Assignee: David McKnight CLA
QA Contact: Martin Oberhuber CLA
URL:
Whiteboard:
Keywords: api, PII
Depends on:
Blocks:
 
Reported: 2008-05-07 16:02 EDT by Ankit Pasricha CLA
Modified: 2012-05-22 14:58 EDT (History)
0 users

See Also:


Attachments
proposed patch to display label instead of id (18.14 KB, patch)
2008-06-10 15:59 EDT, David McKnight CLA
no flags Details | Diff
updated patch (29.05 KB, patch)
2008-06-25 14:46 EDT, David McKnight CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ankit Pasricha CLA 2008-05-07 16:02:23 EDT
This is a UI issue....The current Service property page (and the corresponding wizard page when creating a new connection) shows subsystem configuration IDs in the "Configuration" section. 
This is non-intuitive for the user.
Comment 1 Ankit Pasricha CLA 2008-05-07 16:03:20 EDT
RSE should show the "name" field for the subsystem configuration instead.
Comment 2 Martin Oberhuber CLA 2008-05-19 04:51:41 EDT
Do we agree that we want the "name" instead of the "id" ?
Comment 3 David McKnight CLA 2008-05-20 10:13:26 EDT
As it stands right now the names of subsystem configurations are not guaranteed to be unique.  For example, the FTP "Files" subsystem configuration and the DStore "Files" subsystem configuration are named the same.  It's also quite likely that they will be the same too since this is what we display in the tree for the subsystem.
Comment 4 David McKnight CLA 2008-06-10 15:59:03 EDT
Created attachment 104378 [details]
proposed patch to display label instead of id

Here's a patch I'm proposing where a new label field is required for a subsystem configuration.  Any thoughts?
Comment 5 David McKnight CLA 2008-06-25 14:46:31 EDT
Created attachment 105830 [details]
updated patch

I updated a couple things in the patch.
Comment 6 David McKnight CLA 2008-06-25 14:47:50 EDT
Let me know what you think of the patch.  The label attributes for subsystem configurations would need translation although "label" is an optional attribute, so the fallback would be to use the id as it does now.

Comment 7 Martin Oberhuber CLA 2008-06-26 06:31:45 EDT
Strictly speaking, your proposed patch is [api][breaking] because ISubSystemConfigurationProxy is not marked @noextend -- an extender could implement ISubSystemConfigurationProxy directly, and break by your addition of the #getLabel() method.

The second problem is that you are adding pii, and while you're right that the plugin.xml's label field is optional with fallback to the ID, your patch adds a translatable label for "Local Files" for instance, so if the additional pii is not translated users would now see
   "%localFilesLabel"
rather than 
   "local.files"
which they see now.

This is a corner case though, and if you think you must have the fix in 3.0.1 then I'd guess it is possible to change the patch in a way that makes it "API compatible". But that's considerable effort.

In my personal opinion, the issue is minor anyways so I'd prefer deferring it to 3.1 -- Ankit, you're the original submitter, what do you think? Is it OK for you to move severity to "minor" ? The current ID's that we have are not too bad, are they?
Comment 8 David McKnight CLA 2008-07-14 14:22:46 EDT
Ankit, could you respond to comment #7?  Is this needed for 3.0.1 or can it wait til 3.1?
Comment 9 Martin Oberhuber CLA 2010-05-03 17:29:53 EDT
Please check whether this is still possible in 3.2 or needs to be deferred to the next release due to breaking API changes.
Comment 10 Martin Oberhuber CLA 2011-05-31 17:42:05 EDT
Moving deferred 3.3 api items to 3.4
Comment 11 Martin Oberhuber CLA 2012-05-22 14:58:54 EDT
We are post API freeze.