Add user's personal groups (acquaintances, family, etс.) support

[nikolaynesov]

User can group/ungroup his/her friends with new methods groupFriend & ungroupFriend.
Personal groups support added to getAllFriendships, getAcceptedFriendships, getFriendsCount, getFriends methods.

[hootlex]

Wow, great feature!
Good job @nikolaynesov

Just a few notes:

1. I suggest naming class CreateFriendshipsGroupsTable instead of CreateFriendFriendshipGroupsTable
2. Make friend groups optional, though the config, and publish migration accordingly
3. Provide some tests?

Thanks!

[nikolaynesov]

@hootlex I think maybe later config array with groups can grow up to a separate table and its migration class can be named like CreateFriendshipsGroupsTable (for the list of groups)... will it still be ok to name the pivot table class CreateFriendshipsGroupsTable (for the list of relationships between friendships and groups) or maybe we should find another name?

[hootlex]

@nikolaynesov Sorry for my late response. The class which will contain the Group names can be called CreateFriendshipsGroupNamesTable. What do you think about that?

[nikolaynesov]

Hi @hootlex,

1. I renamed CreateFriendFriendshipGroupsTable to CreateFriendshipsGroupsTable . We will be able to rename it later anyway in case we have any conflicts in the future and it won't affect table names (moved to config).
2. Made it optional from config but it does not make a lot of sense as the config has to be modified in the package folder not to publish the migration at all. Does not look like a good practice. Maybe we can publish the migration by default but do not migrate this table if there are no groups in the config. Looks better to me. What do you think?
3. Provided some tests

[hootlex]

Hi @nikolaynesov

1. Great!
2. That's a good point. I have an idea. What about adding 2 columns, for the group, within the current friendships table?
   For example, a column 'sender_group_id' and recipient_group_id which will represent the id found in the config file.

[nikolaynesov]

ok, but what if user wants to add one (same) friend to different (2+) groups? What if you do not have any groups at all so why do we need to keep empty columns?

I started with the idea of additional columns but moved to current solution as it is more flexible.

[hootlex]

Okay then, let's do it with a second table! 👍

[nikolaynesov]

so should we just publish the migration for the second table by default?

[hootlex]

Yup!

[hootlex]

Good job! Though, I've found a bug with removing a group from a friendship.
Check #36.

[stephane-monnot]

@hootlex Should we refactor the documentation ?

[nikolaynesov]

@stephane-monnot ,

I have updated the README slightly. Do you want to add more details?

[stephane-monnot]

@nikolaynesov ,
The content of the readme is ok, but maybe there are so many "how to use" examples without structure, just a huge list. Should we make differents sections ? @hootlex, Maybe wait the refactor PR (for refactoring documentation) ?

[nikolaynesov]

@hootlex added tests

[hootlex]

@stephane-monnot Yeah, definitely. I was also thinking that readme has to be improved/updated.

@nikolaynesov Good job, thanks!

[hootlex]

@stephane-monnot maybe it's time to remove the word Eloquent and mention only friendships between users.

[stephane-monnot]

@hootlex Good idea !

[stephane-monnot]

@hootlex What do you think about adding a tag 1.1.0 ?

[hootlex]

@stephane-monnot I think it's a good idea. After merging #39 it will be solid to have v1.1 and go to v2.0 with #16.

[stephane-monnot]

Maybe we can add deprecated note in 1.1 if we want to change friends() method in 2.0. Because I want to use friends() (return User query builder) and friendRequests() (return FriendShips query builder).

Actually, getMutualFriends in my PR return a collection, but I think that it's better to always return querybuilder, because people may want to filter, order and after ->get().

[hootlex]

I agree with the deprecation note.
I like $user->friendRequests()->get() and $user->friends()->get(). 👍

getMutualFriends should return the actual friends like getFriends and the other methods. For the query builder it should be something like $user->friends()->mutual($user2) and $user->friends()->mutual($user2)->get() to get the friends.

[hootlex]

@stephane-monnot In the v2 we should refer only this way in the docs $user->friends()->mutual($user2)->get(), $user->friendRequests()->get(), etc and create a section 'Lazy Getter' where all the getSomethingFriends and Friendships will be mentioned.

[stephane-monnot]

I will try to finish my PR with incoming() and outgoing soon.

So we should :

• Add deprecated notes/docs/errors on master v1.1 (like this )
• Create a develop branch
• Merge my PR with develop
• Improve documentations/tests....

[hootlex]

Great!
I created a 'dev' branch, push there when you are ready.

Feel free to add deprecation notices to the methods you are removing. Since we are renaming the most deprecated methods, instead of 'You should implement this method yourself...' you can inform the user which will be the new method name for this feature, where possible.

[stephane-monnot]

Thank you !

The friends() method doesn't have an equivalent method in 2.0, because I plan to make private or protected the morphMany (See this commit ).

But you are right, because we can use $user->requests()->outgoing()->get(), right ? Though this is a little different.

[hootlex]

@nikolaynesov what's the purpose of tests/config directory?
From what I see in Travis file only to friendships.php is being used.

[nikolaynesov]

@hootlex yes, we use only this file.

[hootlex]

@nikolaynesov so, the rest of them can be safely deleted, right?

[nikolaynesov]

@hootlex sure. Those that are not used in Travis file can be removed.