SoftDelete2 use null or -1

[Sicria]

It is possible to specify the value which is used, as currently null does not work as it's expecting -1.

I think the use of null is fair for rows which have not been deleted, whereas -1 works for rows which have been deleted but have been restored.

[rodeyseijkens]

Yes I agree, the softDelete2 is actually broken/not working for me.

In a SQL database for example you can't have a -1 as a typeDATETIME, cause that always needs to be a proper date. When you submit -1 it will change it to a DATETIME and then it will be the 1970 date and then the if statement is broken.

It checks if the value > -1 or >= 0
code Line 93

const result = context.result ? context.result.data || context.result : null;
if (skipProbeOnGet && result && result[deletedAt] >= 0) {
  throw new errors.NotFound(deletedMessage, { id: context.id });
}

code Line 155

if (record[options.deletedAt || defaultDeletedAt] > -1) {
  throw new errors.NotFound(deletedMessage, { id: context.id });
}

I copied the hook and changed that line to:

const result = context.result ? context.result.data || context.result : null;
const resultDeletedAt = result[options.deletedAt || defaultDeletedAt];
if (skipProbeOnGet && result && resultDeletedAt && resultDeletedAt !== -1) {
  throw new errors.NotFound(deletedMessage, { id: context.id });
}

const recordDeletedAt = record[options.deletedAt || defaultDeletedAt];
if (recordDeletedAt && recordDeletedAt !== -1) {
  throw new errors.NotFound(deletedMessage, { id: context.id });
}

Everywhere else where the deletedAt is set to -1, I put it to null instead.

Hotfix file I'm using right now

[Sicria]

I ended up creating a hook which does this for me.

import { iff, softDelete2, callingParams } from 'feathers-hooks-common';
import set from 'lodash/set';
import get from 'lodash/get';
import moment from 'moment';

const probeCall = async (context) => {
  const params = callingParams({
    newProps: { provider: undefined },
    hooksToDisable: ['softDelete2'],
  })(context);

  const result = await context.service.get(context.id, params);

  // Modify the result to be what the internal `getActiveRecord` function is expecting
  if (get(result, 'deletedAt') === null) {
    return {
      ...result,
      deletedAt: -1,
    };
  }

  return result;
};

const patchCall = async (context) => {
  const params = callingParams({
    query: Object.assign({}, context.params.query, { deletedAt: null }),
    newProps: { provider: undefined },
    hooksToDisable: ['softDelete2'],
  })(context);

  return context.service.patch(context.id, { deletedAt: moment.utc().toISOString() }, params);
};

export default function softDelete() {
  return iff(
    context => !(context.params.$disableSoftDelete2 && (context.method === 'get' || context.method === 'patch')),
    softDelete2({
      probeCall,
      patchCall,
    }),
    (context) => {
      // Replaces the -1 logic with null since our fields are date|null not timestamp|integers
      if (get(context, 'params.query.deletedAt') === -1) {
        set(context, 'params.query.deletedAt', null);
      }

      // Return the potentially mutated context
      return context;
    },
  );
}

[FossPrime]

That would be a breaking change.
Correct AJV for softDelete2

'deletedAt': {
      'type': 'integer'
    }

Docs say it's using deletedAt: Date.now()... that's a js unix-y timestamp, an integer.