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.

Sign up for GitHub

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

Jump to bottom

Model.restore is broken when using use$neOperator = false #14338

Open
2 tasks done
supersime opened this issue Feb 7, 2024 · 4 comments · May be fixed by dsanel/mongoose-delete#150
Open
2 tasks done

Model.restore is broken when using use$neOperator = false #14338

supersime opened this issue Feb 7, 2024 · 4 comments · May be fixed by dsanel/mongoose-delete#150
Labels
plugin

Comments

@supersime
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Mongoose version

7.4.2

Node.js version

20.11

MongoDB server version

5.0.24

Typescript version (if applicable)

No response

Description

We have noticed that the Model.restore is removing the field deleted from the document.
This previously used to work.
We are using the use$neOperator with a value of false - in other words, all of our queries look for data where deleted = false as a result of dsanel/mongoose-delete#50.

The following change, made in version 1.0.1 has broken this functionality:

Before this code change, the schema.statics.restore was:

    schema.statics.restore =  function (conditions, callback) {
        if (typeof conditions === 'function') {
            callback = conditions;
            conditions = {};
        }

        var doc = {
            deleted: false,
            deletedAt: undefined,
            deletedBy: undefined
        };

        return updateDocumentsByQuery(this, conditions, doc, callback);
    };

After this code change, the function is:

    schema.statics.restore =  function (conditions, callback) {
        if (typeof conditions === 'function') {
            callback = conditions;
            conditions = {};
        }

        var doc = {
            $unset:{
                deleted: true,
                deletedAt: true,
                deletedBy: true
            }
        };

        return updateDocumentsByQuery(this, conditions, doc, callback);
    };

The issue here, is that the deleted key is being removed from the model, which doesn't support how the use$neOperator works!

I have tested the following which appears to work - could this be considered for the 7.6.x release please?

    schema.statics.restore =  function (conditions, callback) {
        if (typeof conditions === 'function') {
            callback = conditions;
            conditions = {};
        }

        var doc = use$neOperator ? {
            $unset: {
                deleted: true,
                deletedAt: true,
                deletedBy: true
            }
        } : {
            deleted: false,
            $unset: {
                deletedAt: true,
                deletedBy: true
            }
        };

        return updateDocumentsByQuery(this, conditions, doc, callback);
    };

Steps to Reproduce

Configure the mongoose-delete plugin as follows:

import softDelete from 'mongoose-delete';

export const softDeletePlugin = [
    softDelete,
    {
        deletedAt: true,
        deletedBy: true,
        overrideMethods: true,
        use$neOperator: false,
    },
];

When you add this plugin to your schema, delete a document, and then try to restore it:

await Model.restore({ _id})

The deleted key has been removed from the underlying document.

Expected Behavior

The deleted key should remain in the document, and have a value of false
@vkarpov15
Copy link
Collaborator

This GitHub repo is for mongoose, not mongoose-delete, so it would likely be better to report this as a bug on the mongoose-delete GitHub repo.

That being said, this does look like an unintentional bug introduced in mongoose-delete. We will see if we can open a PR to fix

@vkarpov15 vkarpov15 added this to the 8.1.2 milestone Feb 7, 2024
vkarpov15 added a commit to vkarpov15/mongoose-delete that referenced this issue Feb 8, 2024
@vkarpov15
set deleted to false instead of unsetting when calling restore()
7f95506 
Fix Automattic/mongoose#14338
@vkarpov15 vkarpov15 linked a pull request Feb 8, 2024 that will close this issue
Open
@vkarpov15 vkarpov15 modified the milestones: 8.1.2, 8.1.3 Feb 8, 2024
@supersime
Copy link
Author

This GitHub repo is for mongoose, not mongoose-delete, so it would likely be better to report this as a bug on the mongoose-delete GitHub repo.

That being said, this does look like an unintentional bug introduced in mongoose-delete. We will see if we can open a PR to fix

My apologies @vkarpov15 ... silly mistake! Thank you so much!

@vkarpov15 vkarpov15 removed this from the 8.1.3 milestone Feb 14, 2024
@mareksuscak
Copy link

I think this issue can be closed since it indeed relates to mongoose-delete. This issue has also been addressed in a PR that's pending a review and merge.

@vkarpov15
Copy link
Collaborator

I prefer to keep this open until the mongoose-delete PR is merged so we can remember to follow up on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

set deleted to false instead of unsetting when calling restore() vkarpov15/mongoose-delete
3 participants
@mareksuscak @vkarpov15 @supersime