Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model instance should have toJSON method #21

Closed
ghost opened this issue Feb 22, 2016 · 9 comments
Closed

Model instance should have toJSON method #21

ghost opened this issue Feb 22, 2016 · 9 comments
Assignees

Comments

@ghost
Copy link

ghost commented Feb 22, 2016

When getting a record via Model.find(), the returned object ignores the visible/hidden fields declared in the Model declaration. Also; the record itself is inside the attributes key & calling model.toJSON() doesn't work.

Getting the record via model.where().fetch() works fine. However; I can't access the first object of the returned array.

const record = yield Model.where('id', '=', 1).fetch()
console.log(record)
res.send(record[0])

When logging the record object in console, it looks like an array [{ ... }] but getting the first object of that array record[0] does not work. It does work after calling the model.toJSON() method.

  1. Why do find() & where() behave so differently?
  2. Why isn't it possible to get the first object of the array?
@ghost ghost changed the title Model.find ignores visible/hidden settings Model.find (ignores visible/hidden settings) vs. Model.fetch Feb 22, 2016
@thetutlage
Copy link
Member

Couple of things here.

First .find method returns the model instance not a collection, so that is the reason .toJSON does not work. Ideally model instance also should have .toJSON method.

Second when you call .fetch method you get a collection back, which is nothing but a lodash wrapper. So in order to get the first record, you have to say.

const record = yield Model.where('id', '=', 1).fetch()
console.log(record.first())

Also i am refactoring Lucid, so in coming future all these inconsistency issues will be resolved.

@thetutlage thetutlage self-assigned this Feb 22, 2016
@thetutlage thetutlage added the bug label Feb 22, 2016
@thetutlage thetutlage changed the title Model.find (ignores visible/hidden settings) vs. Model.fetch Model instance should have toJSON method Feb 22, 2016
@gabriel-alecu
Copy link
Contributor

gabriel-alecu commented Jun 22, 2016

Any reason why find() is a oh-so-special method that doesn't return a collection?
Was it just forgotten?
What's the point in using Collection, if some methods use it, but others don't?
It creates inconsistencies, so I end up just removing wrappers from everything just to make things work.

And collection also uses a very old version of lodash, so I have to install a separate version...

Collection seems less like something convenient, and more like a pain in the ass...

@thetutlage
Copy link
Member

Coz Collection means collection of multiple rows not collection of a single row.

When you have multiple rows wrapped inside a collection, you can perform operations like map, filter etc. Why would you perform those operations one a single row?

@gabriel-alecu
Copy link
Contributor

gabriel-alecu commented Jun 22, 2016

where().first() returns just one row (not inside a list), but it's considered a collection (aka has a lodash wrapper, because that's really all that Collection is...)

For consistency it should either return the row inside a list, or return just a Model instance like find, or make find also return a collection.

@thetutlage
Copy link
Member

If you are on adonis-lucid:^2.0 than yes. In 3.0 it will return the model instance.

@thetutlage
Copy link
Member

Anytime when a single row is fetched intentionally, it will always be an instance otherwise a collection.

@gabriel-alecu
Copy link
Contributor

Also, I notice that whenever a Collection is used, the wrapped values are not model instances, thus creating another inconsistency.
Is this also fixed in 3.0?

@thetutlage
Copy link
Member

Yes

@navdeepsingh
Copy link

@thetutlage @marines

const feed =  yield user.feed().fetch()
let index = 0
while( index < _.size(feed.value()) ) {
  // as feed.value() will fetch collection not instance of model
 // i need instance to update data in feed table
  index++
}

All good with Database query update, wanna do with db instance. Please guide.

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants