Refactor Person/Friendship relationships to be more intuitive #9747

Merged
merged 1 commit into from Mar 17, 2013

Projects

None yet

3 participants

@MacksMind
Contributor

PR #5210 added a Friendship model to illustrate a bug, but in doing so
created a confusing structure because both belongs_to declarations in
Friendship referred to the same side of the join. The new structure
maintains the integrity of the bug test while changing the follower
relationship to be more useful for other testing.

@MacksMind
Contributor

/cc @Pliny

@steveklabnik
Member

Stalkers? :/

@MacksMind
Contributor

Less creepy now.

@MacksMind
Contributor

Squashed

@rafaelfranca rafaelfranca commented on an outdated diff Mar 17, 2013
...es/associations/has_many_through_associations_test.rb
@@ -901,4 +902,13 @@ def test_has_many_through_with_polymorphic_source
readers(:michael_authorless).update(first_post_id: 1)
assert_equal [posts(:thinking)], person.reload.first_posts
end
+
+ test "check sanity of friend/follower relationship" do
@rafaelfranca
rafaelfranca Mar 17, 2013 Member

Why do we need this test? Seems it is a test to test the test structure 😕

@MacksMind
Contributor

@rafaelfranca That's what it amounts to. I was using Friendship to try to replicate #9263, but I stopped because I couldn't use it to actually join 2 People. The test isn't necessary, but the restructuring of Friendship makes it do what the naming indicates it should do.

@rafaelfranca
Member

I'm 👍 fot the model changes, but I think we don't need the test since it is not testing behaviour, but the model structure.

@MacksMind MacksMind Refactor Person/Friendship relationships to be more intuitive
PR #5210 added a Friendship model to illustrate a bug, but in doing so
created a confusing structure because both belongs_to declarations in
Friendship referred to the same side of the join. The new structure
maintains the integrity of the bug test while changing the follower
relationship to be more useful for other testing.
1d6eabb
@MacksMind
Contributor

@rafaelfranca Removed the test and squashed.

@rafaelfranca rafaelfranca merged commit f5ecb13 into rails:master Mar 17, 2013
@rafaelfranca
Member

Thank you ❤️ 💚 💙 💛 💜

@MacksMind MacksMind deleted the MacksMind:refactor_friend_follower branch Mar 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment