AmpersandJS / ampersand-model

Observable objects, for managing state in applications.
MIT License
84 stars 31 forks source link

model.destroy() returning opts.success() with no DELETE xhr created #61

Closed damionvega closed 9 years ago

damionvega commented 9 years ago

The model is removed from the collection when category.destroy() is invoked, yet no xhr request is made.

// categories.js
import Collection from 'ampersand-rest-collection'
import xhr from 'xhr'

import Category from './category'
import config   from '../config'

export default Collection.extend({
  url: config.apiUrl + '/categories',
  model: Category,
  mainIndex: '_id',

  create(category) {
    xhr({
      uri: this.url,
      json: category,
      method: 'post',
    }, (err, res, body) => {
      if (err) return alert(err)
      if (body && body.success === false) return alert(body.error)

      category._id = body._id
      this.add(category)
    })
  },

  fetchWithTags() {
    xhr({
      uri: this.url + '?tags=true',
      method: 'get',
      json: true,
    }, (err, res, body) => {
      if (err) return alert(err)
      if (body && body.success === false) return alert(body.error)

      this.set(body)
    })
  },
})
// category.js
import Model from 'ampersand-model'

import cfg from '../config'
import Categories from './categories'

export default Model.extend({
  // urlRoot: cfg.apiUrl + '/categories',

  // url() {
  //   return cfg.apiUrl + '/categories/' + this._id
  // },

  props: {
    _id:  'string',
    name: 'string',
    tags: 'array',
  },
})
// pages/categories.jsx
import app   from 'ampersand-app'
import AmpMixin from 'ampersand-react-mixin'
import React from 'react'
import xhr   from 'xhr'

import { preAuth } from '../util/helpers'
import cfg      from '../config'
import Category from '../models/category'
import Topbar   from '../components/topbar.jsx'
import CategoryItem from '../components/category-item.jsx'

export default React.createClass({
  displayName: 'CategoriesPage',
  mixins: [AmpMixin],

  getInitialState() {
    return {
      categoryInput: '',
    }
  },

  categoryInputChanged(e) {
    this.setState({ categoryInput: e.target.value })
  },

  addCategory(e) {
    e.preventDefault()

    if (this.state.categoryInput.trim() == '') return

    preAuth(() => {
      app.categories.create({ name: this.state.categoryInput })
    })
  },

  deleteCategory(category) {

    console.log('category', category)

    category.destroy({
      success(model, res, opts) {
        console.log('category destroyed!')
        console.log('model, res, opts', model, res, opts)
      },
      error(model, res, opts) {
        console.log('There was an error destroying the category')
        console.log('model, res, opts', model, res, opts)
      },
    })

  },

  render() {
    var {categoryInput} = this.state
    var {categories}    = this.props

    var content = categories
      ? (<ul>
          {categories.map((cat) => {
            if (cat.name) return <CategoryItem key={cat._id} deleteCat={this.deleteCategory} cat={cat}></CategoryItem>
          })}
        </ul>)
      : <h4>Loading categories...</h4>

    return (
      <div className='form-wrap'>
        <header>
          <h1>Categories</h1>

          <form onSubmit={this.addCategory}>
            <input onChange={this.categoryInputChanged} value={categoryInput} type='text' placeholder='Add category'/>
            <button type='submit' className='commit'>Add</button>
          </form>
        </header>

        {content}
      </div>
    )
  },
})

Here's the console output, res is undefined

screen shot 2015-09-15 at 8 00 24 pm
wraithgar commented 9 years ago

Looking at the source, this appears to happen if it thinks the model is new, i.e. if model[model.idAttribute] (in this case model._id) is falsey.

Can you log out category.isNew() and category.toJSON() in your deleteCategory function?

damionvega commented 9 years ago

Yeah that looks like the case. I was looking for this in the documentation, but it seems it was removed? Here's the link on ampersandjs.com under ampersand-model -> destroy: http://ampersandjs.com/docs#ampersand-model-isnew

Here's the output:

categories.jsx:72 category.isNew()
true
categories.jsx:73 category.toJSON() 
Object {_id: "AU_TyfmweKF7y9ngEHq7", name: "computer", tags: Array[3]}

After adding idAttribute: '_id' to the model, DELETE xhr is made and model removed from collection:

category.isNew()
false
categories.jsx:73 category.toJSON() 
Object {_id: "AU_TyfmweKF7y9ngEHq7", name: "computer", tags: Array[3]}
categories.jsx:77 
category destroyed!

So use idAttribute if you plan on using that model for persistence and mainIndex for collection tracking (if your ID !== id)?

Thanks for your help!

wraithgar commented 9 years ago

mainIndex is for checking for existing copies of yourself in ampersand-collection and defaults to the idAttribute if I remember correctly