-
Notifications
You must be signed in to change notification settings - Fork 41
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
fix: ensure fields do not have undefined initial values #3246
fix: ensure fields do not have undefined initial values #3246
Conversation
🚀 Deployed on https://pr-3246.dashboard.netlify.dhis2.org |
So, I'm a follower of the idea of only testing the intended public interface of a module (hook in this case) rather than testing implementation details (e.g. isValidUuid, createInitialState). This keeps code cleaner and prevents unintended uses of those "internal" functions. It also makes it easier to identify when a constant or function is not in use and to refactor the implementation. As far as testing goes, in the case of the current tests, it is really easy to do these tests just by testing the hook:
|
This is interesting. Up until now I have mostly given preference to testing the smallest unit in isolation when writing unit tests. And while I do think that in general this is a sensible notion, and that this will actually result in the cleanest code, I do see your other points and will start writing unit tests against the intended public interface in the future. My current approach does involve exporting these internals and I have seen the problems you mention happen in practice. I'll proceed to update the entire spec file. |
…e-modal-DHIS2-19266
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.
Can you add the "Untitled dashboard" placeholder to the title, since that's what it gets set to if the user doesn't type anything. This is also in line with regular dashboard creation.
OK, done 0109115 |
|
## [101.1.4](v101.1.3...v101.1.4) (2025-03-26) ### Bug Fixes * Ensure external dashboard fields do not have undefined initial values ([#3246](#3246)) ([d63eaa6](d63eaa6))
🎉 This PR is included in version 101.1.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Implements DHIS2-19266
Key features
values
andinitialValues
do not contain properties that areundefined
Description
The error we were seeing when opening the modal was caused by the fact that a dashboard did not have a code. So the
initialValues
object provided tocreateInitialState
was not undefined itself, but one of its properties did have a value ofundefined
. The validation logic was assuming each property would default to the value defined in thedefaultInitialValues
object, but this was the case.In this fix I chose to implement a generic fix that scans all
initialValues
properties and if anundefined
value is encountered, it falls back to the default. IMO this preferable over making the validation logic more flexible, or implementing a fix specific to thecode
field.