Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug when create BusinessBindingListBase, internal _deletedList is created with one element #3696

Closed
ProDInfo opened this issue Feb 16, 2024 · 4 comments · Fixed by #3699
Closed
Labels

Comments

@ProDInfo
Copy link
Contributor

ProDInfo commented Feb 16, 2024

Describe the bug
When I create a BusinessBindingListBase the object it's create with one element.
This cause a problem when I wan't to save object, because try to save this item but never are created or removed.
The Csla call to InsertChild to this element

I lookup the code and found the error in file BusinessBindingListBase.cs
The code it's diferent between BusinessBindingListBase and BusinessListBase

The DeletedList it's created with DI and create always one element.

In versión 5.5.3 my same code works well.

Version and Platform
CSLA version: 7.0.3
OS: Windows
Platform: WinForms

Code that Fails

   private MobileList<C> _deletedList;

    /// <summary>
    /// A collection containing all child objects marked
    /// for deletion.
    /// </summary>
    [System.Diagnostics.CodeAnalysis.SuppressMessage(
      "Microsoft.Design", "CA1002:DoNotExposeGenericLists")]
    [EditorBrowsable(EditorBrowsableState.Advanced)]
    protected MobileList<C> DeletedList
    {
      get 
      { 
        if (_deletedList == null)
          _deletedList = (MobileList<C>)ApplicationContext.CreateGenericInstanceDI(typeof(MobileList<>), typeof(C));
        return _deletedList; 
      }
    }

Stack Trace or Exception Detail
No exception, pass to InsertChild the element in DeletedList

Additional context
This code has change to same code that BusinessListBase?

  /// <summary>
    /// A collection containing all child objects marked
    /// for deletion.
    /// </summary>
    [System.Diagnostics.CodeAnalysis.SuppressMessage(
      "Microsoft.Design", "CA1002:DoNotExposeGenericLists")]
    [EditorBrowsable(EditorBrowsableState.Advanced)]
    protected MobileList<C> DeletedList
    {
      get
      {
        if (_deletedList == null)
          _deletedList = new MobileList<C>();
        return _deletedList;
      }
    }
@rockfordlhotka
Copy link
Member

rockfordlhotka commented Feb 20, 2024

Yes, I think the binding list implementation should match the standard list like here

protected MobileList<C> DeletedList

@ProDInfo do you have time to submit PRs to fix this in 7 and 8?

@ProDInfo
Copy link
Contributor Author

@rockfordlhotka

Hi, I create a PR in the main branch.

I modify DeletedList bug and change the order of methods to compare more easy with BusinessListBase.
I found some diferences (more importants), that not include in PR because don't know the implications.
Now resume some of that:

InsertItem don't check if it's IsChild object
image

Chil_UpdateAsync not include in BusinessListBase
image

SetParent identity are not equal
image

If you wan't that include some of that, please let me know

Thank

@rockfordlhotka
Copy link
Member

Thank you @ProDInfo !!

I'd appreciate it if you can resolve the issues you list to get the two types in sync with each other, that would be fantastic!

@ProDInfo
Copy link
Contributor Author

I've made the changes in the PR.

I found a error when reordering the methods in main branch. I removed IsBusy method override, I've fixed in the new PR change

I reviewed the code, and found where the issue was entered:
#1068 Add DI code such that a local dp fetch now works

rockfordlhotka added a commit that referenced this issue Feb 23, 2024
Fixes #3696, DeletedList without DI (branch main)
rockfordlhotka added a commit that referenced this issue Feb 23, 2024
Fixes #3696, DeletedList without DI (branch v7.x)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants