hoodiehq / pouchdb-hoodie-api

:dog: Hoodie-like API for PouchDB
https://hoodiehq.github.io/pouchdb-hoodie-api
Apache License 2.0
44 stars 11 forks source link

allow .remove(object) to change properties before deleting #48

Closed gr2m closed 9 years ago

gr2m commented 9 years ago
store.add({id: 'funk'})
then(store.remove.bind(null, {id: 'funk', reason: 'too funky'})
.then(function(object) {
  // object.reason should be 'too funky'
})

With that, I'd also add these variations to the API

store.remove(id, changedProperties)
store.remove(id, changeFunction)
zoepage commented 9 years ago

@gr2m not sure, if I understand why we want to do this. Could you explain please? :)

gr2m commented 9 years ago

When we delete a document in CouchDB, we don't use the DELETE /db/call, but instead use a PUT and only add the _deleted: true property. This way, all the document properties remain in tact and we can for example show a notification that a todo has been removing, including it's properties like title and status.

Now I see (and actually right now have) a use case, where I want to add comment when I delete an object, to explain why I deleted it. That is right now not possible, I need to update it first, then delete. Instead, I'd like the remove method to optionally pass additional properties, so that the deleted revision of an object has them for later reference.

NickColley commented 9 years ago

Working on this.

NickColley commented 9 years ago

I've wrote some failing tests for this incase someone else wants to have a go at this before I get the chance: https://github.com/hoodiehq/pouchdb-hoodie-api/commit/32c55a0b08fe4c8e81c8d7829f68f2b38d469f1a

gr2m commented 9 years ago

Inspired by @kentcdodds's amazing First Timers Only, I'd like to assist a new contributor to do the fix for this issue. Here are the steps it would take out of my head, but please don't hesitate to ping me at any time here in the comments, at http://hood.ie/chat/ or at https://twitter.com/gr2m/.

  1. Clone this repository

    Find more information on Forking at https://guides.github.com/activities/forking/

  2. Install dependencies
    • You will need Node.js installed on your machine
    • You will need a recent npm version, run npm install -g npm@^2.1.0
    • Run npm install in the pouchdb-hoodie-api folder. This could take a moment
  3. Run the tests

    • Run npm test in the pouchdb-hoodie-api folder. The output should look like this. The most important bit is
    total:     255
    passing:   255
  4. Enable the first failing test

    • open [tests/specs/remove.js], look for "test.skip" in line 117
    • replace "test.skip" with just "test"
    • run "npm test" again
    • You should now see two failing assertions
    total:     259
    passing:   257
    failing:   2
  5. Fix the failing test (aka the interesting bit :)

    • api.remove can be called with an "id", and object with a "id" property, or an array of IDs, objects, or a combination of both:
    api.remove(id)
    api.remove(object)
    api.remove([object1, id2])
    • api.remove is implemented in remove.js. In lines 17-18 you see two things:
      1. It differentiates between being called with just an id or an object, or an array
      2. It uses update methods to delete. This is a CouchDB speciality. You can delete an object by adding a _delete property as you can see it in the code. That way, all the other properties remain intact, the object doesn't dissapear entirely, it's just "marked" as deleted.
    • In the test we enabled above, we test that a call of store.remove with an array of objects with changed properties (foo: 'changed') will return an array of objects with the foo property set to changed, so that is what we need to implement now. The updateMany method already supports all we need, what we need to change is to call it with an array of objects that have the _deleted: true property set, instead of passing {_deleted: true} as an extra argument.
    • There are many ways to solve this, I'll leave it up to you :) But I'm here for questions

Note this is the first time I try to make an issue "first time contributer"-friendly. I likely missed a lot in the steps above that I take for granted, but really isn't. So please keep in mind: We can learn as much from you as you can learn from us :)

Good luck!

luoto commented 9 years ago

Thanks guys, I'll give this a shot!

gr2m commented 9 years ago

cool :) Good luck @luoto!

kentcdodds commented 9 years ago

:+1: :confetti_ball: