Relationships do not update on save

[salcedo]

If relationship fields sent from a backend are null - or in the case of a many to many relationship - the contents of an array have changed, pinia-orm does not appear to keep track of these changes by updating the id fields and/or pivot tables.

Please see https://github.com/salcedo/pinia-orm-relationship-unlinking for reproduction of this issue.

[CodeDredd]

I don't know if this is a bug. I try to think about it, because i also get your point that normally it is an update which just updates the relation which in this case is a delete and should update the field.

The thing is if you retrieve a model and don't eager load a relation, than you get it as null. If you save it back than i think it's right behaviour that a inverse relation isn't deleted just by passing null to the relation.

For now surly it should work if you use postId isntead of post.

function unlink() {
  useRepo(Comment).save({
    id: 1,
    postId: null,
    // post: null
  });
  // The expected behavior here is if `post` is null on save, pinia-orm should not retain the `postId`.
  // The current undesired behavior is `postId` is still 1, causing pinia-orm to show the relationship still exists.
  console.log(JSON.stringify(useRepo(Comment).find(1), null, 2));
}

[salcedo]

To clarify, I'm not expecting it to delete the post because a comment is updated with its post set to null. Is there another possible workaround that does not require a backend to send both post and postId in its response?

If you retrieve a model and don't eager load a relation, I think the more precise behavior could be that the relational fields don't exist period - instead of being null. Then, when saving it back, it keeps any relationships because the post property was not present.

[CodeDredd]

I know what you mean. But you have to think more the crud way. Just have for example a hasOne relation which is saved in a different table. In the inverse relation no data is really lost because only the "binding" is. But in the "hasOne" relation an update would delete that record by this defenition which would be against the law 😉. An update should never be able to delete data.

Yes there is an other workaround for you:

export class Comment extends Model {
  static fields() {
    return {
      id: this.uid(),
      userId: this.attr(null),
      user: this.belongsTo(User, "userId"),
      postId: this.attr(null),
      post: this.belongsTo(Post, "postId"),
    };
  }

  static updating (model) {
      // change values before saving
    if (model.post  === null) {
        model.postId = null
    }
  }
}
Comment.entity = "comments";

[salcedo]

I'm satisfied with that workaround and this issue may be closed.

One last question: to make the workaround even better - From within the updating() method:

1. Is there a way to derive which fields are relational without hardcoding for them?
2. If there is, is it also possible to derive which values were passed to e.g. this.belongsTo() - namely "postId" ?

Thanks again for all of the help!

[CodeDredd]

For a cup of coffee ☕ i will answer your question 😜 ...
Yes there is a way:

import { Relation, Model } from 'pinia-orm'

export class Comment extends Model {
  static fields() {
    return {
      id: this.uid(),
      userId: this.attr(null),
      user: this.belongsTo(User, "userId"),
      postId: this.attr(null),
      post: this.belongsTo(Post, "postId"),
    };
  }

  static updating (model) {
      // change values before saving
    if (model.post === null && model.$fields().post instanceof Relation // or "instanceof BelongsTo") {
        const relationForeignKey = model.$fields().post.foreignKey
        model[relationForeignKey] = null
    }
  }
}
Comment.entity = "comments";

[salcedo]

Enjoy your coffee 😃

[CodeDredd]

@salcedo Even per month ! Thanks a lot ❤️ . It means a lot to me! I new name on the readme list inoming 😃.

[CodeDredd]

@salcedo If you haven't done it already you can make a BaseModel and extend from it so every model has this logic:

import { BelongsTo, Model } from 'pinia-orm'

export class BaseModel extends Model {
  static updating (model) {
    const fields = model.$fields()
    for (const name in fields) {
      if (fields[name] instanceof BelongsTo && model[name] === null) {
        model[fields[name].foreignKey] = null
      }
    }
  }
}

export class Comment extends BaseModel { ... }

[salcedo]

Now that I've had a chance to try this, it doesn't work.

Because model[name] is derived, it will always be an undefined property of model. Is there a way to get access to the incoming data in these hooks?

[CodeDredd]

Really? in my unit tests it worked, but maybe you have a differnet approach of data saving. what about trying updated hook and saving the data with useRepo again?

Update: Ok i have your point now. I need to enhance the hooks so it works what you need.

[CodeDredd]

Should be working now with the new release v1.1.0 😉

import { BelongsTo, Model } from 'pinia-orm'

export class BaseModel extends Model {
  static updating (model, record) {
    const fields = model.$fields()
    for (const name in fields) {
      if (fields[name] instanceof BelongsTo && record[name] === null) {
        model[fields[name].foreignKey] = null
      }
    }
  }
}

export class Comment extends BaseModel { ... }

[salcedo]

Forgot to mention this earlier when you said that it worked fine during unit tests:

It seems import { BelongsTo, ... } from "pinia-orm" will work fine during unit tests, but only unit tests.

I worked around this by getting at *.constructor.name which is probably not ideal.

ETA: e.g. instead of importing BelongsTo (seems it's not exported except during unit tests), if (fields[name] instanceof BelongsTo) becomes if (fields[name].constructor.name == "BelongsTo")

Aaaaand. Also now every property of relations are declared public so they can be accessed. So you can make queries dependend on field declarations. looks like you caught it too! :)

[CodeDredd]

You are absoluty right. The classes weren't exported. So this couldn't work ^^. But at least now it does 💯

[salcedo]

Thanks again! I will update the BaseModel idea you provided using these new features and report back if there is any further issues.