0

I've spent a good hour and a half trying to figure out what's wrong here, and I'm not having any luck. Would really appreciate some help, thanks in advance!

My problem seems to be that in the "Roles.getUsersInRole(mailingAttributes.role).forEach" loop in the sendMailing() meteor method below, the variable "mailingAttributes", which is passed by the client call, is somehow being overwritten, and so on the second run through the forEach() loop, there's nothing for the .replace()'s to replace

1 - Here's an example of the object that gets sent to the sendMailing() meteor method, and how it appears the first time the forEach() loop runs

{
    role:        "admin", //Which user group to send the email to
    subject:     "Hello [[firstName]]!", //Email subject
    html:        "<h1>Hello [[firstName]] [[lastName]], how do you do?</h1>", //HTML Email
    text:        "Hello [[firstName]] [[lastName]], how do you do?" //Plain Text Email 
}

2 - Now the forEach() passes the user object, and replace those placeholders with the user attributes (first & last name)

3 - Given the user Bob Jones, here's an example of how the same object appears the second time the forEach() loop runs

{
    role:        "admin", //Which user group to send the email to
    subject:     "Hello Bob!", //Email subject
    html:        "<h1>Hello Bob Jones, how do you do?</h1>", //HTML Email
    text:        "Hello Bob Jones, how do you do?" //Plain Text Email 
}

So nothing for the .replaces() to change... however I never change the variable (mailingAttributes) where this object is supposed to be stored... so there should be no difference between example 1, and 2

Here's my actual code:

function sendMailingEmail(mailingAttributes) {
    //Send email
    Email.send({
        from: 'no-reply@test.com',
        to: mailingAttributes.to,
        subject: mailingAttributes.subject,
        html: mailingAttributes.html,
        text: mailingAttributes.text,
    });
}

//Set up rate limiting on the email send function
var sendMail = rateLimit(sendMailingEmail, 75);  // wait at least 75 milliseconds between calls (1 second / 14 emails)

Meteor.methods({
    sendMailing: function(mailingAttributes) {

        //Check incoming attributes
        check(mailingAttributes, {
            role:        String,
            subject:     String,
            html:        String,
            text:        String
        });

         // Confirm the user is logged in
        if (this.userId === null || !Roles.userIsInRole(this.userId, 'admin'))
            throw new Meteor.Error(401, 'You must be logged in and authorized');

        Roles.getUsersInRole(mailingAttributes.role).forEach(function(userObject) {
            //Create a temporary copy of the passed attributes
            var userMailingAttributes = mailingAttributes;

            //Make sure the user is subscribed to mailings
            if (userObject.profile.subscribed === true) {
                //Replace placeholders in text with user profile information
                userMailingAttributes.subject = userMailingAttributes.subject.replace("[[firstName]]", userObject.profile.firstName).replace("[[lastName]]", userObject.profile.lastName);
                userMailingAttributes.text = userMailingAttributes.text.replace("[[firstName]]", userObject.profile.firstName).replace("[[lastName]]", userObject.profile.lastName);
                userMailingAttributes.html = userMailingAttributes.html.replace("[[firstName]]", userObject.profile.firstName).replace("[[lastName]]", userObject.profile.lastName);
                userMailingAttributes.to = userObject.emails[0].address;

                //Call the rate limited function, passing along the temporary copy of passed attributes (edited above)
                sendMail(userMailingAttributes);
            }
        });
    }
});

I'm using the alanning:roles & dandv:rate-limit packages

DanielRH
  • 75
  • 1
  • 9

1 Answers1

1

The issue is that this:

//Create a temporary copy of the passed attributes
var userMailingAttributes = mailingAttributes;

is not creating a temporary copy of the passed attributes - it is creating a new reference pointing at the same location in memory. If you alter the one, you are altering the other.

What you want to do is clone the object and use the cloned object instead of a reference to the same object.

Community
  • 1
  • 1
dave
  • 62,300
  • 5
  • 72
  • 93
  • Used Underscore's _.clone(), it worked like a charm! Thank you very much for pointing me in the right direction! – DanielRH Aug 25 '15 at 23:16
  • A note: using underscore's _.clone saves a reference to nested elements. Although you are not doing it here, If you wish to use this on nested objects you should check out lodash's cloneDeep https://lodash.com/docs#cloneDeep – challett Aug 26 '15 at 00:41