codepunkt / mongoose-patch-history

Mongoose plugin that saves a history of JSON patch operations for all documents belonging to a schema in an associated 'patches' collection
MIT License
96 stars 21 forks source link

Is this repo still active? #53

Closed vinerich closed 4 years ago

vinerich commented 4 years ago

Hey together,

I'd like to use this plugin in a current project. At the moment I got the issue that the Fix null pointer commit 450c5ebc6c2e1270cab5de636882d4b08ddae393 is not included in the latest release. I could work around that with only using the save() function but that wouldn't be my first priority.

So I'm asking if you will find the time to make another release or if one can help out and fix the outstanding problems to make the release as easy as possible for you.

codepunkt commented 4 years ago

The commit from master that you're referring to is not part of 1.3.0?

vinerich commented 4 years ago

Atleast on my side it isn't. I redownloaded dependencies to check but this are the sources.

package.json

"dependencies": {
    // ...
    "mongoose-patch-history": "^1.3.0",
}

node_modules/mongoose-patch-history/lib/index.js

  function postUpdateOne(result, next) {
    var _this3 = this;

    if (result.nModified === 0) return;

    var conditions = Object.assign({}, this._conditions, this._update.$set || this._update);

    this.model.findOne(conditions).then(function (doc) {
      // comment added by myself, the null check should take place here but it is not there
      doc._original = _this3._original;
      return createPatch(doc, _this3.options);
    }).then(function () {
      return next();
    }).catch(next);
  }
codepunkt commented 4 years ago

That's confusing. Let me check!

codepunkt commented 4 years ago

Published version 1.4.0 based on current master with a few dependency updates. Does this help?

vinerich commented 4 years ago

Thank you for reacting so fast! For me the problem is resolved. Edit: I must say the initial issue is resolved but I think that #26 was not getting to the root of the problem. In #22 the issue is reproduced with an _id: 'inexistent, so the fix of just checking the doc on undefined is understandable. For me this issue also exists when the _id exists. I'll try to investigate it in the coming days and provide a PR if I can find the problem.

But if I have you here right now. What can I provide to get #33 merged? I'd really like this feature and if you tell me what you need I'll happily do so.

vinerich commented 4 years ago

I think I'll take on #39 and #43 when I get the time. Also things I would need in the future. I'm not a native javascript developer so I probably don't bring the best code there, but if you are ok with that and got the time to review that should'nt be the problem.

vinerich commented 4 years ago

@codepunkt can you tell me with which version of MongoDB you ran the tests? For me v4.2.5 Community has some failing tests.

Mostly timeouts because the promise of the mongoose query is not returning.

codepunkt commented 4 years ago

Some 3.x version. Whats a typical version thats used nowadays? We should maybe set this up for CI aswell, that might also be running on an old version

vinerich commented 4 years ago

According to the official release-notes I would say 3.x is going to be EOL soon. I'm always trying to run the latest stable so I would propose 4.2 or 4.4. AFAIK travis provides no way to specify the mongodb version if you initialize it with services: mongodb. However pymodm has a working version of different mongodb versions.

I'm still not sure what exactly causes the issues I am seeing. I'll open a PR and would propose we move our conversation.