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

Bug 347463

Summary: No delete attribute confirmation
Product: [WebTools] Dali JPA Tools Reporter: Stefan Dimov <stefan.dimov>
Component: Diagram EditorAssignee: Stefan Dimov <stefan.dimov>
Status: VERIFIED FIXED QA Contact: Stefan Dimov <stefan.dimov>
Severity: major    
Priority: P3 CC: cbridgha, david_williams, jolene.moffitt, neil.hauge, raghunathan.srinivasan, tranle1
Version: 3.0Flags: david_williams: pmc_approved+
raghunathan.srinivasan: pmc_approved+
stefan.dimov: pmc_approved? (naci.dai)
stefan.dimov: pmc_approved? (deboer)
stefan.dimov: pmc_approved? (neil.hauge)
stefan.dimov: pmc_approved? (kaloyan)
cbridgha: pmc_approved+
stefan.dimov: review+
neil.hauge: review+
Target Milestone: 3.0 RC3   
Hardware: All   
OS: All   
Whiteboard: PMC
Attachments:
Description Flags
patch
none
patch v2 none

Description Stefan Dimov CLA 2011-05-27 10:30:17 EDT
There is a regression - the deletion of attributes doesn't require confirmation.
It's been performed immediatly.

It's related to the bug #345725 - common reason
Comment 1 Stefan Dimov CLA 2011-05-27 10:45:55 EDT
Created attachment 196769 [details]
patch
Comment 2 Stefan Dimov CLA 2011-05-27 10:59:16 EDT
Created attachment 196771 [details]
patch v2
Comment 3 Stefan Dimov CLA 2011-05-27 11:03:43 EDT
This is a bad defect, because nothing should be deleted without confirmation.

No workaround.

The fix was tested manually. All the existing JUnit tests are passing
succesfully.

The bug is result of a recent port of the editor to a newer Graphiti version.
There was an API change in the Graphiti. A certain parameter was added to the
method getUserDecision() of the DefaultDeleteFeature class. This way, the
method is not being invoked (from the subclass ClickRemoveAttributeButtonFeature). Instead
of it, another method from the super class is being invoked and it just
returns true without confirmation.

The fix is very small - i've just added a parameter to the invokation of
getUserDecision() and the fix is in no conflict with other pending fixes, so the risk is very low.
Comment 4 Neil Hauge CLA 2011-05-27 11:21:37 EDT
Given that the Undo function doesn't seem to work in the editor (I believe there is an open bug for this), this problem is a bit worse than it might otherwise be.  There is one possible workaround, which is to open the class in the Java editor and perform a series of Undo's there to return the class back to its original state, assuming the entity/class had not be saved already by the user.

Given that the fix is extremely isolated and low risk and potentially involves data loss, I think I can support this fix going into RC3 (with a re-spin) or RC4.  I will ask others from the PMC to provide feedback on whether re-spinning RC3 is an option for this bug.  The change poses no risks to other components.
Comment 5 David Williams CLA 2011-05-27 11:57:34 EDT
I'm glad Neil clarified that about 'undo' not working ... because "...because nothing should be deleted without confirmation." doesn't sound right. Things are deleted all the time without confirmation ... as long as undo is available. 

I'm ok with this fix, but see it as the "least of all evils" ... until undo is implemented.
Comment 6 Neil Hauge CLA 2011-05-27 12:01:58 EDT
(In reply to comment #5)
> I'm ok with this fix, but see it as the "least of all evils" ... until undo is
> implemented.

David, I assume you prefer to respin RC3 for this fix, rather than wait for RC4?
Comment 7 David Williams CLA 2011-05-27 12:10:55 EDT
> 
> David, I assume you prefer to respin RC3 for this fix, rather than wait for
> RC4?

I hadn't thought about that (one way or the other) ... but, if you want to change your green checkmark to a red x and ask for a respin, I'm fine with that ... as long as someone can confirm the build over the weekend (Monday is a Holiday in US).
Comment 8 Raghunathan Srinivasan CLA 2011-05-27 15:08:02 EDT
+1 for re-spin of RC3 to pick this fix.
Comment 9 Chuck Bridgham CLA 2011-05-27 15:19:55 EDT
approve - respin sound like the best decision at this point
Comment 10 Neil Hauge CLA 2011-05-27 15:29:49 EDT
I will note that I asked Stefan if he could confirm the build this weekend, but given the late hour in his location he probably wasn't around to see it.  He may be able to verify tomorrow, but I don't know that for sure.  Given the low risk nature of this fix, I would be OK with putting this into RC4 although generally I would be in favor of getting a fix into the earliest possible build.
Comment 11 David Williams CLA 2011-05-27 16:13:20 EDT
Ok, let's do it then (apply, respin for rc3) ... I'd like to get that releng fix in too ... and if Tran's not around, I'll be back around 8 or 10 tonight to do it (if he hasn't already). 

Much thanks.
Comment 12 Tran Le CLA 2011-05-27 17:45:46 EDT
Patch applied and released in RC3.
Comment 13 Jolene Moffitt CLA 2011-06-29 15:39:39 EDT
Verified in Build I-3.3.0RC4-20110603221533

Verified you receive a confirm delete dialog when you try to delete an attribute or entity from the diagram editor.  See the link to view test steps for verification. http://wiki.eclipse.org/Dali_3.0_RC3