CSLA 4.11.1 breaks deletions when root object is list

[GreatBarrier86]

Question
The change to UndoableBase in #1191 seems to have caused a new problem in my scenario. In order to delete an item from my list, i'm calling DoRemove(myItem) followed by SaveAsync. Since ViewModelBase.SaveAsync clones the list, and your change below removes _stateStack from being serialized in UndoableBase.OnGetState and UndoableBase.OnSetState, when BusinessListBase.AcceptChanges is called, the loop through DeletedList that calls C.AcceptChanges throws an exception because C.EditLevel - 1 ends up being -1 while parentEditLevel is 0.

In a similar style to my previous issue, see if this breaks for you as well

DiariesVM lvm = DiariesVM.Fetch(); //Assume this creates a DiariesVM that has a BusinessListBase Model containing 2 Diary BusinessBase objects
lvm.DoRemove(lvm.Model[0]);
lvm.SaveAsync();

Code removed from CSLA UndoableBase

UndoableBase Failure Point

Version and Platform
CSLA version: 4.11.1
OS: Windows
Platform: Xamarin

[GreatBarrier86]

Btw, it looks like the reason it works successfully on Update/Insert + Save but not Delete + Save is because when you clone a list item that is NOT in the DeletedList, BusinessListBase.InsertItem that occurs when deserializing to the clone sets the new item to the target level.

Since DeletedList is just a regular MobileList, it doesn't reset the children's EditLevel to the expected state when the deserialization during Clone, so the Delete fails.

Hope that can be some help.

[rockfordlhotka]

Thanks @GreatBarrier86 - I see this thread, but am very busy this week, so may not get to it. Just wanted to know I'm not ignoring you (well, I am, but only due to time constraints 😉 )

[GreatBarrier86]

No worries! Glad the report made sense!

[rockfordlhotka]

I believe I have this resolved for 4.11.2. That's the good news @GreatBarrier86 !

There was a change in 5.0 (#1080) that actually masks the underlying issue with MobileFormatter and its interaction with UndoableBase. As a result, I am having a hard time porting the fix forward to 5.0.0 - more research is required.

[GreatBarrier86]

@rockfordlhotka, great to hear about 4.11.2. How does this cause problems for 5.0? I probably cant offer advice but i'm curious to know. the link you provided has a lot of stuff in it so i can't fully appreciate the context like you can.

[rockfordlhotka]

Basically for this issue I dug down to the root issue, which is that we special-cased the use of MobileFormatter within UndoableBase rather than having MobileFormatter serialize the state stack like the other formatters.

So what I did to fix this now is make UndoableBase use MobileFormatter to serialize all its data when appropriate (depending on clone or undo) - including the state stack. The result is parity with the other formatters.

Unfortunately in the meantime @Hadronicus ran into a somewhat related issue to what you reported, and we came up with what seemed like a good solution at the time - basically a workaround for the fact that MobileFormatter was being treated in a special way.

Of course now that MobileFormatter works like the other formatters, it turns out that the "solution" in #1080 is a problem. Basically a special case to fix a special case 😢

I think I see a simple fix to that issue. But I am exploring a more comprehensive fix that'll eliminate a whole bunch of other special case code between NS2 and full .NET - mostly in FieldData and FieldDataManager.

[b-higginbotham]

This is possibly related to #1125 which I discovered after implementing the fix for #1080. I’ll be able to take a closer look at it in the morning.

[rockfordlhotka]

@Hadronicus You should wait until I'm done with the changes for this issue in v5 - right now the surgery I'm doing is pretty deep and there's no sense looking at anything to do with n-level undo until I either roll this forward or discard what I'm doing.

I'm literally consolidating all the n-level undo stuff in FieldDataManager and FieldInfo so there's no difference between NS2 and full .NET.

[GreatBarrier86]

To me, it seems that the fix to remove special handling for NS2 would be the preferable option. That would yield a more consistent result across platforms, I’d imagine. It also seems scary and could break things at the onset, but would probably be better in the long run.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.