Skip to content

Conversation

@mackenziehicks
Copy link

What's in this PR?

Refactored by adding helpers from api_case.ex to the Organization Membership Controller Test

References

Fixes #419

Progress on: #413

begedin
begedin previously requested changes Nov 3, 2016
Copy link
Contributor

@begedin begedin left a comment

Choose a reason for hiding this comment

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

Just a minor change I suggest. Otherwise looks good.


test "filters resources by membership id", %{conn: conn} do
[membership_1, membership_2] = insert_pair(:organization_membership)
insert(:organization_membership)
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly to a comment in my other review, while this is good, it could be rewritten in one line as

[membership_1, membership_2 | _] = insert_list(3, :organization_membership)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 94.83% when pulling 470a03a on 419-refactor-organization-membership-controller-test into f9eef9d on develop.

@joshsmith
Copy link
Contributor

Just rebase and squash!

@joshsmith joshsmith dismissed begedin’s stale review November 3, 2016 19:51

Updated by my review.

@mackenziehicks mackenziehicks force-pushed the 419-refactor-organization-membership-controller-test branch from 470a03a to 6a6a356 Compare November 3, 2016 19:55
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 94.83% when pulling 6a6a356 on 419-refactor-organization-membership-controller-test into 46cbaaf on develop.

@mackenziehicks mackenziehicks merged commit f438e9b into develop Nov 3, 2016
@mackenziehicks mackenziehicks deleted the 419-refactor-organization-membership-controller-test branch November 3, 2016 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants