WildGums/Orc.Wizard

Raising the Resumed event required for closing the wizard

Closed this issue · 6 comments

Summary

Since the introduction of the Resumed event it is necessary for all classes derived from WizardBase to call base.ResumeAsync() in order to close the wizard (saving all pages & raising the event). This makes it impossible (short of re-implementing IWizard) to modify the Save behavior- e.g. calling unitOfWork.Save() after the page-save iteration (and aborting in case of failure).

API Changes

Move the pages-save iteration back to the SaveAsync method (which is currently marked as obsolete) and have the ResumeAsync method call it, allowing the user to override the page-save iteration subroutine, before raising the Resumed event (unless of course an exception is thrown by the SaveAsync method).

Intended Use Case

I have already created my own WizardBaseEx (introducing an additional PagesUpdated event)- so the proposed modification would not affect me- but I feel like it might be useful to others.. Here are a few use cases that come to mind (i.e. ways of modifying the save-behavior):

  1. Wrapping the save operation in a unit of work (assuming each page holds its own repository)
  2. Have the pages update the in-memory model-object (something like UpdateExplicitViewModelToModelMappings) and then have it send over to the persistence layer (aborting on failure)
  3. Add progress update: something like _pleaseWaitService.UpdateStatus(currentPage++, nbPages)

If we would introduce RaiseResumed, would that be sufficient?

Generally speaking- yes it would, but it kinda feels like a hack: are there other going to be other Raise* methods, should I be able to call RaiseResumed without a prior call to SaveAsync?
Additionally- keeping the previous SaveAsync behavior would allow people (if any) that are upgrading from v3 to keep their original code.
As I mentioned - I'm not directly affected by these issues either way, so even if you decided to keep the current design- it would be fine- I just wanted to point out 'a possible issue that was maybe overlooked'.

Good points. Will investigate.

We were in a similar situation. Creating a fix on the develop branch now.

The RaiseResumed should now be available.

We renamed the Save => Resumed (because the wizard is "resuming" to the app). Therefore I think it's confusing to keep using the SaveAsync.