updateAttributes on model calls validation for other fields

[kussberg]

Hello,

we have noticed a problem, that when we use the update of specific fields and before that we queried data with "fields:{...}" it is throwing validation errors for other fields, that are out of the update scope. For example:

Required: title, description
Query: fields{title:true}
Update: title: "aaa"
Error: description is required

This should be fixed to validate only the attributes, that are the part of update scope?

[florianheinze]

I experience the same. I had to create (or better extend) a dirty workaround:

// in common/models/my-model.js
MyModel.observe('before save', function(ctx, next) {

  // TODO this is another ugly workaround to trigger validation also for updates
  // i expect this is a bug: https://github.com/strongloop/loopback-datasource-juggler/issues/771
  if (ctx.data) {
    var d = new MyModel(ctx.data);
    d.isValid(function (valid) {
      if (valid) {
        next();
      } else {
        // TODO dirty workaround to detect if an unmodified field is claimed as invalid
        // https://github.com/strongloop/loopback-datasource-juggler/issues/759
        for (var k in ctx.data) {
          if (d.errors[k]) {
            next(d.errors);
            return;
          }
        }
        next();
      }
    }); // is valid function
  } else {
    next();
  }

});

Note that there are actually two workarounds. There seems to be another scary bug that the validation isn't triggered at all for (some?) updates (#771). I am not sure if that workaround is necessary for this bug.

Maybe it helps someone.

[Amir-61]

Yes this is a bug; closing in favour of #771

[soyacz]

Hello,
I understand that bug #771 is regarding ignoring validation for updateAll method. But still issue with validation of fields out of scope in updateAttribute method occurs. I suggest to reopen this bug.

[dehypnosis]

I thinks certainly a bug too

[vkruoso]

I think this is a separate but related issue considering #771. Here we are concerned that we always need to send a valid object to the API even if we want to update a single field. Over there, the validation not being triggered is the problem.

There is any updates related to this? I'm trying to update an user record, but it is asking me to send its password because it is a required field when creating the user.

[thijsvanbuuren]

This is still an issue in my opinion, partial updates cause all kinds of validation errors on fields we aren't even changing.

I've tested it with the latest versions of loopback v3 but i'm still running into this problem.

[barocsi]

@bajtos Still an issue. Repopen suggested. Why is this being ignored? Seems a design problem.

[kjdelisle]

Is this still happening in 3.x, or is it 2.x that's having the issue?
Please respond here with the versions that are running into this validation issue and @bajtos and I will take a look.

[barocsi]

yes, 3.3.0.

[michaelfreund]

const user = await userModel.findById(<id>);
user.updateAttributes({ status: 1 });

This operation triggers the following error

The 'user' instance is not valid. Details: 'email' Email already exists.

Is this behavior really intended or is it a bug?

[huitre]

Same problem here with version 3.x, any update on this problem?

[rafaelgom3s]

running into the same problem as @michaelfreund in version 3.15.5, but I can pinpoint that the problem in my case was that indeed there was a duplicate user with the same email, once I've removed the duplicated user the problem was gone.

[dhmlau]

@bajtos , any ideas?

[bajtos]

I have two comments:

• For long term, we need to implement partial validations. user.updateAttributes({ status: 1 }); should validate only the status property and ignore everything else. Right now, we always validate all properties.

• As for the error The 'user' instance is not valid. Details: 'email' Email already exists., this may be a bug in the unique validator and or the configuration of email validations. Since the user instance was fetched from the database, it should be considered as "not new" and therefore the email validation should allow a seemingly duplicate email value if the database record with the same email is actually the same record we are trying to update.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

[Allan1]

+1

[bajtos]

Essentially, we need to run a partial validation that will validate only properties affected by updateAttribtes or updateAll request. I am afraid this is way too difficult to achieve in the current LB 3.x code base. We are aware of this issue and would like to eventually address it in LoopBack 4.x (or later) - see the discussion in loopbackio/loopback-next#1872.

I am going to lock this issue down, in order to move the discussion to loopbackio/loopback-next#1872.