Closed (fixed)
Project:
Privatemsg
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
19 Apr 2012 at 10:47 UTC
Updated:
23 Aug 2013 at 12:29 UTC
Jump to comment: Most recent file
Comments
Comment #1
jrao commentedInitial port, will need the following patch to work:
#1529394: Pass in recipient type name to load and autocomplete callback
Unit test relies on:
#1539274: Allow reset of static types in privatemsg_recipient_get_types
Comment #2
berdirThanks for the patch, review is below.
You ported the existing roles tests to privatemsg_group, right? It wouldn't be necessary to have all of them, because it *should* not matter for the recipient type implementing module if the recipients are added through the batch or cron, the roles tests are at the same time also tests for the privatemsg recipient type API. However, doesn't hurt to have them.
Its not necessary to list the .module file here. That is always included anyway.
git diff doesn't like not having an empty line at the end of a file, so let's add one.
Should be hook_permissions().
Trailing spaces.
More trailing spaces here. Also many more below. Try looking at the git diff in console that supports colors, that should highlight them. You could also configure your IDE/editor to remove them, most support that.
$type is the same to all $gids so it should be possible to load them all together using entity_load() and then loop over the loaded entities to add the recipient properties. That should be faster as it only executes a single query.
Two semicolons.
Is it not possible to query for this directly? I assume that there could be a large amount of groups on a site...
Ah, I see, because we depend on the label. Hm, that is unfortunate...
Can't we use og_get_all_group_bundle() here and then build $type? Messing with privatemsg_recipient_get_types() here is kinda weird.
A one line docblock for these would be nice.
Tabs here, also a few below in setUp().
Should be formatted according to the coding standard: // Add user as group member.
No need for that reminder comment here. Especially because it has been commited now :)
A short docblock with a single sentence describing what's tested would be great. Also for those below. I take these don't existing privatemsg_roles.test either...
Comment #3
jrao commentedOk, all fixed now.
Comment #4
Sylense commentedDoes this patch only work with OG 7.x-2.x? I tried it with OG 7.x-1.4 and the private messages screen doesn't render properly
Comment #5
jrao commentedYes. OG 7.x-1.x and 7.x-2.x has an architecture difference, see #1342632: Deprecate OG group entity
Comment #6
nicodv commentedHello, first, thanks for the module, i was so needing it.
I'm not a coder, but when I install it, i get this error:
PHP Fatal error: Call to undefined function og_get_all_group_bundle() in /Applications/MAMP/htdocs/tester/sites/all/modules/contrib/privatemsg_groups/privatemsg_groups.module on line 63
Any idea what to do?
thanks
nico
Comment #7
jrao commentedPlease make sure you're using og 7.x-2.x, this won't work with 7.x-1.x
Comment #8
berdirCan we maybe add a versioned dependency here?
Comment #9
jrao commentedSorry, I'm not sure how to add it, any examples?
Comment #10
berdirHave a look at http://drupal.org/node/542202.
Adding a (2.x) suffix should work, the problem is that it is a bit problematic with git checkout because that has no version (git_deploy.module helps there). Please try it out if it works.
Comment #11
nicodv commentedMy apologies, i read it but forgot it. Now i installed everything in a new test site and i get this errors when i try to type a name in the To: field in the write a message form:
Warning: Missing argument 4 for privatemsg_groups_autocomplete(), called in /Applications/MAMP/htdocs/001/sites/all/modules/contrib/privatemsg/privatemsg.pages.inc on line 702 and defined in privatemsg_groups_autocomplete() (line 189 of /Applications/MAMP/htdocs/001/sites/all/modules/contrib/privatemsg_groups/privatemsg_groups.module).
Notice: Undefined variable: type in privatemsg_groups_autocomplete() (line 190 of /Applications/MAMP/htdocs/001/sites/all/modules/contrib/privatemsg_groups/privatemsg_groups.module).
Notice: Undefined index: controller class in entity_get_controller() (line 7655 of /Applications/MAMP/htdocs/001/includes/common.inc).
BTW, i didn't have to apply any patch, the module is already included in the 7.x-2.x version, right? At least I compared both modules and there were no diff.
Comment #12
Sylense commentedWould it be possible to backport the patch to work with OG 7.x-1.x?
Comment #13
jrao commentedog 7.x-2.x dependency added, if og is checked out from git you'll need git_deploy to give it a version for the dependency to work.
Comment #14
jrao commentednicodv: Please make sure you're using latest dev version of privatemsg, I don't think privatemsg_groups is included in 7.x-2.x yet, it's only available in patch format
Sylense: I'm sure it's possible but I'm not sure there's a point to do this since og is moving away from 7.x-1.x architecture.
Comment #15
Sylense commentedThanks jrao. I actually went ahead and successfully uninstalled and upgraded to 7.x-2.x and it works great.
Comment #16
berdirThanks, commited.
Comment #17
audriusb commentedHi,
I needed version for OG 1.x and backported it myself. Here is the patch.
Comment #19
jayaram commentedDo you have any issues ? Is it working ?
I'm also looking for a similar functionality for OG 7 - 1.x
I used to back ported patch submitted by audriusb but i get an error which says
"Fatal error: __clone method called on non-object in C:\xampp\htdocs\drupal7\sites\all\modules\privatemsg\privatemsg_groups\privatemsg_groups.module on line 188"
Did anyone else get this and know how to fix it ?
Comment #20
jayaram commentedMY bad, its an error in my database! the backport for OG 1.x works!
Comment #21
audriusb commentedI am glad You figured out what was wrong
Comment #22
hanskuiters commentedI used the patch in #13 to create the module. But it doesn't seem to do anything. On creating a new private message the autocomplete shows all users, not just the ones in my group. I tested as group administrator and as group member. I believe I've set all the permissions right (/admin/people/permissions and /admin/config/group/permissions).
I use OG 7.x-2.x-beta2 and Private messages 7.x-2.x-dev
Am I missing something?
Comment #23
hanskuiters commentedWell, I did miss something. I missed that the code from #13 was allready in the Private Messages module. So I deleted and uninstalled my module, cleared the cache and used the default Private Message OG. But still no luck. Again I can see all users, not just the ones in my group.
Comment #24
berdirThat's not what this module does. It provides groups as recipients. Implementing access control based on groups is something different and will need to be implemented in a new issue.
Comment #25
hanskuiters commentedOk, thanks. So I missed something again, sorry for that.
Comment #26
goaldg commentedHi,
I'm using OG 7.x-1.5 and Privatemsg 7.x-1.3 with no problem.
Then I wanted to add Private Msg OG and it didnt work.
As i'm using the 7.x-1x OG version I tried to apply the patch in #17 to Privatemsg 1.x-1.3, then to Privatemsg 1.x-1.x-dev, and then to Privatemsg 1.x-2.x-dev having different error messages (i should have copy..) when activating the module.
Maybe i just dont know how correctly apply that patch which seems to work ... which version of each module should i use ? Privatemsg 1.x-2.x-dev and then replace the /privatemsg_groups/privatemsg_groups.module files by the patch in #17 ? Is that right ?
Thx ..
Comment #27
sreher commentedHi,
this patch works great. Thanks, especially audriusb for backporting to 1.x.
At the moment the admin can write to all group members and they could answer him.
The answers are not displayed to the other participants.
Is there a possibility to show all messages in a group thread to all participants?