For both functional and scalability reasons, let's impose a reasonable limit to the number of trusted contacts that a user can have. The functional justification here is that it's not possible to trust a large number of people. Facebook limits friends at 5,000. Groups in Commons are unlimited in membership and can be private. Users can still follow an unlimited # of users so if we imposed a limit of 1,000 contacts per-person, I think we'd avoid situations with extreme #'s of contacts (eg, users with 7,000 trusted contacts).
It seems reasonable to me for this limit to be either hard-coded at 1,000 or configurable.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 2173637-commons-trusted-contacts-limit-6.patch | 7.17 KB | japerry |
| #5 | 2173637-commons-trusted-contacts-limit-5.patch | 2.91 KB | ezra-g |
| #4 | 2173637-commons-trusted-contacts-limit-4.patch | 2.15 KB | ezra-g |
Comments
Comment #1
ezra-g commentedLet's set the goal of shipping this feature in February.
Comment #2
ezra-g commentedComment #3
ezra-g commentedSome suggestions from lea.refice:
- We should clearly display for users where they are relative to their trusted contact limit. I think we could consider making this display optional and only appear when the user is 75% of the way there.
- Rather than changing the appearance of the "Add as trusted contact" buttons, we could instead intercept the request and display a message letting the user know that she has reached her trusted contact limit (or that the other user has reached her own) and that the user will have to remove some contacts in order to add more.
Comment #4
ezra-g commentedI made less progress on this than I'd hoped since the last update. Here's a super in-progress stub patch. Marking as "needs work"
Comment #5
ezra-g commentedI got a bit more time to work on this this morning. Another progress patch.
Comment #6
japerryHere is an updated version. It adds a check to make it optional (setting is on the people settings page), and it works when approving or adding a user. Ignored users do not count against the number of trusted contacts.
Comment #7
ezra-g commented#6 is committed. Thanks! I filed #2247443: Show user's trusted contact limit when the user reaches ~80% of the limit as a followup.
http://drupalcode.org/project/commons.git/commitdiff/c1c2b0d?hp=e09291d4...
Comment #10
axlroach commentedWhat's the rational for the drupal_goto() inside the commons_trusted_contacts_check_limit() function?
Essentially, if $ajax is FALSE and the $exceeds_limit variable is TRUE, then it goes to the front page. Is there a particular reason for this behavior? I'd rather just get back FALSE in this scenario.
Comment #11
axlroach commentedWe need to be able to count pending memberships toward the trusted contact limit. I had attached a patch that allows an admin to decide whether or not to include pending memberships when calculating a user's number of trusted contacts.
As I thought through this a bit more though, I'm realizing that the use case we have for this is a fairly unique use case and may be best handled outside the context of the commons_trusted_contacts.module. Essentially, we need to count pending and active trusted contacts towards the limit EXCEPT when accepting an incoming pending request. I've hidden the patch file that I attached earlier.
Comment #12
axlroach commentedComment #13
ezra-g commentedSetting back to "needs review" so that the new patch in #11 isn't lost. In general, it's a best practice to file a new issue for revisions to an existing closed/fixed feature for that reason. Thanks for the patch - we'll take a look!
Comment #14
japerryI think we should be adding pending contacts as part of this limit because its possible you could go over the limit otherwise... which means we probably shouldn't have an explicit configuration option for it.
Is there a reason why you guys would need to have the pending user limit disabled?
Comment #16
japerryI've committed the change to add pending users to the count. If there needs to be an option to disable the pending users, feel free to re-open.