SQLITE_MISUSE: not an error

[adonisjs]

SQLITE_MISUSE: not an error

[RomainLanz]

Hi @arma-gast !

Can you share with us a sample of your code?

[arma-gast]

Somewhere in app/Model:

class Product extends Lucid {
  colors() {
    return this.belongsToMany('App/Model/Color');
  }
}

Somewhere in app/Http/Controllers:

class SomeController {
  * update(request, response) {
    const productId = request.param('id'),
    data = request.post();
    // validation and error rendering
    const product = yield Product.find(productId);
    // attributes update
    yield product.save();
    yield product.colors().sync(data.productColors || []);
    // redirect
  }
}

I am updating product's attributes with data from post (loaded in data). Earlier, if no colors were selected, I received other error, something about requiring an array in .attach. Now, if no colors selected, I get MISUSE error.

Is that enough?

[RomainLanz]

Yes, that's perfect!

It seems like a bug. Let's wait for @thetutlage.

[thetutlage]

@arma-gast sync should pass references to the detach method here. https://github.com/adonisjs/adonis-lucid/blob/develop/src/Lucid/Relations/BelongsToMany.js#L282

I am happy to accept the PR with tests

[arma-gast]

Created PR with failing test. Will try to resolve it later.

Update: resolved.

[arma-gast]

sync works obvious: it removes all references than creates new ones. Thats why it does not pass references to detach.

And syncing with an empty array should be allowed: it still an array, just zero length. Why should I check for it's length, call detach if length is zero and sync if it contains items?

[thetutlage]

Ohh yeah sync is working as expected. (My bad)

With detach it is more of a design question than the implementation question.

It will be over responsibility of detach method to do the right thing inside a black box, whereas right now the expectations are clear. When you pass a parameter it should have values or do not pass anything to remove all the references.

With an empty check on the array, the layer will become too complicated. If there is an array remove selected, if array is empty do nothing and if there is no parameter passed remove everything. Which gives me less confidence on what really is going to happen with the detach method.

It should be the end user responsibility to handle the domain logic and try to be as explicit as possible within the code.

[arma-gast]

Hmmm... Looks like you are talking about empty-array check in attach in my PR.

Here is my thoughts:

1. attach expects array (or map) as first parameter.
2. Empty array is still an array.
3. There is no way to prevent end user from passing an empty array to attach other than make a note in docs.

On the other hand, we can modify attach to correctly process empty array: I say "attach no courses to student", and it does nothing. For me this is expected behaviour.

[thetutlage]

Fixed in 4.0