Skip to content
This repository has been archived by the owner on Mar 20, 2022. It is now read-only.

Fixed denormalization skipping of falsy valued ids used in Object schemas #345

Merged
merged 1 commit into from
Mar 6, 2019

Conversation

bsara
Copy link
Contributor

@bsara bsara commented Jul 20, 2018

Problem

Currently, if one has an Object schema that contains properties that are mapped to other entities, if the id of one of those mapped entities is 0 or some other falsy value, it will be skipped during denormalization. Seeing that 0 is not an uncommonly used id, it makes sense to allow non-null/non-undefined falsy values to be evaluated rather than skip them as if they were null or undefined.

Here is an example schema that would not currently allow a categoryOwner with an id of 0 to be denormalized, instead, denormalization of categoryOwner would be skipped and the value of categoryOwner would remain 0:

const userSchema = new schema.Entity('users');
const otherSchema = new schema.Entity('others', {
  categoryInfo: {
    categoryOwner: userSchema
  }
});

Solution

The solution here was very simple. There is a check for see if an object's property evaluates to a falsy value; I simply changed this check from if (object[key]) to if (object[key] != null) so that only null and undefined values would be skipped.

I also added a test to Object.test.js for the expected behavior.

TODO

  • Add & update tests
  • Ensure CI is passing (lint, tests, flow)
  • Update relevant documentation

… that allows for a potentially falsy id is skipped during denormalization
@bsara
Copy link
Contributor Author

bsara commented Jul 23, 2018

@paularmstrong bumping this to see if it can get some love

Copy link
Owner

@paularmstrong paularmstrong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the slow response here. Looks good to me

@paularmstrong paularmstrong merged commit b012f3a into paularmstrong:master Mar 6, 2019
@bsara
Copy link
Contributor Author

bsara commented Mar 26, 2019

@paularmstrong thanks!

@lock
Copy link

lock bot commented Sep 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the Outdated label Sep 24, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants