-
Notifications
You must be signed in to change notification settings - Fork 32
Fixing missing instantiation error(s) #3259
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
test/e2e/pageobjects/party-mocks.ts:164
- Using the logical OR (||) here forces a falsy activeInstances value (specifically false) to default to an empty array, which may not be the intended behavior. Consider checking explicitly for a false value to ensure clarity in the mock behavior.
cy.intercept('**/active', whatToMock.activeInstances || []).as('activeInstances');
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe do a quick screen-reader test on validation messages popping up in the error report before and after? Otherwise it looks good 💯
# Conflicts: # src/features/instantiate/selection/InstanceSelection.tsx
….tsx
|
Description
This fixes the missing instantiation error that didn't appear when creating a new instance from the instance selection screen.
I added a route for this error, and read the exception to print out the actual error. This also means that you can navigate to that route and just end up with a generic/unknown error.The error is now displayed inside anErrorReport
around the button.It also fixes the same when instantiating via a
InstantiationButton
from a stateless app. I wanted to rework this even more for stateless apps, as you might want to use aIInstantiationValidator
to disallow some prefill values when creating a new instance with prefill (i.e. using theInstantiationButton
withmapping
). However, it doesn't seem like that's possible yet in the backend.If we get a feature request for that, we'd probably have to change both the backend and frontend at that point instead. Optimally the error should be displayed inline as a validation error in the stateless form, such that the user can make changes in the stateless form and retry - but right now I can't find a clear way for a change in the stateless form to make a difference in theIInstantiationValidator
, so I'm scrapping that idea.Update: We've been discussing alternative solutions here: https://digdir.slack.com/archives/C085N9FPDFS/p1744199886413489
Related Issue(s)
InstanceSelection
doesn't render error messages #2578Verification/QA
kind/*
andbackport*
label to this PR for proper release notes grouping