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

tests(insert): add insert tests for json, text array and integer array #5451

Conversation

minh-hoang-trinh
Copy link
Contributor

TLDR: add various tests case for the PR #5431 (rollback of #5321)

the PR #5431 by @caseywebdev should be merged first

A little bit more explanation:

#5365 : user want to insert array of string into text array column, or array of integer into integer array column, which is valid and should be inserted, but the PR #5321 merged sometime ago break it (it will always stringify the array to json string)

@kibertoad At first I thought I can add a check for Array containing at least one plainObject with my previous PR #5444, but turn out it will not solve the #5430, my previous changes will need user to input a valid array of string into text array column

#5430 : user want to insert array of object into text array column (worked in previous version of knex). the PR #5321 break it (it will always stringify the array of object to json string)

@kibertoad
Copy link
Collaborator

@minh-hoang-trinh Fix was merged with #5439, but additional tests are very welcome. Could you please resolve conflicts in your PR, then I could merge it?

@minh-hoang-trinh minh-hoang-trinh force-pushed the tests/add-insert-tests-json-text-array-integer-array branch from 0bc30d5 to f6c7956 Compare January 17, 2023 23:44
@minh-hoang-trinh
Copy link
Contributor Author

@minh-hoang-trinh Fix was merged with #5439, but additional tests are very welcome. Could you please resolve conflicts in your PR, then I could merge it?

thank @kibertoad, conflicts resolved ^^

@coveralls
Copy link

Coverage Status

Coverage: 92.392% (+0.02%) from 92.372% when pulling f6c7956 on minh-hoang-trinh:tests/add-insert-tests-json-text-array-integer-array into d102fe3 on knex:master.

@kibertoad
Copy link
Collaborator

Thank you!

@kibertoad kibertoad merged commit 5caf526 into knex:master Jan 18, 2023
@minh-hoang-trinh minh-hoang-trinh deleted the tests/add-insert-tests-json-text-array-integer-array branch January 30, 2023 18:03
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.

None yet

3 participants