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

Upsert Can't handle required 'false' boolean values #197

Merged

Conversation

NoahDavey
Copy link
Contributor

When trying to run upsert command, it errors if passing in a boolean attribute that is 'required' and is set to false. It seemed that the dynamo command created includes the attribute in ExpressionAttributeNames but did not pass through the value correctly into the ExpressionAttributeValues object, it instead sets it to undefined. This causes the following error to be thrown by the document client

ElectroError: Pass options.removeUndefinedValues=true to remove undefined values from map/array/set. - For more detail on this error reference: https://github.com/tywalch/electrodb#aws-error

I think I hunted down where the bug came from, it seems it was due to the following line short circuiting on the value of upsertAttributes[field] which was fine in most cases, except for when that value equals false in which case it looks for updatedKeys[field] and then value gets assigned undefined

const value = upsertAttributes[field] || updatedKeys[field];

Im not super familiar with the Electro codebase and just hunted this down with some logs etc, but please feel free to redo if what i have done would break anything. I just figured i could hopefully make a good start :)

@netlify
Copy link

netlify bot commented Dec 16, 2022

Deploy Preview for electrodb-dev canceled.

Name Link
🔨 Latest commit 672f464
🔍 Latest deploy log https://app.netlify.com/sites/electrodb-dev/deploys/639e7890e858fd00096043e5

@tywalch tywalch merged commit cdcc0f4 into tywalch:master Dec 18, 2022
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.

2 participants