jagi / meteor-astronomy

Model layer for Meteor
https://atmospherejs.com/jagi/astronomy
MIT License
605 stars 66 forks source link

astronomy classes over simple ddp connection #535

Closed tzapu closed 7 years ago

tzapu commented 7 years ago

Hi, Thank you very much for a wonderful package.

I have a collection which i am enhancing with a simple 'relation' so i can get some fields from another collection, using transient fields

import { Mongo } from 'meteor/mongo';
import { Class } from 'meteor/jagi:astronomy';
import { User } from './users';

export const ConversationParticipants = new Mongo.Collection('conversationParticipants');

export const ConversationParticipant = Class.create({
  name: 'ConversationParticipant',
  collection: ConversationParticipants,
  fields: {
    userId: {
      type: String,
      default: '',
    },
    participantName: {
      type: String,
      transient: true,
    },
    participantAvatar: {
      type: String,
      transient: true,
    },
  },
  events: {
    afterInit(e) {
      const doc = e.currentTarget;
      if (doc.userId !== '') {
        const user = User.findOne(doc.userId);
        doc.participantName = user.name;
        doc.participantAvatar = user.avatar;
        console.log('user', user);
      }
    },
  },
});

This all works nicely. All populated, all reactive, etc.

Now my problem, or more likely, lack of knowledge, is that I also have a client connecting through plain DDP, using the asteroid lib. As much as I understand of things, through DDP, only the data from the publication would be sent and astronomy would do the population of the transient fields locally. Because the client is not Meteor, and astronomy is not present on the client, it obviously can't do that.

