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] ValidationContext is not producing updates. #19

Closed
BenWhite27 opened this issue Apr 26, 2019 · 8 comments · Fixed by #20
Closed

[BUG] ValidationContext is not producing updates. #19

BenWhite27 opened this issue Apr 26, 2019 · 8 comments · Fixed by #20
Assignees
Labels
bug Something isn't working outdated

Comments

@BenWhite27
Copy link

Describe the bug
ValidationContext does produce updates and current values do not reflect correct validation status or update in response to property changes.
Tested in a UWP project.

Steps To Reproduce
Create a Simple ViewModel and View with associated Bindings.
Create simple validation rules for the ViewModel
Create subscriptions to various Validation public APIs (ValidationContext.ValidationStatusChange, ValidationHelper.ValidationChange, etc.)
Run project and update text in View Controls
Observe that the behavior isn't as expected.

Expected behavior
I'd expect the ValidationContext to produce Observable sequeneces indicating the current status of the ViewModel's validation.

Environment

  • OS: Windows 10
  • Version 1809+
  • Device: Custom build PC and Surface Pro 4

Additional context
I'm seeing updates to ValidationHelper.ValidationChanged in accordance with the associated Validation Rule, but no updates beyond the initial values from ValidationContext.

I've attached a bare bones project I've been using to identify this.
ReactiveUI.Validation.Sample.Uwp.zip

@BenWhite27 BenWhite27 added the bug Something isn't working label Apr 26, 2019
@open-collective-bot
Copy link

open-collective-bot bot commented Apr 26, 2019

Hey @BenWhite27 👋,

Thank you for opening an issue. We will get back to you as soon as we can. Also, check out our Open Collective and consider backing us.

https://opencollective.com/reactiveui

PS.: We offer priority support for all backers. Don't forget to add priority label when you start backing us 😄

An advanced, composable, functional reactive model-view-viewmodel framework for all .NET platforms!

@BenWhite27
Copy link
Author

Some more info.

View Code Behind

this.Bind(this.ViewModel, vm => vm.Username, v => v.userTxt.Text)
    .DisposeWith(disposables);
this.Bind(this.ViewModel, vm => vm.Password, v => v.passTxt.Password)
    .DisposeWith(disposables);
this.BindCommand(this.ViewModel, vm => vm.Login, v => v.loginBtn)
    .DisposeWith(disposables);
this.BindValidation(this.ViewModel, v => v.errorsTxt.Text)
    .DisposeWith(disposables);

ViewModel

public class SampleViewModel : ReactiveObject, ISupportsValidation
{
    private ValidationHelper valHelper1;
    private ValidationHelper valHelper2;

    public SampleViewModel()
    {
        this.Login = ReactiveCommand.Create(() => { Debug.WriteLine("Login Invoked"); }, this.IsValid());
        this.SetupValidations();
    }

    [Reactive] public string Username { get; set; } = "";
    [Reactive] public string Password { get; set; } = "";
    public ReactiveCommand<Unit, Unit> Login { get; }

    public ValidationContext ValidationContext { get; } = new ValidationContext();

    private void SetupValidations()
    {
        this.valHelper1 = this.ValidationRule(vm => vm.Username,
            username => !String.IsNullOrWhiteSpace(username),
            "The username cannot be empty");
        this.valHelper1.ValidationChanged.Subscribe(change =>
        {
            Debug.WriteLine("valHelper1.ValidationChanged");
        });

        this.valHelper2 = this.ValidationRule(vm=>vm.Password,
            password => password?.Length > 6,
            "The password must be longer than 6");
        this.valHelper2.ValidationChanged.Subscribe(change =>
        {
            Debug.WriteLine("valHelper2.ValidationChanged");
        });

        this.ValidationContext.ValidationStatusChange.Subscribe(change =>
        {
            Debug.WriteLine("ValidationContext.ValidationStatusChange");
        });

        this.ValidationContext.Valid.Subscribe(change =>
        {
            Debug.WriteLine("ValidationContext.Valid");
        });

        this.IsValid().Subscribe(valid =>
        {
            Debug.WriteLine("this.IsValid()");
        });
    }
}

So whilst running the above and updating text in the ui controls I'll see debug console output like the following.

// These are the initial values coming from the observables.
valHelper1.ValidationChanged
valHelper2.ValidationChanged
ValidationContext.ValidationStatusChange
ValidationContext.Valid
this.IsValid()

// These are during the test
valHelper1.ValidationChanged
valHelper2.ValidationChanged
valHelper2.ValidationChanged
valHelper1.ValidationChanged
Login Invoked

So whilst the rules are updating just fine the state for the whole object is valid all the time and allows me to execute the command.

@glennawatson
Copy link
Collaborator

@alexmartinezm Mind having a look at this one?

@alexmartinezm
Copy link
Contributor

I will check it out over the weekend. Thank you for reporting this @BenWhite27.

@alexmartinezm alexmartinezm added this to To do in Bug Stomping via automation Apr 26, 2019
@alexmartinezm alexmartinezm self-assigned this Apr 26, 2019
@BenWhite27
Copy link
Author

BenWhite27 commented Apr 26, 2019

Thanks all. I'll download the source myself tonight when I get home and see if I can pin-point it further, I'm still a rookie with Reactive stuff though 😕

Edit - 16:54
More info.
Adding another ValidationRule at a later point via a command suddenly triggers an update from all objects and I can see some validation errors displayed in the UI. The first 2 rules then seem to function properly but the newly added rule doesn't behave correctly, almost as though the text it's producing is 1 property change behind the current value.

@alexmartinezm
Copy link
Contributor

Hi there, I'm trying to fix this bug, but I cannot reproduce this on a XF project and I don't have a Windows machine. However, I have a VM but the debug session is a bit slow. I hope to get this fixed this weekend.

@BenWhite27
Copy link
Author

BenWhite27 commented May 6, 2019

Thanks @alexmartinezm, I had a look at the code but struggled to get my head around it. If there's anything I can do let me know. I could setup a windows VM with Visual Studio and the repro for you to remote desktop into if that helps?

When I was looking at the code I think everything inside BasePropertyValidation.cs was working, but the flow of sequences doesn't seem to get pass this observable.

_validationConnectable = validationChangedObservable

@worldbeater
Copy link
Collaborator

worldbeater commented May 6, 2019

@BenWhite27 for a temporary solution, try passing the ImmediateScheduler.Instance to your ValidationContext constructor. Instead of doing this:

new ValidationContext();

do this:

// The ImmediateScheduler will make your sample work.
new ValidationContext(ImmediateScheduler.Instance);

Currently, _validationSource and _validations won't stay synchronized without the ImmediateScheduler due to the following code blocks:

.CountChanged()
.Count()
.StartWith(0)
.Select(_ =>
_validations
.Select(v => v.ValidationStatusChange)

This issue takes place here due to DynamicData misuse. Feels like I've broken some stuff while migrating from ReactiveList<T> to SourceList<T>. The _validations collection is empty when _validationConnectable ticks on a scheduler other than ImmediateScheduler. Working on a fix now.

worldbeater added a commit that referenced this issue May 6, 2019
Bug Stomping automation moved this from To do to Done May 6, 2019
@lock lock bot added the outdated label Aug 5, 2019
@lock lock bot locked and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working outdated
Projects
Bug Stomping
  
Done
Development

Successfully merging a pull request may close this issue.

4 participants