ViewModelBase/CanDelete/EditableObject

ViewModelBase/CanDelete/EditableObject

Old forum URL: forums.lhotka.net/forums/t/9902.aspx


Peran posted on Tuesday, December 28, 2010

I have found that my Delete button is enabled when the Model.IsNew.  

I would not expect CanDelete = true when the editable object does not currently exist on the database (as there is nothing to delete!).

A potential patch below:

Index: Shared/Source/Csla/4.1.0-101022/Source/Csla.Xaml/ViewModelBase.cs

===================================================================

--- Shared/Source/Csla/4.1.0-101022/Source/Csla.Xaml/ViewModelBase.cs (revision XXX)

+++ Shared/Source/Csla/4.1.0-101022/Source/Csla.Xaml/ViewModelBase.cs (working copy)

@@ -335,7 +335,7 @@

         else

           CanCreate = false;

 

-        if (CanDeleteObject && !isObjectBusy)

+        if (CanDeleteObject && !targetObject.IsNew &&!isObjectBusy)

           CanDelete = true;

         else

           CanDelete = false;

Peran replied on Wednesday, January 26, 2011

Hi Rocky,

Do you also regard this as a bug?

 

Regards

Peran

RockfordLhotka replied on Wednesday, January 26, 2011

I'm not sure about this. The data portal plumbing actually considers this a valid operation - though obviously it doesn't really do any work beyond modifying the object's metastate.

Peran replied on Thursday, January 27, 2011

Ok, Ill make some changes within my own code.

 

Thanks

Peran

RockfordLhotka replied on Thursday, January 27, 2011

I'd be interested in other people's thoughts as well. I'm not sure which way is "correct", and am willing to be swayed.

tmg4340 replied on Thursday, January 27, 2011

From my perspective, CanDelete = true really only makes sense on an existing object (i.e. IsNew = false).  And in my weird world, my point is supported by the fact that deleting an object turns IsNew = true (sometimes a point of contention on the forum, but the current behavior nonetheless.)  It seems to make little sense to let me delete an object multiple times.  Smile

HTH

- Scott

ajj3085 replied on Saturday, January 29, 2011

On the surface this seems to make sense, but I keep having a nagging feeling that there could be an operation where your BO only exists to delete something, and for some reason CommandBase isn't enough. 

Maybe a DeleteAccount BO, where you have to enter a password that gets validated on the server.  And you have business rules where the password cannot be empty, or you have so many minutes from the time the confirmation screen comes up to finish the command...

I know it's contreived, but what's wrong with hiddening your delete button when IsNew = true?  Cant be that hard, and this might break some functionality someone is actually using..

 

RockfordLhotka replied on Saturday, January 29, 2011

Maybe the simple answer is that the property in ViewModelBase should be virtual?

Peran replied on Saturday, January 29, 2011

Or another possibility would be to make SetProperties() virtual? So an override would look like:

{

    base.SetProperties()

    CanDelete = myImplementation....;

}

 

Could result in the property value getting changed and flipped back again though...

tiago replied on Sunday, January 30, 2011

RockfordLhotka

I'd be interested in other people's thoughts as well. I'm not sure which way is "correct", and am willing to be swayed.

When an object IsNew (from the point of view of the 'user') it was created on my screen just now and I have to decide whether to save it or discard it. It's like when I pick an object in a shop: I can buy it or I can return it to the shelf. I'm not supposed to eat it or do anything else before buying it.

As I see it, when IsNew=true, Delete can only mean Cancel. I can't think of a scenario where these buttons could have different meanings. So one of them is in fact a duplication.

If the user sees both buttons enabled, he might think the object is already saved. Following the least astonishment principle, I'd force the Delete button to be disabled or even hidden.

 

 

JonnyBee replied on Sunday, January 30, 2011

If we look back at WindowsForms and the Delete button on the BindingNavigator component then from a "user" perspective you can "Remove" items whether they are New or has been saved.

You could have a list of invoice items - add 5 rows and edit data inn these rows. Shouldn't there be just one "Remove" button or should there be separate buttons for "Delete" and "Cancel"?

tmg4340 replied on Sunday, January 30, 2011

I thought about this after my initial posting, thinking that perhaps there was an instance - this one - where having the delete would be useful.

However, I think in this instance my reasoning still applies.

If you think about what you're talking about, it's not deleting an object - it's removing an object from the list.  In CSLA terms, the object is moved to the deleted-items collection.  When "Save" is called, it's on the entire list, and new objects in the deleted list are discarded.  So having the delete option available for that individual new instance still doesn't make sense.

There still is the possibility of a dynamic list.  But even that still doesn't apply, because the object hasn't been saved yet, so if someone doesn't want it they'd hit ESC and cancel the entire edit.

I will admit that where this falls down is the authorization aspect - if a new object has CanDelete = false, then you technically can't get rid of the item from a list, because that would disable the delete button when that item becomes the current item.  While I don't want to make the framework any more complex, conceptually I think it might be better to build in properties to differentiate "deleteability" from "ability to remove from a list".  I realize that to some people, it's essentially a semantic difference, so in the end maybe it is easier just to leave it alone.  But they just feel like two different concepts to me.

- Scott

RockfordLhotka replied on Monday, January 31, 2011

I do think the answer is to make the property virtual. Peran, can you add an enhancement issue to bug tracker for this? Thanks!

Peran replied on Monday, January 31, 2011

Added as issue 894.

 

Peran

Copyright (c) Marimer LLC