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

Allow callOptions to be added when a validated method is created #46

Merged
merged 4 commits into from
Apr 15, 2016
Merged

Allow callOptions to be added when a validated method is created #46

merged 4 commits into from
Apr 15, 2016

Conversation

mark-bradshaw
Copy link
Contributor

FYI, I've signed the meteor contributor agreement.

I want to be able to send in an options object to be used in the Meteor.apply call of a validated method. My goal is to use the noRetry option, but I made this change to allow any options to be passed in, so that it can be used for any current or future apply options that exist. I added a test, but since the only option I could test was client side, the test doesn't do anything on the server side.

// up doing validations twice
// XXX needs option to disable, in cases where the client might have incomplete information to
// make a decision
this.callOptions.throwStubExceptions = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think passed in options should override these.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Done.

@mark-bradshaw
Copy link
Contributor Author

@stubailo Anything else you'd like to see?

@stubailo
Copy link
Contributor

Looks pretty good, let me get into work and I'll merge and publish!

@mark-bradshaw
Copy link
Contributor Author

Thanks man.

On Fri, Apr 15, 2016 at 11:01 AM Sashko Stubailo [email protected]
wrote:

Looks pretty good, let me get into work and I'll merge and publish!


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#46 (comment)

throwStubExceptions: true,
};

options.callOptions = _.extend(defaultCallOptions, options.callOptions);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I think this will actually overwrite the defaultCallOptions with options.callOptions. So we probably want:

_.extend({}, defaultCallOptions, options.callOptions);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good catch.

@mark-bradshaw
Copy link
Contributor Author

@stubailo Ok, I've renamed callOptions to applyOptions throughout, and updated the Readme. Back to you.

@stubailo
Copy link
Contributor

Looks perfect! Thanks.

@stubailo stubailo merged commit 2345233 into meteor:master Apr 15, 2016
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

Successfully merging this pull request may close these issues.

2 participants