fix: secure flag persisted as string #3430
Conversation
|
@michaldziuba03 at first sight looks like a very good work but I am missing the way the current data is going to be migrated to be able to not break any user using Novu. How would you solve that issue in this PR? |
|
@p-fernandez from my experience mongoose will perfrom casting from a string to boolean when fetching. I think we can do a small test for it. And if not, we can do quite an easily mongo db query for that (one for 'false' and one for 'true') |
|
@scopsy I think it depends because if |
|
I decided to write migration script for changing type...
I had problem with type casting by mongoose but I found What do you think about my migration script? |
|
@p-fernandez / @scopsy Can we please prioritize this PR? |
| console.log('Start migration - update credentials.secure from string to boolean'); | ||
|
|
||
| console.log('Updating from "true" to true...'); | ||
| const resultTrue = await updateTrueValues(); | ||
| console.log(`Matched: ${resultTrue.matchedCount}\nModified: ${resultTrue.modifiedCount} \n`); | ||
|
|
||
| console.log('Updating from "false" to false...'); | ||
| const resultFalse = await updateFalseValues(); | ||
| console.log(`Matched: ${resultFalse.matchedCount}\nModified: ${resultFalse.modifiedCount}`); | ||
|
|
||
| console.log('End migration.'); |
There was a problem hiding this comment.
Use Logger from @nestjs/common rather than console.log so we can track in out telemetry tools.
There was a problem hiding this comment.
sure. In other migration scripts I have seen console.logs so I decided to use them in my script as well.
I switched to Nest.js Logger in recent changes.
|
@scopsy it looks ok from my side |
What change does this PR introduce?
It makes
secureflag persisted as boolean.To be more specific it changes Integration mongoose schema, because
credentials.securehad String type instead of Boolean and for that reason secure was persisted as string.Second thing - I added transformer
@TransformToBoolean, because frontend sends "true"/"false" values for credentials and DTO is typed as boolean what makes dto not reliable. Transformer transforms only "true"/"false" it doesn't work likevalue === "true"where any other value than "true" returns false.New transformer is supposed to be used in combination with
@IsBooleanvalidator. I also dropped my previous validator@IsBooleanAnybecause new transformer + regular@IsBooleanmakes it obsolete and is just better.New transformer can be used in places like dtos for query params where booleans are expected, because their values are always strings.
Why was this change needed?
Closes #3360
Other information (Screenshots)