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

Properties not passed to Feathers are getting set to null #140

Closed
green3g opened this issue Jan 26, 2018 · 2 comments
Closed

Properties not passed to Feathers are getting set to null #140

green3g opened this issue Jan 26, 2018 · 2 comments

Comments

@green3g
Copy link
Contributor

green3g commented Jan 26, 2018

Related: #55

Steps to reproduce

There are fields I exclude from my client side application to not allow editing them. These fields are not passed to the endpoint and are being set to null by feathers-knex. This is what I would consider an unexpected behavior.

I would like to change this so that fields that are not passed into the rest endpoint are kept as is.

https://github.com/feathersjs-ecosystem/feathers-knex/blob/master/lib/index.js#L288

      for (var key of Object.keys(oldData)) {
        if (data[key] === undefined) {
          newObject[key] = oldData[key] || null; // <--note reusing old data 
        } else {
          newObject[key] = data[key];
        }
      }

Expected behavior

Keys that are not passed to feathers should not be changed.

Actual behavior

These keys are set to null.

Thoughts??

Module versions (especially the part that's not working):
feathers-knex@3.0.1

NodeJS version:
8.9.4

Operating System:
Windows 7

Browser Version:
Chrome 64

Module Loader:
Steal.js

@daffl
Copy link
Member

daffl commented Jan 26, 2018

It is making a copy and not modifying the original object. The behaviour you are seeing is as intended by the update method:

completely replaces a single record identified by id with data

So the old data will be replaced by the new, removing existing fields if they don't exist in the new data. If this is not what you want then you should patch instead.

@green3g
Copy link
Contributor Author

green3g commented Jan 26, 2018

Ahh, that makes sense. Patch is definitely what I was looking for. Thanks @daffl .

@green3g green3g closed this as completed Jan 26, 2018
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

No branches or pull requests

2 participants