Hopefully I managed to explain that correctly. Is there any way in which i could pass the transient fields as well? What do you see as the simplest way of doing this? Should I just not try to use the model for this and have a separate method call to get those fields? Or maybe an astronomy method? Would that be a correct use case for it? (all examples i ve seen for meteorMethods on astro are for modifying the object, not fetching some more data) Would helpers be usable from within a meteorMethod call? What would be the actual method call name? (not in docs, i figure from the merged pull that it should be like 'Astronomy/conversationParticipants/methodName' ?

Currently I m thinking that maybe the way to do it(if at all possible) is to define a helper that does the secondary fetch, then have the method return the fields fetched from the helper and have the transient fields also populated by the helper to use within Meteor. Would that be ok?

Or if within afterInit i have access to the method, maybe populate from that directly.

Any insight is much appreciated, i ll try and do my tests as well. Thanks again

tzapu commented 7 years ago

maybe i should actually ask, are meteorMethods callable directly using Meteor.call ? /edit: slowly wiggling my way through this i was able to call the method through asteroid using something like this.asteroid.call('/Astronomy/execute', {className: 'ConversationParticipant', methodName: 'test', id: 'id', methodArgs:[] }) onwards and upwards, and any hints really appreciated :P still a newb...

/edit2 i ve managed to make it work, seems to work quite nicely. i am not really sure if the overhead of an extra query to rehydrate the document is worth it or i d be better off just having a regular meteor method ... although, i might be making a mess of things for all i know

for reference, here s my model

import { Mongo } from 'meteor/mongo';
import { Class } from 'meteor/jagi:astronomy';
import { User } from './users';
import { Client } from './clients';

export const ConversationParticipants = new Mongo.Collection('conversationParticipants');

export const ConversationParticipant = Class.create({
  name: 'ConversationParticipant',
  collection: ConversationParticipants,
  fields: {
    appId: String,
    conversationId: String,
    userId: {
      type: String,
      default: '',
    },
    clientId: {
      type: String,
      default: '',
    },
    participantName: {
      type: String,
      transient: true,
    },
    participantAvatar: {
      type: String,
      transient: true,
    },
    participantType: String,
    isTyping: {
      type: Boolean,
      default: false,
    },
    isActive: {
      type: Boolean,
      default: true,
    },
    unreadCount: {
      type: Number,
      default: 0,
    },
    notified: {
      type: Boolean,
      default: false,
    },
  },
  events: {
    afterInit(e) {
      const doc = e.currentTarget;
      const data = doc.participantData();
      doc.participantName = data.participantName;
      doc.participantAvatar = data.participantAvatar;
    },
  },
  helpers: {
    participantData() {
      let doc = {};
      if (this.userId !== '') {
        const user = User.findOne(this.userId);
        if (user) {
          doc.participantName = user.name;
          doc.participantAvatar = user.avatar;
        }
      }
      if (this.clientId !== '') {
        const client = Client.findOne(this.clientId);
        if (client) {
          doc.participantName = client.name;
          doc.participantAvatar = client.avatar;
        }
      }
      return doc;
    },
  },
  meteorMethods: {
    getParticipantData() {
      return this.participantData()
    },
  },
  behaviors: {
    timestamp: {
      hasCreatedField: true,
      createdFieldName: 'createdAt',
      hasUpdatedField: true,
      updatedFieldName: 'updatedAt',
    },
  },
});

and here s how i call it through asteroid/ddp, although this could be a Meteor.call as well

        this.asteroid.call('/Astronomy/execute',
          { className: 'ConversationParticipant', methodName: 'getParticipantData', id, methodArgs: {} })
          .then(result => {
            console.log('Success', result);
          })
          .catch(error => {
            console.log('Error', error);
          });
tzapu commented 7 years ago

and simpler:

  events: {
    afterInit(e) {
      const doc = e.currentTarget;
      if (doc.userId !== '') {
        const user = User.findOne(doc.userId);
        if (user) {
          doc.participantName = user.name;
          doc.participantAvatar = user.avatar;
        }
      }
      if (doc.clientId !== '') {
        const client = Client.findOne(doc.clientId);
        if (client) {
          doc.participantName = client.name;
          doc.participantAvatar = client.avatar;
        }
      }
    },
  },
  meteorMethods: {
    getParticipantData() {
      return { participantName: this.participantName, participantAvatar: this.participantAvatar };
    },
  },

slept on it and realised data would of been populated on init, no need to fetch it again

lukejagodzinski commented 7 years ago

Meteor methods should be used when you want to perform operation on a single document. It may also be fetching some extra data. However, in your case I would not use them as your problem is a little bit different.

The best way to solve your problem is preparing regular Meteor method. It should fetch given document and all its relation and return such a document in plain form, because as you said your other client is not using Astronomy. So it could look like this:

Meteor.methods({
  getConversationParticipant(cpId) {
    // It will also fetch all related document in the afterInit event.
    const cp = ConversationParticipant.findOne(cpId);
    return cp.raw();
  }
});
tzapu commented 7 years ago

thank @jagi for all your hard work and taking the time to answer this. it's very cool to know you can do it that way as well. my 'dilemma' is that i already get all the data from the participant through publish/subscribe (reactive pub/sub as well) and am just missing the transient fields. i assume, from reading around here that there is no way to publish with everything included so i am left with either:

thanks again for all this

lukejagodzinski commented 7 years ago

Maybe there is a way of doing it through some publication and automatic merging documents coming from different collections. I'm not familiar with all Meteor packages for publishing but I think you should start by going through the list of packages at Atmosphere. You could probably use added, changed and removed methods of publication to manage documents merging http://docs.meteor.com/api/pubsub.html#Subscription-added. But I'm not an expert in this field. Maybe just as on Meteor forums or stackoverflow.

tzapu commented 7 years ago

thank you @jagi , thank you very much

tzapu commented 7 years ago

for posterity, and maybe for other's interest, here s where i ended up using your suggestions above

Meteor.publish('/Conversations/SDK/ConversationParticipants', function () {
  let conversation = Conversation.findOne({ clientId, appId });
  let subHandle = ConversationParticipant.find({ conversationId: conversation._id }).observeChanges({
    added: (id, fields) => {
      let conversationParticipant = new ConversationParticipant({ _id: id, ...fields });
      console.log(conversationParticipant.raw());
      this.added('conversationParticipants', id, conversationParticipant.raw());
    },
    changed: (id, fields) => {
      this.changed('conversationParticipants', id, fields);
    },
    removed: (id) => {
      this.removed('conversationParticipants', id);
    },
  });

  this.ready();

  this.onStop( () => {
    subHandle.stop();
  });
});

would this: new ConversationParticipant({ _id: id, ...fields }); do a new findOne on the _id although it has all fields, or does it just populate the transients, etc?

looks, so far, to me, to be the best of both worlds. i can keep my model in one place, and can serve it to non meteor clients as well.

thanks again for everything

lukejagodzinski commented 7 years ago

would this: new ConversationParticipant({ _id: id, ...fields }); do a new findOne on the _id although it has all fields, or does it just populate the transients, etc?

I don't quite understand question

tzapu commented 7 years ago

sorry, i meant to ask if when i intialise an Astro model like that, giving it all the fields, instead of just an id, does it still internally perform a fetch? or does it do just initialisation of fields (transients, mapping) and uses the passed object instead of re fetching it?

i hope i was able to explain better now, thanks

lukejagodzinski commented 7 years ago

That is what I was thinking but I wanted to make sure that I've understood it correctly :). So it's not refetching document. It will just set all fields with values passed as the first argument.

tzapu commented 7 years ago

that s kind of what i figured reading around here, but wanted to be sure. well, for me at least this is perfect.

thank you very much

lukejagodzinski commented 7 years ago

No problem :)