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.

Sign up for GitHub

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

Jump to bottom

(refactor): remove meteor-collection-hooks dependency for orders #3639

Merged
merged 4 commits into from
Feb 6, 2018
Merged

(refactor): remove meteor-collection-hooks dependency for orders #3639

merged 4 commits into from
Feb 6, 2018

Conversation

willopez
Copy link
Member

@willopez willopez commented Feb 2, 2018

This PR removes the meteor-collection-hooks package dependency for new orders. Hooks.Events is now used to achieve the same functionality

Testing Procedure

  • Place an order by completing the checkout process
  • Verify that a new AnalyticsEvent document with a field named value is set to the new order's _id

@willopez willopez requested a review from aldeed February 2, 2018 05:59
aldeed
aldeed approved these changes Feb 2, 2018
View reviewed changes
Copy link
Contributor

@aldeed aldeed left a comment

Choose a reason for hiding this comment

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

Verified that analytics document gets inserted. Code looks good.

aldeed
aldeed requested changes Feb 2, 2018
View reviewed changes
Copy link
Contributor

@aldeed aldeed left a comment

Choose a reason for hiding this comment

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

@willopez I approved but then noticed the test failures. There are places in the tests where the orders hooks were stubbed, so you need to remove that code. Search for "Orders._hookAspects"

@willopez
Copy link
Member Author

willopez commented Feb 2, 2018

@aldeed tests are now fixed.

@spencern spencern changed the base branch from master to release-1.8.0 February 6, 2018 18:54
@spencern
Merge remote-tracking branch 'origin/release-1.8.0' into refactor-363…
95509fb 
…2-willopez-use-reaction-event-hooks

# Conflicts:
#	server/methods/core/orders.app-test.js
@spencern spencern merged commit 3a24ddb into release-1.8.0 Feb 6, 2018
@spencern spencern deleted the refactor-3632-willopez-use-reaction-event-hooks branch February 6, 2018 20:49
@spencern spencern mentioned this pull request Feb 6, 2018
Merged
@spencern spencern mentioned this pull request Feb 16, 2018
Merged
@spencern spencern mentioned this pull request Mar 9, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aldeed aldeed aldeed approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@willopez @aldeed @spencern