bsiddiqui / bookshelf-paranoia Goto Github PK

If a model has already been previously soft-deleted, and you try to delete the model again, a NoRowsDeletedError is not thrown. It is instead registered as a successful operation and updates the deleted field accordingly.

There needs to be a check on update to make sure that the record has not been previously deleted. I'm working on a PR to demonstrate/fix the error.

Hello, I am trying to perform a soft delete operation on an already fetched model from the database.
After performing destroy() on a model, a timestamp should be added into deleted_at column and new row should be created.

System_Config.forge()
            .where({device_id: req.body.deviceId})
            .fetch()
            .then(function (config) {
                if(config) {
                      config.destroy()
                            .then(function (deletedConfig) {
                                new System_Config()
                                    .save({
                                       //save new row
                                    })
                                    .then(function (new_config) {
                                        console.log('New system config saved');
                                    });
                            });
                }

Now, when I use above code to perform a soft delete operation, a new row is getting created but deleted_at column remains null for an old row (which is unexpected).

But when I write my code like this

System_Config.forge()
            .where({device_id: req.body.deviceId})
            .fetch()
            .then(function (config) {
                if(config) {
                    System_Config.forge({id: config.get('id')})
                        .destroy()
                        .then(function (deletedConfig) {
                            new System_Config()
                                .save({
                                    //save new row
                                })
                                .then(function (new_config) {
                                    console.log('New system config saved');
                                });
                        });
               }

It is working as expected.
So my question is

• What is the difference between these two implementations (why one works as expected and other does not)?
• Does this mean I need to provide an id of a model explicitly to perform destroy()?

Thanks

postgreSQL can't use ALL_ZERO datetime to query.

failed knex query is like:

{ method: 'select',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [ '5afa50be3a3ff4506727e85e', '0000-00-00 00:00:00', 1 ],
  __knexQueryUid: 'e3f9ee71-38e4-426c-a593-38777954a348',
  sql: 'select "admins".* from "admins" where "admins"."some_id" = ? and "admins"."deleted_at" is null or "admins"."deleted_at" = ? limit ?' }

error: date/time field value out of range: "0000-00-00 00:00:00"
  at Connection.parseE (/app/node_modules/pg/lib/connection.js:553:11)
  at Connection.parseMessage (/app/node_modules/pg/lib/connection.js:378:19)
  at Socket.<anonymous> (/app/node_modules/pg/lib/connection.js:119:22)
  at emitOne (events.js:116:13)
  at Socket.emit (events.js:211:7)
  at addChunk (_stream_readable.js:263:12)
  at readableAddChunk (_stream_readable.js:250:11)
  at Socket.Readable.push (_stream_readable.js:208:10)
  at TCP.onread (net.js:607:20)

I am using MySQL 5.6.35 (MAMP). When i tried to fetch records from the database, supposedly it should fetch all the undeleted data, however all data was fetched and the reason was that the "deleted_at" default value was "0000-00-00 00:00:00" instead of NULL.

I have tried to set the default to null "table.timestamp("deleted_at").defaultTo(null);" and even "table.timestamp("deleted_at").defaultTo(knex.schema.raw("NULL"));" but the database throws an Invalid default value error. I tried to solve that by changing the sql_mode but nothing helps.

My question is how can I change the "WHERE deleted_at IS NULL" to "WHERE deleted_at IS 0000-00-00 00:00:00" .

To be noted, the same project with the same knex migrations works fine on another machine with mysql version 5.7.

Hi, another great plugin from your team!

In many cases when I do a soft delete, I also set an active flag to false to deactivate the model. So for example, I have an account model which contains an active and deleted_at column. If I soft delete it, I'd also like to be able to deactivate that account rather than having to always ensure that when I'm specifying the state of deleted_at when I'm doing a query that involves active.

Is this something that you could see as being valuable?

Thanks!
James

Hi @alanhoff,

I have a problem when trying to soft delete some related model (not all model). Is it support?
Below is my part of code:

            let feedbacks = yield QualityFeedback.fetchAll({
                withRelated: ['user', 'product', 'brand'],
                withDeleted: true,
            })

I want withDeleted: true only for user, product & brand only, not for QualityFeedback model.

This is QualityFeedback model:

bookshelf.plugin(require('bookshelf-paranoia'));

module.exports = bookshelf.Model.extend({
    tableName: 'qualityFeedbacks',
    hasTimestamps: true,
    softDelete: true,

    user: function() {
        return this.belongsTo(Users, 'user');
    },

    product: function() {
        return this.belongsTo(Products, 'product');
    },

    brand: function() {
        return this.belongsTo(Brands, 'brand');
    },
});

Sorry for my bad english :)
I hope you can help me.
Thanks!

Call plugin like this
bookshelf.plugin(require('bookshelf-paranoia'), { field: 'is_delete' })

This permanently deletes row from table instead of setting is_delete flag as true.
I'm using bookshelf 0.10.3 and bookshelf-paranoia 0.10.4 version.

Thanks,
Vaidehi

Hi,

I'm trying to get soft deleted elements with the fetchPage function (added with the pagination plugin).

With the param withDeleted : true I get correctly my element, but, the count result not include the soft deleted elements :/

This is my request :

Model
    .forge()
    .where((qb) => {
        qb.whereNot('deletedAt', null);
    })
    .fetchPage({
        withDeleted : true
    });

And this is the result :

{
    models : [
        { ...mymodel },
    ],
    pagination : {
        page : 1, 
        pageSize : 20, 
        rowCount : 0,
    },
}

EDIT :

After research, the fetchPage method make two requests, one with elements (using limit/offset), one for only count. It's appear, the count request ignore the withDeleted attribute :

select `Routes`.* from `Routes` where `deletedAt` is not null limit ?

select count(distinct `Routes`.`id`) from `Routes` where `deletedAt` is not null and `Routes`.`deletedAt` is null

Have you a solution with this problem ?

Hi,

what do i need to put into my knex-mifgration to get soft-deletion to work.
The documentation didn´t made this clear to me.
Maybe i am missing something?

I format all date columns as ISO timestamps by defining format. However the deleted_at paranoia/softDelete column does not pass through format; because my driver doesn't specially handle date objects this means the old default toString date format is used.

I am able to get softDelete to work according to the documentation on most tables, but on my users table, where I hash passwords, this doesn't seem to work:

`var User = bookshelf.Model.extend({
tableName: 'users',
hasTimestamps: true,
softDelete: true,

initialize: function() {
this.on('saving', this.hashPassword, this);
},

hashPassword: function(model, attrs, options) {
var password = options.patch ? attrs.password : model.get('password');
if (!password) { return; }
return new Promise(function(resolve, reject) {
bcrypt.genSalt(10, function(err, salt) {
bcrypt.hash(password, salt, null, function(err, hash) {
if (options.patch) {
attrs.password = hash;
}
model.set('password', hash);
resolve();
});
});
});
},`

When I remove the initialize and hashPassword functions, everything works as intended.

Hi,

I am using {require: true} as the options passed to destroy method and works fine without the plugin. As is supposed to delete only if exists.

With the plugin I am having:

{ CustomError: No Rows Deleted
    at Promise.resolve.then.then.then (<MY_MODULE_PATH>/node_modules/bookshelf-paranoia/index.js:149:19)

It soft deletes all relations except the current model that I invoked the destroy.

It is supposed to not support {require: true}? If so, it is a bummer to remove from all current models.

I didn't find anything about a recovery for soft-deleted records. Is that considered in this project?

Expected Behavior

Calling Model.fetchAll() to fetch a collection only adds the deleted_at condition once.

select "table".* from "table" where "table"."deleted_at" is null

Actual Behavior

Calling Model.fetchAll(), the outputted SQL has the deleted_at condition repeated twice:

select "table".* from "table" where "table"."deleted_at" is null and "table"."deleted_at" is null

Steps to Reproduce the Problem

1. call Model.fetchAll() on a model that is marked as softDelete: true
2. inspect the sql coming out of knex (I discovered while using mockKnex to write unit tests for some of my code)

Specifications

• bookshelf-paranoia version: 0.11.0
• bookshelf version: 0.12.0
• knex client: pg

Possible Solution

I was able to fix this issue by removing the following line:

This line seems to bind to collection fetching even though we are already binding to that on line 61. I think this line can be removed (or line 61) but I don't know enough about the implementation to know if that'd have negative effects on other parts of the library (or a different db driver).

I'm not using this packaging currently so I'm looking for someone who is interested in helping maintain this project

Hi @alanhoff,

I have a problem when try to soft delete with related model. Is it package support for related model?
Below is my model.

Question Model

'use strict';

const Answer = require('./Answer');

bookshelf.plugin(require('bookshelf-paranoia'));

module.exports = bookshelf.Model.extend({
    tableName: 'questions',
    hasTimestamps: true,
    sofDelete: true,

    answer: function() {
        return this.hasMany(Question, 'question');
    },
});

Answer Model

'use strict';

const Question = require('./Question');

bookshelf.plugin(require('bookshelf-paranoia'));

module.exports = bookshelf.Model.extend({
    tableName: 'answers',
    hasTimestamps: true,
    sofDelete: true,

    question: function() {
        return this.belongsTo(Question, 'question');
    },
});

So, I want when the question was deleted, automatically answers are delete too.

Sorry for my bad english :)
Thanks!

Specifying in this line the time stamp field for deletion
bookshelf.plugin(require('bookshelf-paranoia'), { field: 'deletedat' })
I had to change it in the source code to work. Otherwise it gave me error.

{
"name": "error",
"length": 126,
"severity": "ERROR",
"code": "42703",
"position": "23",
"file": "analyze.c",
"line": "2163",
"routine": "transformUpdateTargetList"
}

And running this code on my controller:
destroyWithParanoia: function destroy(req, res) { //paranoia plugin externo de borrado logico return ( usuarioModel.forge('id', req.params.id).destroy()) // throws NoRowsDeletedError if no rows affected .then(function () { //Funciona pero no guarda en JSON el registro eliminado res.status(201).json({error: false, eliminado:registro}); console.log("\nlos datos fueron eliminados!!!\n"); }).catch(function (err) { res.status(400).json(err); }) }

I came across this issue running queries with option withRelated and polymorphic props. The queries executed against the db end up having their "deleted_at" checks mixed up and break.

In my scenario there are 3 entities: girls, boys, parents. parents have a child prop that can either be a girl or a boy.

const Boy = bookshelf.Model.extend({
  tableName: 'boys',
  softDelete: true
})
const Girl = bookshelf.Model.extend({
  tableName: 'girls',
  softDelete: true
})
const Parent = bookshelf.Model.extend({
  tableName: 'parents',
  child () {
    return this.morphTo('child', Boy, Girl)
  },
  softDelete: true
})

Then, have to create a boy, a parent having that boy, a girl and a parent having that girl:

const waterfall = require('promise-waterfall')

function createBoy() {
  return Boy.forge().save().then(function (child) {
    return child.id
  })
}
function createGirl() {
  return Girl.forge().save().then(function (child) {
    return child.id
  })
}
function createParent() {
  return Parent.forge().save().then(function (parent) {
    return parent.id
  })
}
function setChild(parentId, childId, type) {
  return Parent.forge({
    id: parentId
  }).save({
    child_id: childId,
    child_type: type
  })
}

const tasks = waterfall([
  function () {
    return createBoy()
  },
  function (childId) {
    return createParent().then(function (parentId) {
      return setChild(parentId, childId, 'boys')
    })
  },
  function () {
    return createGirl()
  },
  function (parentBoyId) {
    return createParent().then(function (parentId) {
      return setChild(parentId, parentBoyId, 'girls')
    })
  }
])

Finally, fetch all parents with their children:

tasks.then(function () {
  return Parent.fetchAll({
    withRelated: ['child'],
    debug: true
  })
})

The output is:

{ method: 'select',
  options: {},
  timeout: false,
  bindings: [],
  sql: 'select "parents".* from "parents" where "parents"."deleted_at" is null and "parents"."deleted_at" is null' }
{ method: 'select',
  options: {},
  timeout: false,
  bindings: [ '1' ],
  sql: 'select "boys".* from "boys" where "id" in (?)' }
{ method: 'select',
  options: {},
  timeout: false,
  bindings: [ '1' ],
  sql: 'select "girls".* from "girls" where "id" in (?) and "boys"."deleted_at" is null and "girls"."deleted_at" is null' }
Err: error: select "girls".* from "girls" where "id" in ($1) and "boys"."deleted_at" is null and "girls"."deleted_at" is null - missing FROM-clause entry for table «boys»

As can be seen in the logs, the and "boys"."deleted_at" is null in the last query (girls) is wrong and breaking. In addition, that condition is missing from the second query (boys).

Tested using:

• [email protected]
• [email protected]
• [email protected]

Will really appreciate any help with the issue. Thanks!!!

PS: You can see the whole test file at test-bookshelf-paranoia.js