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

uSyncItemEventArgs.Cancel is reversed #236

Closed
bachratyg opened this issue May 17, 2021 · 0 comments
Closed

uSyncItemEventArgs.Cancel is reversed #236

bachratyg opened this issue May 17, 2021 · 0 comments
Assignees
Labels
release/8.9.6 Patch release

Comments

@bachratyg
Copy link

Describe the bug
When handling uSyncService.ImportingItem setting e.Cancel=true actually allows processing the item and leaving on false skips the item. The exact reverse of what should happen based on the property name.

When the event is not hooked FireItemStartingEvent defaults to true. When the event is hooked and the handler is empty it defaults to false therefore skipping all items.

To Reproduce

  1. Start from a vanilla umbraco+usync installation
  2. hook the uSyncService.ImportingItem (see skeleton code below)
  3. change something in usync folder and restart the app
  4. the change will not be synced

This reproduces the bug:

public class USyncComposer : ComponentComposer<USyncComponent>
{
}

public class USyncComponent : IComponent
{
	public void Initialize()
	{
		uSyncService.ImportingItem += this.USyncService_ImportingItem;
	}
	public void Terminate()
	{
		uSyncService.ImportingItem -= this.USyncService_ImportingItem;
	}

	private void USyncService_ImportingItem(uSyncItemEventArgs<XElement> e)
	{
		if( condition )
		{
			e.Cancel = true;
		}
	}
}

This works around the bug:

// ...
	private void USyncService_ImportingItem(uSyncItemEventArgs<XElement> e)
	{
		e.Cancel = true;
		if( condition )
		{
			e.Cancel = !true;
		}
	}

Expected behavior
Setting e.Cancel to true should skip the item being imported.

About your Site (please complete the following information):

  • Umbraco Version: 8.11.1
  • uSync Version: 8.9.1 (content edition)
  • Browser N/A

Additional context
I believe this line https://github.com/KevinJump/uSync8/blob/ca25f2eec51127b793d7b372380bcc6318b88141/uSync8.BackOffice/Services/uSyncService_Events.cs#L142 should be

return !e.Cancel;

Note that this would be a breaking change, no clue how to handle this without breaking the whole world.

@KevinJump KevinJump self-assigned this Jul 28, 2021
@KevinJump KevinJump added the release/8.9.6 Patch release label Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/8.9.6 Patch release
Projects
None yet
Development

No branches or pull requests

2 participants