-
Notifications
You must be signed in to change notification settings - Fork 16
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(molgenis-components): null-check for expressions when a field is emptied after being filled #4189
Conversation
@@ -52,6 +52,10 @@ let props = defineProps({ | |||
|
|||
const emit = defineEmits(["update:modelValue"]); | |||
|
|||
function updateModelValue(value) { | |||
emit("update:modelValue", value == "" ? null : value); |
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.
Please use === instead of ==
apps/molgenis-components/src/components/forms/InputHyperlink.vue
Outdated
Show resolved
Hide resolved
Included |
Quality Gate passedIssues Measures |
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.
see comment
export function updateModelValue(object, value) { | ||
updateModelValueEmit(object.$emit, value); | ||
} | ||
|
||
export function updateModelValueEmit(emit, value) { |
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.
- These contain ts errors (implicit any type). Do you have your IDE setup correctly? It should show red lines on these functions.
- Is it possible to pass this.$emit into the second function? In that case we could remove the first.
- This comment should not be here, comments double the maintenance of code so should be used only if really necessary.
- I suggest renaming the function to better reflect what they do e.g.
emitModelValueNullSafe
This PR is closed in favor of #4353 |
What are the main changes you did:
Fixed a bug where empty value validation was bugged. Once a field is filled, it cannot be put back into its original (
value is null
) state. Similar to #3933.Example:
Schermopname.2024-09-05.om.18.54.56.mov
To be more specific, if having a table with 2 columns (
c1
&c2
), wherec2
can only be filled ifc1
is empty, different code would give different errors:c1 == null
-> only worked as long asc1
is empty, if filled and emptied again the error message would remainc1.length === 0
-> only worked AFTER value was set and then emptied, but when trying to save give the errorUpdate into table 'test' failed.: Transaction failed: script failed: TypeError: Cannot read property 'length' of null
c1 == null || c1.length === 0
-> works but uglyThis PR should address the issue above and make
<colName> == null
always usable. It fixes InputString, InputText, InputEmail, InputHyperlink & InputPassword.Only
InputString
was tested manually with validation string in an actual database table. Other ones have e2e tests to see if value is set tonull
if empty input field.how to test:
validation
for a column,<colName> == null
should now work correctly. F.e. http://localhost:8080/apps/molgenis-components/#/component/InputString and checking ifvalue
is removed (null
) when field is empty.todo: