Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Comment | File | Size | Author |
---|---|---|---|
#85 | common_trusted_pmsg_error.png | 49.95 KB | chenhaw |
#73 | Screenshot_6_2_13_10_59_AM.png | 35.13 KB | ezra-g |
#65 | trusted_contact-anonymous_link.png | 24.14 KB | Zarabadoo |
#63 | quicktabs.png | 55.23 KB | itamar |
#54 | 1975198-commons_bw-trusted_contacts-54.patch | 395 bytes | itamar |
Comments
Comment #1
ezra-g CreditAttribution: ezra-g commentedAlso, an email notification should be sent when a user receives a private message or invitation, and when an invitation is confirmed.
Comment #2
amitaibuNice feature! Some technical breakdown:
Comment #3
the604 CreditAttribution: the604 commentedreally like this feature, great work!
Comment #4
ezra-g CreditAttribution: ezra-g commentedGreat to see folks are excited for this feature :).
I'm not sure I understand this part (though I certainly trust your expertise here). Could you elaborate on the purpose of this field?
Also, one thing to be aware of: There may be a few places in Commons Groups and Commons Radioactivity that assume that groups are nodes. This may not affect the feature here but I'm mentioning it just in case.
Comment #5
amitaibuSince User == Group we need a new group-audience field to be able to reference it (as each group-audience field == entity reference field -- can reference only a single group type).
So 1st group-audience, is the "default" will reference nodes.
2nd will reference users.
Indeed, I saw this. We're patching it as we stumble upon it (e.g. grant role on first contribution)
Comment #6
victoria_b CreditAttribution: victoria_b commentedEzra-g,
Fantastic! I was playing around with the Private Message module trying to get something like this to work. This is exactly what I need!
Just one question at the moment...
1) Will we somehow be able to over-ride the "Trusted Contact" text everywhere (string overrides module - maybe) ? I would prefer it to say "Friend"
Thank you!
Comment #7
ezra-g CreditAttribution: ezra-g commentedMy assumption is that this will be possible, at least through a code-based alter/override, though configuration would be a nice option. I think that making the text configurable would be a great separate followup issue.
Comment #8
victoria_b CreditAttribution: victoria_b commentedEzra-g,
Excellent and thank you for the quick reply.
Keep up the great work as this addition is really making me excited for using Commons V3!
Comment #9
NofarG CreditAttribution: NofarG commentedWork in progress.
Next is working on providing access API for private-message to hook in.
Comment #10
NofarG CreditAttribution: NofarG commentedAssigning to myself.
Comment #11
NofarG CreditAttribution: NofarG commentedWork in progress.
Next is working on Invitations view & Trusted Contacts view.
Comment #12
victoria_b CreditAttribution: victoria_b commentedNofarG,
I'm following this development and it looks great.
Re: The "Message" link. Could we eventually have this as a button in the same style as "Follow" ?
Thank you!
Comment #13
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #14
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #15
amitaibuNote that the patch requires OG --dev version.
Comment #16
amitaibuPHPdocs
Tabs.
Remove.
Explain each "return early"
Simplify code
Comment that we get a non-saved OG-membership.
Add comment (i.e. the purpose of this token).
, and not re-saved.
PHPdocs.
his => thier
message type should be
Approve . So the argument should be only the url().
Remove
, in commons_groups_...
Add type hint
his -> thier.
un => untrust
make sure to
git checkout .
after drush make.missing t().
Comment #17
NofarG CreditAttribution: NofarG commentedPrivate-Msg module comes with a "view" (not using Views) - list of messages, but it seems too different from the design above. If we want it to be exactly like the design, it might be easier to create our own View's view than modify the out-of-the-box one. What do you think?
Comment #18
NofarG CreditAttribution: NofarG commentedWork in progress.
Note that the patch requires:
Next:
Comment #19
NofarG CreditAttribution: NofarG commentedWork in progress.
Next:
Comment #20
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #21
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #22
amitaibuuse @ t('Trusted Contacts (@count)', array('@count' => ...
module_load_include()
Page callback; Trusted contacts related pages
return views_embed_view()
No need.
Comment #23
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #24
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #25
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #26
NofarG CreditAttribution: NofarG commentedAlso notice that this patch is updated with the latest changes on commons --dev version.
Comment #27
itamar CreditAttribution: itamar commentedThe field instances of og_user_group_ref are missing. I'm attaching a patch to the patch, for adding the instances.
Comment #28
amitaibuLet's not patch the patch :), just push to the working repo
Comment #29
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #30
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #31
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #32
NofarG CreditAttribution: NofarG commented[ Needs Review ]
Comment #33
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #34
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #35
NofarG CreditAttribution: NofarG commentedWork in progress.
Comment #36
NofarG CreditAttribution: NofarG commentedPlease ignore the previous patch, it's the wrong one.
Here's the right patch.
Comment #37
NofarG CreditAttribution: NofarG commentedI'm including here all 4 patches on their latest versions.
Comment #38
amitaibuPlease move numbers inside t().
Patch already committed, you can remove this.
Comment #39
NofarG CreditAttribution: NofarG commentedHere are the 4 patches on their latest versions.
Comment #40
ezra-g CreditAttribution: ezra-g commentedThanks for this work, NofarG!
It looks like we're re-introducing part of the commons_group_privacy module in this patch. This seems like an accidental inclusion but I wanted to check.
More functional review coming soon but I wanted to note the above first given our differing timezones :).
Comment #41
NofarG CreditAttribution: NofarG commentedgit repositories:
commons (main)
https://github.com/NofarGizra/commons/
commons_groups
https://github.com/NofarGizra/commons_groups/
commons_notify
https://github.com/NofarGizra/commons_notify/
commons_activity_streams
https://github.com/NofarGizra/commons_activity_streams/
Comment #42
ezra-g CreditAttribution: ezra-g commentedNote to folks who may be reviewing that you also need the commons_user_profile_pages patch from #9.
A) Thanks for the repository URLs. I'm not sure this answers my question in #40 but I assume that the info file is an accidental addition. I'll plan to not include it in the commit for this issue unless you direct otherwise.
B) With the present group of patches, the components of trusted contacts and private messaging features are distributed across commons_groups and commons_notify but really, they are distinct pieces of functionality. Is there a particular benefit to not submitting them as individual feature modules?
C) I'd love to see some light code commenting inside our hook_field_formatter_view() implementation.
D) Without og_access enabled, we should make sure that we don't present UI that suggests that content will be private to trusted contacts. I wasn't able to get far enough into the functional review to see how we handle this (see below).
E) I wan't seeing the "Trusted contact" formatter when viewing an existing user's Panels-powered user profile page. Initial debugging shows that we're returning at
for existing users on the site. There should be an upgrade path to make sure existing site users are groups if necessary. I did a drush cc all, drush features-revert-all for good measure.
F) To work around E, I tried to test by creating a new user, I attempted to add another user at admin/people/create, but after submitting the form I get a " Maximum function nesting level of '100' reached" error. At this point I stopped the functional review.
Here's the backtrace:
Comment #43
amitaibuCommons notify only deals with the notifications for new events. It's not really coupled with the Trusted contacts feature.
No problem, let me know the exact wording you want -- it would be better than my own English ;)
Indeed, the patch for the user profile panels was missing, I'll upload it.
In fact we have setup the above github repos, which make it easier to code review, but it can also create the diffs
Nice UI: https://github.com/NofarGizra/commons_groups/compare/7.x-3.x...trusted_c...
Add
.diff
to end of the URL and you'll see a proper diff https://github.com/NofarGizra/commons_groups/compare/7.x-3.x...trusted_c...Fixed. Also make sure to clear Drush cache, so you indeed download the latest --dev of OG. (+We need automatic testing so much!)
Comment #43.0
amitaibuClarification.
Comment #44
amitaibuI have updated the issue summary with all the repos, poiniting to their compare. Append a
.diff
to the URL and you get the patch file.Comment #45
ezra-g CreditAttribution: ezra-g commentedFunctional issues:
- 42 E is still not addressed. To clarify: even with the trusted contact/user group field added to display on the user profile Panels page, this field never displays for users that existed on upgrading sites. It looks like this is because the group_group field is empty for pre-existing users on upgrading sites.
I was able to create a new user account for testing, however when viewing that user's profile page, I click the "Trusted contact" toggle, the wheel spins and I get a popup with the error:
At this point I stopped functional testing.
Architectural issues:
I'm not sure that this addresses my concern about separation of features. Plenty of sites may have no use for the private contacts feature at all, especially if they're not using OG access. The same is true of Private Messaging functionality. Is there a specific reason we don't have these new features separated in new feature modules like the rest of the Commons features?
Similarly:
Setting configuration (variables + permissions) in the profile for one specific feature seems like relatively tight coupling. Like all such exportable components, these should be Featurized.
Yep, I'll provide once I'm farther into the functional testing.
Comment #46
ezra-g CreditAttribution: ezra-g commentedComment #47
amitaibuOk, we can move the code from commons_groups into a new commons_trusted_contacts.
I still think we need to keep the on-screen notifications separated, so they can be used also by other modules.
Indeed, we should probably create the group-group field on
.install
anddb_update()
all existing fields (i.e. make all users become groups)Comment #48
amitaibubtw, @ezra-g, until we re-roll it you can do a functional teting on a fresh install.
Comment #49
NofarG CreditAttribution: NofarG commentedUpdated Repositories:
commons
https://github.com/NofarGizra/commons/
commons_trusted_contacts
https://github.com/NofarGizra/commons_trusted_contacts/
commons_notify
https://github.com/NofarGizra/commons_notify
commons_user_profile_pages
https://github.com/NofarGizra/commons_user_profile_pages
commons_groups
https://github.com/NofarGizra/commons_groups
Comment #50
amitaibu@ezra-g, let's talk about the upgrade path -- I think we'll need to provide a (scalable) solution outside of the installation process, to turn all existing users to groups.
Comment #51
ezra-g CreditAttribution: ezra-g commentedI'm running into some issues with my local environment that are preventing me from moving forward with testing on a fresh install:
I believe these are specific to my environment and am troubleshooting.
Comment #52
ezra-g CreditAttribution: ezra-g commentedI got this setup and went through functional testing. I'm impressed with how well this works and the smoothness of the integration with Private Message module.
I ran into the following issues (listed in priority order) and fixed some:
A) Fields didn't revert/weren't properly defined on a fresh install - Fixed.
B) No "no new messages" link per the designs - Fixed.
C) Upgrade path for existing sites.
D) In some cases I get a UI where the "Operations" field set is empty.
E) Can we move #1987136: Browsing widget without group context - radio buttons for posting to own-group or a specific group into the Commons Trusted Contacts module, since that UI only makes sense when the module is enabled?
F) The Invitations/Messages/Trusted Contacts tabs are implemented as local tasks, but the intention is for them to be similar to the tabs for users' Notifications preferences, which are implemented with QuickTabs. The main reason for refactoring here is for consistency - The alternative would be to alter the theming of this specific set of local tasks but note in other places.
G) Can we send an email when invitations are sent and accepted? This seems acceptable as a followup issue.
Comment #53
ezra-g CreditAttribution: ezra-g commentedAlso, I discussed upgrade path with amitaibu earlier this week and we landed on "prompt to visit menu callback with batch process" as the general approach.
Comment #54
itamar CreditAttribution: itamar commentedIssue 52E is resolved on the commons_trusted_contacts github repository, but requires this additional patch.
Comment #55
amitaibuItamar, can you please push the fix to that repo - let's avoid patches on patches
Comment #56
itamar CreditAttribution: itamar commentedPoints C and F are resolved as well. I didn't find the cause for D yet.
@amitaibu, the patch above is on commons_bw dev, do we need a new repository for that?
Comment #57
ezra-g CreditAttribution: ezra-g commentedIf it's a one line patch to Commons BW from my perspective I'm happy to just commit that without the need for a repo.
Comment #58
ezra-g CreditAttribution: ezra-g commentedThanks, however I don't see any updates to the tabs as specified in F. Perhaps a commit needs to be pushed?
Comment #59
Zarabadoo CreditAttribution: Zarabadoo commentedTheming has begun (starting with the header notification) at http://drupalcode.org/project/commons_origins.git/shortlog/refs/heads/is.... If it makes things simpler for your work, feel free to pull this in.
Comment #60
amitaibu> @amitaibu, the patch above is on commons_bw dev, do we need a new repository for that?
No, sorry I thought it was for the commons_trusted_contact repo.
Comment #61
djvito CreditAttribution: djvito commentedHas anyone had an issue with the ajax functions (delete, mark as read/unread) on the user/%/contacts/messages page? When I try any of the actions (admin account or reg user) I get this error:
If I'm on the /messages page, the actions work as they're supposed to.
Any ideas?
Comment #62
itamar CreditAttribution: itamar commentedSorry, I pushed it to the wrong repository. Now it's there.
Comment #63
itamar CreditAttribution: itamar commentedI fixed the URLs that were leading to the previous tabs to lead to the new quick-tabs.
Comment #64
itamar CreditAttribution: itamar commentedI didn't find the reason for why the "create private message" page is not shown as an overlay, even though I was able to display it as overlay using the overlay_paths module.
Comment #65
Zarabadoo CreditAttribution: Zarabadoo commentedIn the course fo theming, I noticed the "Trusted contact" link on profiles is showing for anonymous users.
Comment #66
itamar CreditAttribution: itamar commentedI fixed #65: Users without subscribe access and anonymous users were seeing the "Trusted Contact" link:
https://github.com/NofarGizra/commons_trusted_contacts/commit/2070831ef4...
Comment #67
giorgio79 CreditAttribution: giorgio79 commented#63 How about adding "Invitations" or "Messages" as a "subject" column between "contact" and "received" to avoid too much of a clickety click experience?
Comment #68
ezra-g CreditAttribution: ezra-g commentedThis is ready for re-review.
Comment #69
Zarabadoo CreditAttribution: Zarabadoo commentedIt think I have touched most pages in this branch. One thing that I cannot really touch is the output of the views bulk operations form. At least, I am not sure what to do with it. It appears when there are no options available an the button text and options are a bit confusing.
Comment #70
ezra-g CreditAttribution: ezra-g commentedI fixed 52D (n some cases I get a UI where the "Operations" field set is empty.) with https://github.com/NofarGizra/commons_trusted_contacts/commit/e072b0666c....
Zarabadoo is working to wrap up the themeing on this feature, and I'll work on adding any missing hook_update_N() implementations, such as for Commons User Profile pages and Commons Utility links.
I plan to file followups for:
* Followup: Add "direct message" button on user profiles
* Followup: Performing "Break contact" batch API text is awkward, then "Performed Break Contact on 1 item." let's just do it without batch.
* Followup: Change to "Add as trusted contact" or "Add to trusted contacts"
* Followup: Consider moving invitations field and notifications code from commons notify into commons trusted contacts.
Comment #71
ezra-g CreditAttribution: ezra-g commentedI've made several commits to the GitHub repos to fix minor issues and make sure the Features are revertable to default status, with descriptive commit messages to explain the motivations for the commits.
However, the following blocking functional issues remain:
A) "All" tab does not contain the "Posts" short form.
B) I receive these errors when posting into a group from the short form:
It looks like commons_trusted_contacts_node_presave() is expecting a $node->form_state property.
C) Form validation currently prevents users from submitting the short form on the site homepage when no groups are specified. However, the intent with this design was to allow folks to submit content that is site-wide/not tied to a specific group.
D) We shouldn't show the "Private/Specific groups" form elements when OG Access module is disabled, since it would likely give the impression to users that their resulting content will be private when that is not the case.
Comment #72
itamar CreditAttribution: itamar commented@ezra-g
Issues A, B & C (On comment #71) are resolved once the patch from comment #54 is applied and the github repositories are updated with the latest 7.x-3.x changes.
About D: I'm not sure where we're showing this. Can you please add a screenshot showing what should be removed?
Comment #73
ezra-g CreditAttribution: ezra-g commentedThanks, itamar - I'll try with the patch attached.
Re C, it appears that this validation is unrelated to the patch, but I'll double check:
Here's the form elements I'm referring to in D:
Comment #74
itamar CreditAttribution: itamar commentedThanks ezra.
I pushed to commons_trusted_contacts on github a fix for hiding the audience type selection when og_access is disabled.
Now the validation you quoted is taking place only when og_access is on and you select "Post to specific groups". I hope this is what you meant.
Comment #75
ezra-g CreditAttribution: ezra-g commentedThanks, itamar. I don't see new commits at https://github.com/NofarGizra/commons_trusted_contacts/commits/master - Perhaps you forgot to push?
Comment #76
itamar CreditAttribution: itamar commentedI accidentally pushed the previous fix to the wrong commons_trusted_contacts repository. Now it's in the correct one.
Comment #77
ezra-g CreditAttribution: ezra-g commentedGreat - Thanks, itamar.
Did you get a chance to look at 71 A?
Comment #78
itamar CreditAttribution: itamar commented@ezra-g - Yes, I'm seeing the posts form on the All tab both on a fresh install with all 1975198 applied and also after applying them on a clean 7x installation.
I verified that the problem was that some of the github repositories were not updated to the current 7x status.
Comment #79
Zarabadoo CreditAttribution: Zarabadoo commentedSnagging this to work on a couple small issues.
Comment #80
ezra-g CreditAttribution: ezra-g commentedOne thing I just noticed - The "Approved trusted contact request" mail has an unreplaced token:
Comment #81
Zarabadoo CreditAttribution: Zarabadoo commentedThere is a small javascript bug on the partial forms on the front page.
My guess (as i have not had a chance to fully troubleshoot) is that Drupal's dependency javascript was not made to work with multiple forms on a page. One form's script is seeing the other forms and vice versa, and they are all fighting with each other. The second form is actually acting on the previous form and so you are not seeing the proper actions.
Comment #82
ezra-g CreditAttribution: ezra-g commentedMarking as "needs work" per 80 and 81.
Comment #83
ezra-g CreditAttribution: ezra-g commentedZarabadoo and I resolved 80 and 81 with: https://github.com/NofarGizra/commons_trusted_contacts/commit/60eb38d6b7... and https://github.com/NofarGizra/commons_trusted_contacts/commit/fe939400d9....
I believe this is RTBC.
In the short term, I'll create a new Drupal.org project to be consistent with the other Commons components. In the long-term, we should likely move this into the main Commons repository with #2009294: Consider moving invitations field and notifications code from commons notify into commons trusted contacts..
Comment #84
ezra-g CreditAttribution: ezra-g commentedThis is committed.
I filed these followups:
#2009288: "Trusted contact" action is unclear. "Add as trusted contact" or "Add to Trusted contacts" would be clearer
#2009290: Performing "Break contact" batch API text is awkward
And created the project: https://drupal.org/project/commons_trusted_contacts.
And below are the commits.
Thanks, everone for your work on this issue!
http://drupalcode.org/project/commons_notify.git/commit/c1e364d
http://drupalcode.org/project/commons_groups.git/commit/e6d5572
http://drupalcode.org/project/commons_user_profile_pages.git/commit/16e6c25
http://drupalcode.org/project/commons.git/commit/8e43e44
http://drupalcode.org/project/commons_origins.git/commit/4dc621425f016f2...
Comment #85
chenhaw CreditAttribution: chenhaw commentedHello everyone,
I am wondering what is the minimum Drupal Common version required for https://drupal.org/project/commons_trusted_contacts to works? I downloaded the modules and when trying to activate it in Commons-7.x-3.2, it gave me error as below.
Comment #86
japerryThis was missed the first time -- I've committed it here:
http://drupalcode.org/project/commons_bw.git/commit/89ec4a3
Comment #88
BetaTheta CreditAttribution: BetaTheta commentedQuick question: will there be an option to disable 'Trusted contacts'? I'm trying to develop a site for my organization and I'd like any member to email any other member.
Comment #89
ezra-g CreditAttribution: ezra-g commented@BetaTheta: Please open a new issue for new questions and support requests. Thanks!
Comment #89.0
ezra-g CreditAttribution: ezra-g commentedUpdated issue summary.
Comment #90
deeray CreditAttribution: deeray commentedI really like this feature, How can I use this as a module on my live site?
Comment #91
deeray CreditAttribution: deeray commentedI really like this feature, How can I use this as a module on my live site?
Or is there a similar module I can use in achieving this?
Thanks
Comment #92
bjp232004 CreditAttribution: bjp232004 commentedFacing issue with Bulk approval for Trusted Contacts. I have installed latest version of Drupal commons and send request for Trusted contacts. When I am trying to select checkbox and bulk approve it's not working as expected.
Comment #93
bdupls CreditAttribution: bdupls commentedAs administrator under private messaging I have no pending invitations, but if I look at a particular user that previously did request membership from me I see awaiting approval. Can anyone tell me the name of the field in the database where I can manually un-check this awaiting approval from this member and start over.
Previously when trying to approve the user I got an bulk update AJAX error and have been unable to solve it, I am wondering if its the same error as bjp232004.
ajax error StatusText: error ResponseText: ReadyState: 0