foundersandcoders / open-tourism-platform

An open platform to facilitate the creation of apps to promote local tourism and business in Nazareth
MIT License
17 stars 3 forks source link

populating fields, e.g. location in events document #89

Closed mattlub closed 7 years ago

mattlub commented 7 years ago

I think we need to populate some of the fields, e.g. in events we should send back data about the location and not just the location id.

quite important imo, thinking about nazareth events.

m4v15 commented 7 years ago

I mean, it would just be a case of having to make 2 requests instead of one at them moment tho, right?

m4v15 commented 7 years ago

Also, what do you mean?

If I have an events database with a load of placeId's then run this:

Event.find().populate('placeId').exec().then(console.log).catch(console.log)

It will return all of the events but with the placeId populated by the places table

If I just do this Event.find().then(console.log).catch(console.log)

I'll just get it with placeId as the id

@mattlub

mattlub commented 7 years ago

@m4v15 in the case of loading a list of events, each event would require another request

mattlub commented 7 years ago

It will return all of the events but with the placeId populated by the places table

@m4v15 yes, this is what I think should happen. But i couldn't get it to work using event.find().populate

m4v15 commented 7 years ago

you need the .exec() on the end

m4v15 commented 7 years ago

Also I'm doing the .find on the model, not on the individual document

mattlub commented 7 years ago

Does it work?

m4v15 commented 7 years ago

Yeah, it does for me

m4v15 commented 7 years ago

I mean, obviously you need to make sure you have something in the database with that PlaceId - if you don't it'll just be replaced with null

m4v15 commented 7 years ago
  { _id: 597f0faa1d67c20a641f9d30,
    updatedAt: 2017-07-31T11:08:26.724Z,
    createdAt: 2017-07-31T11:08:26.724Z,
    en: { name: 'Party at the Basilica' },
    placeId: 
     { _id: 597f0faa1d67c20a641f9d2f,
       updatedAt: 2017-07-31T11:08:26.706Z,
       createdAt: 2017-07-31T11:08:26.706Z,
       en: [Object],
       __v: 0,
       accessibilityOptions: [],
       categories: [] },
    __v: 0,
    accessibilityOptions: [],
    categories: [ 'music' ] },
  { _id: 597f44acbe41160ef01863c7,
    updatedAt: 2017-07-31T14:54:36.512Z,
    createdAt: 2017-07-31T14:54:36.512Z,
    en: { name: 'Raving fun times everywhere' },
    placeId: null,
    __v: 0,
    accessibilityOptions: [],
    categories: [ 'music' ] } ]
mattlub commented 7 years ago

@m4v15 cool. then I'm in favour of:

thoughts?

m4v15 commented 7 years ago

So yeah I agree now that place makes more sense than placeId if we're sending back the whole thing

m4v15 commented 7 years ago

and I guess do the same with ownerId in places

m4v15 commented 7 years ago

Just to note, that to remove the fields from the populate, you pass it as the second argument: Event.find().select('-__v -createdAt -updatedAt').populate('placeId', '-__v -createdAt -updatedAt').exec()

Produces:

[{ _id: 597f0faa1d67c20a641f9d30,
    en: { name: 'Party at the Basilica' },
    placeId: 
     { _id: 597f0faa1d67c20a641f9d2f,
       en: [Object],
       accessibilityOptions: [],
       categories: [] },
    accessibilityOptions: [],
    categories: [ 'music' ] },
  { _id: 597f44acbe41160ef01863c7,
    en: { name: 'Raving fun times everywhere' },
    placeId: null,
    accessibilityOptions: [],
    categories: [ 'music' ] } ]
mattlub commented 7 years ago

closing as location field was populated in the above merged PR. Populating of other fields will come up when #96 is addressed