search

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

Maintenance and compatability with newer mongoose #86

Closed jdegger closed 2 years ago

jdegger commented 2 years ago
vinerich commented 2 years ago

Hey @jdegger, sorry that I missed your PR. It is still worth looking at or will you publish your own version?

On the first glance I don't see any issues with your changes, but I'll need to have a closer look. I can make time for it this weekend.

Do you need additional changes which require a fast release cycle or would it be it? I would prefer to not publish the plugin 2 times but I can't hold you from that if we respond so late 😄

jdegger commented 2 years ago

Hi @vinerich we have published our own version for internal use. However I think it might be beneficial to others as well to have support for these changes.

I might look into supporting virtuals (somehow...?) for updateOne and updateMany as well. At the moment the .set method is not called for those instances thus resulting in not populating my virtual in the database. However this is unrelated to my PR.

jdegger commented 2 years ago

ps: i just published my own version, but I now see these changes of course are also included in this PR. That should not happen

vinerich commented 2 years ago

@jdegger I'll have a look and will report back.

I however edited one test, see commit. I do not see why this should create a patch

What do you mean by this line?

Ofc the changes for publishing your version shouldn't be included but that can easily be fixed I'd say.

renamed the internal .data method as I couldn't see why this should not have a less common name

This may break some custom implementations or extensions. I'll see.

jdegger commented 2 years ago

This may break some custom implementations or extensions. I'll see.

I understand, I have problems with this because I was already using the data field in my existing models. It would also be possible to make this configurable (and thus backwards compatible) of course

vinerich commented 2 years ago

This would probably be a good option. I understand the need to define something less common. Maybe you can edit out the publishing changes and I'll have a look later today or tomorrow.

jdegger commented 2 years ago

Unfortunately I have some meetings the rest of today but I might be able to look into it next week. Of course you can also propose changes if you want to go ahead. Thanks in advance!

vinerich commented 2 years ago

I think I'll close this PR and bring in the changes in separate PR's for @codepunkt to check. I hope that fits you.

ypicard commented 2 years ago

Hi, I'm looking into adding this dependency to my project and am trying to get a feel of the project's status. Is this PR going to be merged or closed at some point? What is the status here? Thanks!

vinerich commented 2 years ago

@codepunkt

Am 10. Januar 2022 23:43:32 MEZ schrieb ypicard @.***>:

Hi, I'm looking into adding this dependency to my project and am trying to get a feel of the project's status. Is this PR going to be merged or closed at some point? What is the status here? Thanks!

-- Reply to this email directly or view it on GitHub: https://github.com/codepunkt/mongoose-patch-history/pull/86#issuecomment-1009419693 You are receiving this because you were mentioned.

Message ID: @.***>

jdegger commented 2 years ago

(I do not have much time to work on this PR besides what I have already done, if somebody can pick up where I left off, that would be great)

vinerich commented 2 years ago