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

Implement handling of command rejections #86

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

dpikhulya
Copy link
Contributor

@dpikhulya dpikhulya commented Dec 14, 2024

This PR generalizes CommandOutcomeHandler to a more capable CommandLifecycle object, which can be configured to handle different kinds of situations that can happen when posting the command:

  • One or more possible expected non-rejection events, which are considered as a "positive" command posting outcome (which typically closes the dialog as expected).
  • Zero or more possible expected rejection events, which can come after posting the command. By default, CommandLifecycle displays a configurable message for them, but it's possible to override this behavior either for all rejections at once, or on a per-rejection basis.
  • Posting errors.
  • Outcome waiting timeouts.

By default it displays configurable messages upon either of the "negative" scenarios, but handlers for those scenarios can be customized.

Here are some usage examples (see the KDocs for all configuration parameters):

    val command: SomeCommand = someCommand()
    val succeeded: Boolean = command.post(
        CommandLifecycle({
            event(
                ExpectedEvent::class.java,
                ExpectedEvent.Field.id(),
                command.id
            )
            rejection(
                ExpectedRejection::class.java,
                ExpectedRejection.Field.id(),
                command.id
            )
        })
    )

An example of per-rejection and common (all-rejecctios) custo handling:

    val command: SomeCommand = someCommand()
    val succeeded: Boolean = command.post(
        CommandLifecycle(
            {
                event(
                    ExpectedEvent::class.java,
                    ExpectedEvent.Field.id(),
                    command.id
                )
                rejection(
                    ExpectedRejection1::class.java,
                    ExpectedRejection1.Field.id(),
                    command.id
                )
                rejection(
                    ExpectedRejection2::class.java,
                    ExpectedRejection2.Field.id(),
                    command.id
                ) handledAs {
                    showMessage("Custom rejection message for ExpectedRejection2")
                    anyOtherCodeForThisRejection()
                }
            },
            onRejection = { command, rejection ->
                showMessage("Rejection ${rejection.javaClass.simpleName} was " +
                        "received as an outcome for command ${command.javaClass.simpleName}")
            }
        )
    )

Besides, this PR contains:

  • Updates of CommandMessageForm, CommandWizard, and CommandDialog implementations to use the new CommandLifecycle object instead of the old limited event subscription that they were using up to now.
  • Dialog revisions:
    • Renamed the notion of dialog's "display mode" to "window type" as that is a more precise term for what happens, and extracts the respective window type implementations into a separate file.
    • Renamed Dialog's dialogWidth/dialogHeight properties to width/height (to remove the redundancy in naming).

@dpikhulya dpikhulya self-assigned this Dec 14, 2024
@dpikhulya dpikhulya marked this pull request as ready for review December 17, 2024 00:55
@dpikhulya dpikhulya requested a review from armiol December 17, 2024 00:55
@armiol
Copy link
Contributor

armiol commented Dec 17, 2024

@dpikhulya
Let me suggest an alternative approach:

  val command: SomeCommand = someCommand()
    val succeeded: Boolean = command.post(
        SubscribeTo({
            events<ExpectedEvent>(
                ExpectedEvent.Field.id(),
                command.id
            ),
            rejections<PossibleRejection>(
                PossibleRejection.Field.id(),
                command.id
            ) handledAs { ... }
        })
    )

Notice the change in wording. And also, the generic parameter (which you can infer in Kotlin, AFAIK), instead of the first argument.

Do I understand correctly that the subscription by any field (not just by Field.id() is assumed?

@armiol
Copy link
Contributor

armiol commented Dec 17, 2024

@dpikhulya another question is why we have this duality in your code:

rejection( ... ) handledAs { ... }
// ...
// and then suddenly, again
onRejection { ... }

To me this looks like two ways to do the same. And this is somewhat confusing.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dpikhulya please see my comments.

@armiol
Copy link
Contributor

armiol commented Dec 17, 2024

@dpikhulya one more thing. Events and rejections in Spine know about their origin via EventContext, so theoretically, there is no need to specify the ID when subscribing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants