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

AutoRefresh + Transform + AsObservableList does not work as expected #220

Closed
Wouterdek opened this issue Apr 14, 2019 · 4 comments
Closed
Labels

Comments

@Wouterdek
Copy link
Contributor

Wouterdek commented Apr 14, 2019

The following test fails on the assertion line:

public void TestRefreshTransformAsList()
{
    SourceList<Example> list = new SourceList<Example>();
    var valueList = list.Connect()
        .AutoRefresh(e => e.Value)
        .Transform(e => e.Value, true)
        .AsObservableList();

    var obj = new Example {Value = 0};
    list.Add(obj);
    obj.Value = 1;
    Assert.AreEqual(valueList.Items.First(), 1); //valueList contains 0 instead of 1 here
}

Autorefresh correctly detects the change in value and pushes a refresh change.
Transform receives this and updates its internal list using ChangeAwareList.Refresh.

In the final step, the refresh update arrives at the SourceList<int> (valueList) that should be updated. The changes are applied using ListEx.Clone and here stuff goes wrong. The change is a Refresh change, and the list is a ChangeAwareList so RefreshAt is used instead of RemoveAt and Insert. However, RefreshAt does not update the data in the list so the result is wrong.

@RolandPheasant
Copy link
Collaborator

That's a good bit of analysis. Have you thought of a solution?

@Wouterdek
Copy link
Contributor Author

I'm not entirely sure to be honest. Maybe refresh changes that go through a transform should come out as a replace change if the result of the transformation has changed?

@RolandPheasant
Copy link
Collaborator

The issue is in the transformer class. The item is transformed, but a refresh change is sent whereas it should be a replace.

The fix is easy but some other unit tests break as a result.

@Wouterdek
Copy link
Contributor Author

Great work, thanks!

@lock lock bot added the outdated label Jul 22, 2019
@lock lock bot locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants