Skip to content

Conversation

@ChrisPulman
Copy link
Member

@ChrisPulman ChrisPulman commented May 30, 2021

Added tests to increase codeCov
Added TODO: Create Test to public members without tests to ease the task of creating tests ongoing.
Fixed Bind with trigger to update UI, added test to cover this

What kind of change does this PR introduce?

Bug fix

What is the current behaviour?

Bind with a Observable trigger does not observe the trigger

What is the new behaviour?

Bind with a Observable trigger now observe the trigger and updates the view when the trigger is fired

What might this PR break?

none unless wrong overload was being used previously and still expecting values to update without the trigger

Please check if the PR fulfils these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:
other tests added to increase codeCov

Added tests to increase codeCov
Added TODO: Test Required to public members without tests to ease the task of creating tests ongoing.
Fixed Bind with trigger to update UI, added test to cover this
@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static void WhenActivated(this IActivatableViewModel item, Func<IEnumerable<IDisposable>> block) // TODO: Create Test
{
if (item is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static void WhenActivated(this IActivatableViewModel item, Action<CompositeDisposable> block) // TODO: Create Test
{
if (item is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable WhenActivated(this IActivatableView item, Func<IEnumerable<IDisposable>> block) // TODO: Create Test
{
if (item is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable WhenActivated(this IActivatableView item, Func<IEnumerable<IDisposable>> block, IViewFor? view) // TODO: Create Test
{
if (item is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable WhenActivated(this IActivatableView item, Action<Action<IDisposable>> block, IViewFor view) => // TODO: Create Test
item.WhenActivated(
() =>
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable WhenActivated(this IActivatableView item, Action<CompositeDisposable> block, IViewFor? view = null) => // TODO: Create Test
item.WhenActivated(
() =>
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

Func<InteractionContext<TInput, TOutput>, Task> handler) // TODO: Create Test
where TViewModel : class
where TView : class, IViewFor
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

Func<InteractionContext<TInput, TOutput>, IObservable<TDontCare>> handler) // TODO: Create Test
where TViewModel : class
where TView : class, IViewFor
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

IBindingTypeConverter? viewToVMConverterOverride = null) // TODO: Create Test
where TViewModel : class
where TView : class, IViewFor =>
_binderImplementation.Bind(viewModel, view, vmProperty, viewProperty, signalViewUpdate, conversionHint, vmToViewConverterOverride, viewToVMConverterOverride);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static string ExpressionToPropertyNames(Expression? expression) // TODO: Create Test
{
if (expression == null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static Func<object?, object?[]?, object?>? GetValueFetcherForProperty(MemberInfo? member) // TODO: Create Test
{
if (member is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static Func<object?, object?[]?, object?> GetValueFetcherOrThrow(MemberInfo? member) // TODO: Create Test
{
if (member is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static Action<object?, object?, object?[]?> GetValueSetterForProperty(MemberInfo? member) // TODO: Create Test
{
if (member is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static Action<object?, object?, object?[]?>? GetValueSetterOrThrow(MemberInfo? member) // TODO: Create Test
{
if (member is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static bool TryGetValueForPropertyChain<TValue>(out TValue changeValue, object? current, IEnumerable<Expression> expressionChain) // TODO: Create Test
{
var expressions = expressionChain.ToList();
foreach (var expression in expressions.SkipLast(1))


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static bool TryGetAllValuesForPropertyChain(out IObservedChange<object, object?>[] changeValues, object? current, IEnumerable<Expression> expressionChain) // TODO: Create Test
{
var currentIndex = 0;
var expressions = expressionChain.ToList();


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static bool TrySetValueToPropertyChain<TValue>(object? target, IEnumerable<Expression> expressionChain, TValue value, bool shouldThrow = true) // TODO: Create Test
{
var expressions = expressionChain.ToList();
foreach (var expression in expressions.SkipLast(1))


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static Type? ReallyFindType(string? type, bool throwOnFailure) // TODO: Create Test
{
var ret = _typeCache.Get(type ?? string.Empty);
return ret != null || !throwOnFailure ? ret : throw new TypeLoadException();


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static Type GetEventArgsTypeForEvent(Type type, string? eventName) // TODO: Create Test
{
if (type == null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static void ThrowIfMethodsNotOverloaded(string callingTypeName, object targetObject, params string[] methodsToCheck) // TODO: Create Test
{
var (methodName, methodImplementation) = methodsToCheck
.Select(x =>


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static bool IsStatic(this PropertyInfo item) // TODO: Create Test
{
if (item == null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable AutoPersistCollection<TItem>(this ObservableCollection<TItem> @this, Func<TItem, IObservable<Unit>> doPersist, TimeSpan? interval = null) // TODO: Create Test
where TItem : IReactiveObject =>
AutoPersistCollection(@this, doPersist, Observable<Unit>.Never, interval);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable AutoPersistCollection<TItem, TDontCare>(this ReadOnlyObservableCollection<TItem> @this, Func<TItem, IObservable<Unit>> doPersist, IObservable<TDontCare> manualSaveSignal, TimeSpan? interval = null) // TODO: Create Test
where TItem : IReactiveObject =>
AutoPersistCollection<TItem, ReadOnlyObservableCollection<TItem>, TDontCare>(@this, doPersist, manualSaveSignal, interval);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable AutoPersistCollection<TItem, TCollection, TDontCare>(this TCollection @this, Func<TItem, IObservable<Unit>> doPersist, IObservable<TDontCare> manualSaveSignal, TimeSpan? interval = null) // TODO: Create Test
where TItem : IReactiveObject
where TCollection : INotifyCollectionChanged, IEnumerable<TItem>
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable ActOnEveryObject<TItem>(this ObservableCollection<TItem> @this, Action<TItem> onAdd, Action<TItem> onRemove) // TODO: Create Test
where TItem : IReactiveObject =>
ActOnEveryObject<TItem, ObservableCollection<TItem>>(@this, onAdd, onRemove);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IDisposable ActOnEveryObject<TItem>(this ReadOnlyObservableCollection<TItem> @this, Action<TItem> onAdd, Action<TItem> onRemove) // TODO: Create Test
where TItem : IReactiveObject =>
ActOnEveryObject<TItem, ReadOnlyObservableCollection<TItem>>(@this, onAdd, onRemove);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static bool HasCountChanged(this IChangeSet changeSet) // TODO: Create Test
{
if (changeSet is null)
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IObservable<IChangeSet> CountChanged(this IObservable<IChangeSet> changeSet) => changeSet.Where(HasCountChanged); // TODO: Create Test
/// <summary>
/// Is the change set associated with a count change.


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IObservable<IChangeSet<T>> CountChanged<T>(this IObservable<IChangeSet<T>> changeSet) => changeSet.Where(x => x.HasCountChanged()); // TODO: Create Test
}
}


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changing => // TODO: Create Test
_changing.Value;
/// <inheritdoc />
[IgnoreDataMember]
public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changed => // TODO: Create Test


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<IReactivePropertyChangedEventArgs<IReactiveObject>> Changed => // TODO: Create Test
_changed.Value;
/// <inheritdoc/>
[IgnoreDataMember]


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IDisposable SuppressChangeNotifications() => // TODO: Create Test
IReactiveObjectExtensions.SuppressChangeNotifications(this);
/// <summary>
/// Determines if change notifications are enabled or not.
/// </summary>


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public bool AreChangeNotificationsEnabled() => // TODO: Create Test
IReactiveObjectExtensions.AreChangeNotificationsEnabled(this);
/// <summary>
/// Delays notifications until the return IDisposable is disposed.
/// </summary>


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IDisposable DelayChangeNotifications() => // TODO: Create Test
IReactiveObjectExtensions.DelayChangeNotifications(this);
}
}


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public static IMessageBus Current { get; set; } = new MessageBus(); // TODO: Create Test
/// <summary>
/// Registers a scheduler for the type, which may be specified at runtime, and the contract.


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public void RegisterScheduler<T>(IScheduler scheduler, string? contract = null) => // TODO: Create Test
_schedulerMappings[(typeof(T), contract)] = scheduler;
/// <summary>
/// Listen provides an Observable that will fire whenever a Message is


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<T> ListenIncludeLatest<T>(string? contract = null) // TODO: Create Test
{
this.Log().Info(CultureInfo.InvariantCulture, "Listening to {0}:{1}", typeof(T), contract);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<IChangeSet<IRoutableViewModel>> NavigationChanged { get; protected set; } // TODO: Create Test
[OnDeserialized]
private void SetupRx(StreamingContext sc) => SetupRx();


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public void OnCompleted() => _subject.OnCompleted(); // TODO: Create Test
/// <inheritdoc/>
public void OnError(Exception error) => _subject.OnError(error); // TODO: Create Test
/// <inheritdoc/>


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public void OnError(Exception error) => _subject.OnError(error); // TODO: Create Test
/// <inheritdoc/>
public void OnNext(T value) => _subject.OnNext(value);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IDisposable Schedule<TState>(TState state, TimeSpan dueTime, Func<IScheduler, TState, IDisposable> action) => // TODO: Create Test
AttemptToCreateScheduler().Schedule(state, dueTime, action);
/// <inheritdoc/>
public IDisposable Schedule<TState>(TState state, DateTimeOffset dueTime, Func<IScheduler, TState, IDisposable> action) => // TODO: Create Test
AttemptToCreateScheduler().Schedule(state, dueTime, action);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IDisposable Schedule<TState>(TState state, DateTimeOffset dueTime, Func<IScheduler, TState, IDisposable> action) => // TODO: Create Test
AttemptToCreateScheduler().Schedule(state, dueTime, action);
private IScheduler AttemptToCreateScheduler()
{


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<object> LoadState() => // TODO: Create Test
Observable<object>.Default;
/// <inheritdoc/>
public IObservable<Unit> SaveState(object state) => // TODO: Create Test
Observables.Unit;


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<Unit> SaveState(object state) => // TODO: Create Test
Observables.Unit;
/// <inheritdoc/>
public IObservable<Unit> InvalidateState() => // TODO: Create Test
Observables.Unit;


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<Unit> InvalidateState() => // TODO: Create Test
Observables.Unit;
}
}


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<Unit> IsResuming // TODO: Create Test
{
get => _isResuming.Switch();
set => _isResuming.OnNext(value);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<Unit> IsUnpausing // TODO: Create Test
{
get => _isUnpausing.Switch();
set => _isUnpausing.OnNext(value);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<IDisposable> ShouldPersistState // TODO: Create Test
{
get => _shouldPersistState.Switch();
set => _shouldPersistState.OnNext(value);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<Unit> IsLaunchingNew // TODO: Create Test
{
get => _isLaunchingNew.Switch();
set => _isLaunchingNew.OnNext(value);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@todo
Copy link

todo bot commented May 30, 2021

Create Test

public IObservable<Unit> ShouldInvalidateState // TODO: Create Test
{
get => _shouldInvalidateState.Switch();
set => _shouldInvalidateState.OnNext(value);


This comment was generated by todo based on a TODO comment in 283c6bf in #2772. cc @reactiveui.

@codecov
Copy link

codecov bot commented May 30, 2021

Codecov Report

Merging #2772 (283c6bf) into main (92df299) will increase coverage by 0.67%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2772      +/-   ##
==========================================
+ Coverage   62.84%   63.52%   +0.67%     
==========================================
  Files         134      134              
  Lines        4686     4688       +2     
==========================================
+ Hits         2945     2978      +33     
+ Misses       1741     1710      -31     
Impacted Files Coverage Δ
src/ReactiveUI/Activation/ViewForMixins.cs 81.90% <ø> (ø)
...ngs/Interaction/InteractionBinderImplementation.cs 83.33% <ø> (ø)
...ctiveUI/Bindings/Property/PropertyBindingMixins.cs 95.83% <ø> (+4.16%) ⬆️
src/ReactiveUI/Expression/Reflection.cs 73.18% <ø> (ø)
src/ReactiveUI/Mixins/AutoPersistHelper.cs 86.61% <ø> (ø)
src/ReactiveUI/Mixins/ExpressionMixins.cs 86.53% <ø> (ø)
src/ReactiveUI/Mixins/ObservableLoggingMixin.cs 0.00% <ø> (ø)
src/ReactiveUI/Mixins/ObservedChangedMixin.cs 94.11% <ø> (ø)
...iveUI/Mixins/ReactiveNotifyPropertyChangedMixin.cs 83.33% <ø> (ø)
...I/ObservableForProperty/OAPHCreationHelperMixin.cs 38.40% <ø> (ø)
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92df299...283c6bf. Read the comment docs.

@glennawatson glennawatson merged commit d235ea4 into main Jun 2, 2021
@glennawatson glennawatson deleted the fixBindWithTrigger branch June 2, 2021 01:03
@todo todo bot mentioned this pull request Jun 2, 2021
@ronaldvdv
Copy link

ronaldvdv commented Aug 6, 2021

It seems that since this change, I'm having issues with binding. For example:

  • I have a textbox that I'm binding to some view model property
  • When calling Bind(), I'm passing a trigger - in my case textBox.Events().LostKeyboardFocus()
  • Now since this commit, when I'm opening my view the textbox remains empty (e.g. there's no initial update from vm to vw)
  • When I give focus to the textbox and then move focus to the next control, the textbox is suddenly populated (since the trigger fires)

Has this scenario been tested? It's particularly weird since the trigger should indicate that something in the view has changed - it should not have any influence on the initial update from view model to view. Also see the review comment I added.

@github-actions
Copy link

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

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants