Please add methods for the Services module.

Files: 
CommentFileSizeAuthor
#64 privatemsg-added_pm_services-433780-64.patch9.26 KBptmkenny
PASSED: [[SimpleTest]]: [MySQL] 4,913 pass(es).
[ View ]
#52 privatemsg-pm_services_added-433780-52.patch0 bytesnathan_czh
PASSED: [[SimpleTest]]: [MySQL] 3,157 pass(es).
[ View ]
#46 pm_service.tar_.gz10 KBnathan_czh
#34 privatemsg_service-DRUPAL-6--1.patch31.59 KBhaagendazs
PASSED: [[SimpleTest]]: [MySQL] 3,023 pass(es).
[ View ]
#28 privatemsg_service-DRUPAL-6--1.patch31.41 KBhaagendazs
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg_service-DRUPAL-6--1.patch.
[ View ]
#22 pm_service-DRUPAL-6--1-3.patch22.85 KBhaagendazs
PASSED: [[SimpleTest]]: [MySQL] 2,699 pass(es).
[ View ]
#20 pm_service-DRUPAL-6--1-3.patch21.31 KBhaagendazs
PASSED: [[SimpleTest]]: [MySQL] 2,145 pass(es).
[ View ]
#18 pm_service.zip5.06 KBhaagendazs
#16 pm_service.zip4.84 KBhaagendazs
#14 pm_service.zip3.11 KBhaagendazs
#8 pm_service.zip7.2 KBtayzlor
#6 pm_service.zip6.89 KBtayzlor

Comments

Status:Active» Postponed (maintainer needs more info)

Exactly what do you mean?

Status:Postponed (maintainer needs more info)» Active

We are in need of service methods like pm.send and pm.get. Basically by utilizing the hooks provided by the services module, it's possible to use functions to send and receive messages for use in flash and others.

Privatemsg does provide several api functions to save and load messages (and partly, threads, see #404960: messages/view/% shows the participants of a conversation even if there are no messages that the user can view.), they are documented at http://blog.worldempire.ch/de/api/group/api/1.

Creating a "brigde" module between Services and Privatemsg should be relatively easy.

Bump. Looking for Services options as well. Just starting to research, but figured I'd see if anyone out there has spent time on this one.

I will be looking into this shortly as it is something we require here also. Will post back any findings / code / patches to this thread.

StatusFileSize
new6.89 KB

I am attaching a privatemsg service module for feedback and review. I probably haven't covered everything in terms of service methods, and there may be some flaws, but the module has the following functionality / methods as a start -

  • pm.get - Get a specific users private messages (can be either the users inbox, or their sent messages)
  • pm.getUnreadCount - get a specific users number of unread messages
  • pm.send - Send or reply to a private message

Any feedback / changes welcome.

Note: It has been developed against Services 6.x-2.2 and Privatemsg 6.x-2.x-dev.
I am unsure of how you would want this, would it be part of the privatemsg module, or a separate module in itself. Let me know if you want it as a patch to privatemsg rather than a new module zip file.

Only had a short look over the current code, some feedback...

- We provide api functions for sending new messages or reply to existing threads, have look at the api link in #3. Separating your current send service into a separate send/reply function and use those api functions should vastly simplify your code. You could also check how privatemsg_rules.module uses these api functions.

- There is also an api function to load the amount of unread messages, see http://blog.worldempire.ch/de/api/function/privatemsg_unread_count/6-2

- Note that the list query returns thread_id and although they are also valid mids, using privatemsg_message_load_multiple() will only give you the first message of every thread. You probably want to use privatemsg_thread_load() on these thread_id's instead, but you might want to make a separate service callback out of that, since that function loads a lot.

- I guess you need some sort of paging to limit the number of messages/threads that are loaded at once.

- I did write a similiar, custom remote interface for privatemsg once and what we found to be useful were api functions to load threads with unread messages and/or threads newer than date. (If you have a remote client, like a phone, you can store the time when fetching new messages and only load messages which were sent since then). You can do that by implementing custom list types (instead of 'inbox' for example) and implement http://blog.worldempire.ch/de/api/function/hook_privatemsg_sql_list_alte... to add the conditions for those types.

- It would be really awesome to also have tests for this, I'm not sure how complex it is to do that. I'm assuming that services.module also provides something for that...

- Edit: Oh, and doing it as a separate module is perfect.

StatusFileSize
new7.2 KB

@Berdir,

great thanks for the super quick and helpful feedback!!

I'm attaching an updated module with the following -

  • Refactored send service method to now use the API functions privatemsg_new_thread() or privatemsg_reply() for sending and replying to messages.
  • Similarly the unread messages service method now uses the correct API function - privatemsg_unread_count().
  • Now using privatemsg_thread_load() to load the full thread of the message if the $load_full parameter passed in to the service is TRUE. Have kept it as 1 service method rather than splitting it since you can pass in false to the method which will not load the whole thread and have also...
  • Implemented paging using db_query_range() to query based on optional $limit and $offset args which can now be passed to the service method. If no args are passed it will load the full amount of messages.

I have not yet implemented your final 2 points (custom conditions + tests) yet but i figure these can come soon. The services module does not currently have any tests itself so I may have to figure that one out myself.

To confirm - Do you want it as a separate module bundled in with the privatemsg module, or would you rather it was a whole separate module with its own drupal.org project page?

Status:Active» Needs review

No need to for a separate project, we can bundle it with privatemsg.module if it is well enough tested.

For that, it would be really important to have some automated tests as well. I'd suggest to look at the existing tests (There are some json-based autocomplete tests for example, and the XML-RPC core tests might be interesting too). It shouldn't be that complicated, it's basically just like writing any other client.

#728552-12: Send private message to all friends contains an example on how to require an additional contrib project/module for a test, the test bot will then automatically download that project too. We can add the same for services here I guess.

If you have questions regarding writing tests, I'm often in #drupal-games where you can ping/ask me directly.

Oh, and one last request. It would be nice if you could upload the module as a patch, that allows me to review it with Dreditor. That is a bit tricky in CVS, but it isn't that hard with the cvsdo utility (Part of the cvsutil package on ubuntu/debian). It should work like this:

cvsdo add directory
cvsdo add directory/*
cvs diff -upN directory > directory.patch

Note that is important to explicitly use the directory in the diff command and the -N option.

Bump :)

What's the state of this? Any plans to try adding a few tests?

@Berdir,
at the moment I have not had any time to write any tests, would be awesome if anyone else can try pick this up and write some tests. Otherwise I can attempt to pick it up some time when I am free but not sure when that may be.

Subscribing, and expressing great gratitude that this is underway. I wish I could help move it along, though I'm not exactly sure what's required.

Testing is required. Try it out and report back.

StatusFileSize
new3.11 KB

@tayzlor: Thanks a lot for putting together this add-on module. I need to integrate the PrivateMSG module for an iPhone app and need a module like this.

Unfortunately, I'm not the best at writing automated tests as well, but here's some feedback from a manual test:

  1. Security issue for User ID parameter: A required parameter for all three services is $uid. In my implementation of the services module, I use key authentication and require a valid session id for security purposes. In the module from comment #8, I was able to get anyone's private messages and message counts with a valid session id. IMHO, it would make more sense to only return data for the currently logged-in user, identified by the session id.
  2. Documentation for load_full paramter: This parameter was a little confusing to me in the documentation, as it was documented as "loading the full message" when set to TRUE. What this really does is return all threads containing all messages in each thread instead of just the message previews.
  3. Missing service: Load individual thread With the available services from your module, I wasn't able to load just an individual thread, which is one of my requirements. I assume others might need this as well

I modified the module from comment #8 to accommodate these three issues (see attached file). I'd love got get your feedback on this, especially the first issue. If I misunderstood your reason for setting the $uid in the service call instead of using the currently logged-in user, I apologize in advance.

@Berdir: My apologies for not having written the required tests, but I also need some more time to learn how to write Simpletest tests for Drupal.

Manual testing is great too.

1. Not sure. I'm sure that there are use cases which will include loading messages for multiple users in a single session. I'm also not sure how it would work with other authentication mechanisms.

I really don't know much about Services, is it maybe possible to have two different groups of services? One group that only allows access messages of the current user (however that is determined) and a second group that basically allows the same methods but allows to choose the user (and maybe more stuff later).

Privatemsg itself works basically the same. Normal users have permissions to view their own messages but there is a "read all" permission that allows access to all messages.

3. Agreed, sounds useful.

The main reason I want tests is that I don't know much about Services and have never used it. The only way to keep it working is to have tests. It shouldn't be very complicated I think, all that's necessary imho is basically writing a client that uses all available services and verifies that they return the expected information (which has to be created first by using direct api functions).

StatusFileSize
new4.84 KB

Berdir:

Authentication

From the way I understand it, the Services module allows for different authentication mechanisms, it has a pluggable interface for authentication. Similar to how almost every theme for Drupal is based on phptemplate, I think most people who use authentication with Services use key authentication that comes with it.

Getting messages for other users

I agree, there might be a use case when one needs to get the messages of a user other than the currently authenticated user. For that case, I added the $uid argument as an optional argument in pm.get. In order to get messages of another user, the currently authenticated user needs to have the "read all private messages" permission that comes with the PrivateMSG module.

Testing

The Services module itself comes with a services browser, which allows the logged-in user to put in arguments and get the results without having to use an actual service. So rather than writing a test for a specific services server (xml-rpc, json etc.), I'm simply testing to make sure the that the pm_service module exposes the data correctly to the services module, using the results of the services browser. The services module then takes care of making it accessible to the various servers. Please note that this is my first test-case for Drupal. The tests cover the basic functionality including:

  • Getting multiple threads (pm.get service)
  • Getting a single thread (pm.getThread service)
  • Getting the count for unread message (pm.unreadCount service)
  • Replying to an existing thread with a new message (pm.send service)

If you could test the test and let me know if it's done right, that'd be great.

Status:Needs review» Needs work

Would be great if you could upload the module as patch (see http://drupal.org/patch/create for creating patch with new directories), then I can give you a detailed review with http://drupal.org/project/dreditor.

Some things I noticed already:

    'get private messages remotely', 'send private messages remotely'

Not sure about the permission names... maybe "through services" instead of "remotely"?

  // User needs to be authenticated to proceed

All comments should end with a period and the first character needs to be upper case (this is fine here). Exceptions like the service name in the .module file are OK.

* @param <string> $type

Great that there is already a lot of documentation. and similiar can be removed though :)

  if (is_numeric($uid) && ($uid != $user->uid)) {

is_numeric('7e5') => TRUE. That's not what we want here :) The second check is actually enough because we already checked that it $user->uid is not 0.

  elseif ($type != 'inbox' && $type != 'sent') {
    return services_error(t('Invalid type passed'), 400);
  }

I think this check can be removed. $type is just a string, it can be everything and modules can then implement the sql query alter hook to add whatever conditions they want.

  module_load_include('inc', 'privatemsg', 'privatemsg.pages');

I recently commited a patch that should make this obsolete, can you try without?

  // Load the full account object of the current user
  else {
    $account = user_load($user->uid);
  }

No reason to load the current user again imho, just do $account = $user. This also exists in other functions.

function pm_service_unread_count() {

Can we add a uid argument too here?

    // Convert the recipients string to an array of user objects.
    list($recipients, $invalid) = _privatemsg_parse_userstring($recipients);

That function does now return an array with 4 arguments, see how it is used in privatemsg_new_validate(). Also, it should be checked if $recipients is not empty and return invalid/not unique usernames.

    elseif (!empty($result[0])) {
      // If $result[0] this means the thread could not be loaded.
      return services_error($result[0], 404);
    }

Hm, this is a bug in privatemsg, shouldn't be returned like that. Care to write a patch that does return in inside $result['messages']['error'] ?

Testing
Great, yes, if we can directly test the services, then that's even better.

class PmServiceTestCase extends DrupalWebTestCase {

Let's respect the "Privatemsg" namespace, don't abbreviate the classname.

  public static function getInfo() {

Because this test relies on another project, you need to add " 'dependencies' => array('services')," to the returned array.

    parent::setUp(
      'privatemsg',
      'services',
      'services_keyauth',
      'pm_service'
    );

Arrays without keys are usually written on a single line.

    $this->assertRaw('<h3>Result</h3><code><pre>2</pre>
', t('Verify that the count of unread messages is "2".'));

This is a bit fragile but I can see why you did it ;) I wouldn't know a better way..

    // Have recipient click on the "Call method" button
    $edit = array(
      'arg[0]' => 1,
    );

privatemsg_new_thread() returns the thread_id, don't assume that it is always 1. For example, some databases are configured to increase auto_increment by 2 or more (d.o for example)

- Seems there is no test for creating a new thread yet..

Great work on the tests, looks really good to me.

Status:Needs work» Needs review
StatusFileSize
new5.06 KB

Patches: I really apologize for asking this question again, but I'm not fully understanding how I should create the patch so you can review it because I'm not sure what you want me to patch it against. Should I put this module into the /privatemsg directory and then create the patch?

In regards to your other points:

  • Permissions: I changed the permission names according to the terminology that the services module itself users (in some cases), which is "from remote". Not the biggest fan, but I just wanted to keep it consistent.
  • Comment format: I went through the code to start all comments with a capital letter and ended it with a period. I removed the duplicated documentation.
  • if (is_numeric($uid) && ($uid != $user->uid)) {
    If I remove the first check, the function doesn't work properly anymore. The first check makes sure that no $uid was passed, meaning the service call should return messages for the current user. If I take out the first check, this function never returns messages for the current user.
  • Check for 'inbox' or 'sent': I agree, removed this check
  • I removed the module_load_include, you were right, it wasn't necessary
  • I removed the unnecessary calls to user_load
  • I added an optional $uid argument when receiving the message count, so the message count for other users can be retrieved through services as well (with the proper permissions)
  • list($recipients, $invalid) = _privatemsg_parse_userstring($recipients);
    I added a check to make sure that the $invalid array is empty. If not, I'm returning an error mentioning the incorrect usernames. Earlier in the code (if ($recipients == '' && !$thread_id) {), we already made sure that $recipients is not empty.
  • I didn't get to writing a patch for the $result['messages'] part

Testing

  • I changed the testing namespace to Privatemsg
  • I added the services dependency to the getInfo() function
  • I changed the format of the array
  • Yes, I agree that making sure the result is found in the raw HTML isn't the best way to test this, but I hope it'll do for now.
  • Instead of assuming 1 for the thread_id, I'm querying the database for the actual thread_it created, which should fix this issue
  • I created an additional test for creating a new thread through the pm.send service

Yes, exactly. The problem is that new files and especially new folders make it a bit more complex than that.

If you are on Linux, try installing the cvsdo utility, that makes it quite simple (sudo apt-get install cvsutils on Ubuntu/Debian). Then it works like this:

cvsdo add pm_service
cvsdo add pm_service/pm_service.*
cvs diff -upN pm_service > pm_service.patch

That and other tools are also explained on the page I linked above.

StatusFileSize
new21.31 KB
PASSED: [[SimpleTest]]: [MySQL] 2,145 pass(es).
[ View ]

I'm running OSX and wasn't able to either get cvs utils or fakeadd installed (CVS is always good for a headache). I was following the instructions on http://drupal.org/patch/create to add the files manually locally. Also, I created the patch against the 6.x-1.3 version of the module, but since I'm just adding new files, I don't see why it wouldn't work with the 6.x-1.x-dev version.

Please let me know if there are any issues with the patch.

Status:Needs review» Needs work

+++ pm_service/pm_service.inc 3 Dec 2010 01:47:41 -0000
@@ -0,0 +1,231 @@
+  ¶
+  // User needs to be authenticated to proceed.
+  global $user;
+  if (!user_is_logged_in()) {
+    return services_error(t('This user is not logged in.'), 403);
+  }
+    ¶
+  // If a user id other than the current user's ID is passed,
+  // validate that the authenticated user has the correct ¶

There are many empty spaces everywhere, I suggest you install http://drupal.org/project/dreditor, then you can see them yourself easily in the patch.

+++ pm_service/pm_service.inc 3 Dec 2010 01:47:41 -0000
@@ -0,0 +1,231 @@
+  if (is_numeric($uid) && ($uid != $user->uid)) {

Not sure why this should be necessary but you need to at least switch this to a $uid > 0 check, and set the default value of $uid to 0.

+++ pm_service/pm_service.inc 3 Dec 2010 01:47:41 -0000
@@ -0,0 +1,231 @@
+function pm_service_unread_count($uid = '') {
...
+ */
+function pm_service_send($recipients, $subject, $body = '', $thread_id = NULL) {

Not sure if we should split new messages/replies into two separate api calls just like the privatemsg api does too. Reason is that for reply, only body and thread_id are required. Also, the logic would be much simpler for these two functions. send would have recipients, subject and body arguments, while reply would only have body and thread_id.

Also wondering if we should allow to pass in an options array, just like we do for the internal api functions.

+++ pm_service/pm_service.inc 3 Dec 2010 01:47:41 -0000
@@ -0,0 +1,231 @@
+      // Rlse there was some other problem.

This looks like a typo :)

+++ pm_service/pm_service.inc 3 Dec 2010 01:47:41 -0000
@@ -0,0 +1,231 @@
+
+}
\ No newline at end of file

Make sure to always add an empty line at the end of the file, cvs diff doesn't like it if you don't :)

+++ pm_service/pm_service.info 3 Dec 2010 01:47:41 -0000
@@ -0,0 +1,7 @@
+name = Privatemsg Service
+description = Integrates PrivateMSG functionality with Services.

Maybe use "Private messages" instead of Privatemsg as that is just the technical name?

I think this is close, thanks for your work on this! I'm confident that we can commit this soon...

Powered by Dreditor.

Assigned:Unassigned» haagendazs
Status:Needs work» Needs review
StatusFileSize
new22.85 KB
PASSED: [[SimpleTest]]: [MySQL] 2,699 pass(es).
[ View ]

Berdir: Here's a revised version of the patch. Here's a list of the changes:

  • Spaces should be removed
  • I changed the check to make sure ($uid > 0) (instead of using is_numeric)
  • I split the service calls into pm.send and pm.reply following your suggestion and adjusted the test for pm.reply
  • Fixed the typos, added a newline at the end of the files and changed the name in the info file

Let me know how this looks.

Great work. I've been lightly testing the last couple weeks, about to get more involved.

Quick feedback on a couple things I noticed just now:

pm.reply -- Arguments are listed as optional on the services browse page though they're required.

pm.getThread -- messages[i].author is returning a full user object. I'm wondering if that is a security issue or not.

Very cool! Again, great work.

Status:Needs review» Needs work

I've noticed the optional arguments on pm.reply too, those need to be changed to required.

Good point about the author object. I guess we want to remove everything except uid and username. But maybe we want to make that configurable somehow, for example so that the realname could be incuded if that is used.

A simple variable_get('pm_service_author_fields', array('uid', 'name')) could be enough for now. If someone wants more, then he can set it in settings.php. No need for an UI imho. Then the module can iterate over that array and do something like

<?php
$simple_author
->$key = $full_author->$key;
?>

Another thing, I'm wondering if we should rename the module to "privatemsg_service" so that it is within our module namespace. We might have to rename our existing pm_* modules at one point and it is certainly easier to do it now than later.

Edit: I'm going to commit this once these issues are resolved, promised! ;)

Glad you feel the same way. If desired the user object is available via user.get via the uid.

Picture is a good field to include in the basis, since it seems fundamental to Privatemsg and Drupal user presentation.

This is already understood probably, but as well as pm.getThread I see the author object appears in pm.get as well.

I had a quick look at the Privatemsg API docs (http://drupal.org/node/369399) and didn't immediately see whether deleting messages is supported via the API. I'm looking for that functionality in my build.

If it is possible then I'd be able to work on expanding the privatemsg services to enable it here. Update: I'd look to do this in early February.

That link is unfortunately outdated, see http://blog.worldempire.ch/api/privatemsg/privatemsg.api.php/group/api/6-2.

That group is actually not complete either, but all API functions can be found there.

The naming convention is:

privatemsg_[message|thread]_change_[status|delete]()

StatusFileSize
new31.41 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch privatemsg_service-DRUPAL-6--1.patch.
[ View ]

Here's a version of the module that is hopefully very close to being committed.

  • I added a README file with some basic documentation
  • I fixed the arguments for pm.reply and made them required
  • I renamed the module to privatemsg_service
  • The privatemsg.get service now has more information for the participants than the default (originally only uid, now uid + username, configurable through global variable)
  • The privatemsg.getThread service now has less information for the thread author than the default (originally full user object, now uid + username, configurable through global variable)
  • The privatemsg.getThread service now has less information for the message authors than the default (originally full user object, now uid + username, configurable through global variable)
  • I added a hook (hook_privatemsg_service_enhance_message($message)) to allow other modules to add content to each message when used in the privatemsg.getThread function

Berdir: Let me know if anything is missing here. I'd love to see this committed :)

Status:Needs work» Needs review

Sorry, forgot to change the status.

Status:Needs review» Needs work

The last submitted patch, privatemsg_service-DRUPAL-6--1.patch, failed testing.

- Getting the error "patch: **** malformed patch at line 735: Index: privatemsg_service/privatemsg_service.test" when trying to apply the patch.

- README: Great!. Can you also document at least the hook in a privatemsg_service.api.php file? Maybe even the whole API documentation in a group, see privatemsg.api.php for guidance. Advantage: We can add it as a topic here: http://api.worldempire.ch/

+++ privatemsg_service/privatemsg_service.module 17 Jan 2011 00:16:09 -0000
@@ -0,0 +1,322 @@
+      $counter++;

What exactly is $counter used for? :)

I want to see this commited too, will make it much easier to add further improvements, for example services to mark messages as deleted/read and so on.

Powered by Dreditor.

  • I'll add the privatemsg_service.api.php file
  • I'm using a $counter argument to create an array of account details for each participant in a message
  • I have no idea why the patch failed. I'm getting the same error message, but after comparing it to the previous patch, I can't really see any differences. Do you have any idea what I did wrong?

    - Not idea, can't see an obvious difference myself either. If the patch still doesn't apply the next time, feel free to upload it at as an archive.

    - Ah, haven't seen the line where you use $counter in the array, only where you incremented it.

    A few more things I've noticed...

    +++ privatemsg_service/privatemsg_service.module 17 Jan 2011 00:16:09 -0000
    @@ -0,0 +1,322 @@
    + *   Array of private message previews with more information than the participant user ids.

    Comment is too long.

    +++ privatemsg_service/privatemsg_service.module 17 Jan 2011 00:16:09 -0000
    @@ -0,0 +1,322 @@
    +function _privatemsg_service_enhance_participants($pms = '') {
    +
    +  if (!is_array($pms)) {
    +    return;
    +  }

    $pms is not an optional argument. The is_array() check isn't really necessary.

    +++ privatemsg_service/privatemsg_service.module 17 Jan 2011 00:16:09 -0000
    @@ -0,0 +1,322 @@
    +  if (!is_array($thread)) privatemsg_service

    No idea what this is but it doesn't look like PHP :)

    +++ privatemsg_service/privatemsg_service.module 17 Jan 2011 00:16:09 -0000
    @@ -0,0 +1,322 @@
    +    // Add all additional information to the message.
    +    foreach ($enhancements as $enhancement) {
    +      if (isset($enhancement['key']) && isset($enhancement['value'])) {
    +        $message[$enhancement['key']] = $enhancement['value'];
    +      }
    +    }

    Why don't you simply use array($key => $value) in your hook? Then the foreach loop is much simpler. You could probably simply use array_merge() then.

    +++ privatemsg_service/privatemsg_service.module 17 Jan 2011 00:16:09 -0000
    @@ -0,0 +1,322 @@
    +/**
    + * Implementation of hook_privatemsg_service_enhance_message($message).
    + *
    + * @param $message
    + *   Privatemsg message.
    + * @return
    + *   Associative array. The key will be used as the array key in the message
    + *   array, the value will be used as the value for that message enhancement.
    + */
    +function hook_privatemsg_service_enhance_message($message) {
    +
    +  $enhancement = array(
    +    'key' => 'test',
    +    'value' => time(),
    +  );
    +
    +  return array($enhancement);
    +
    +}

    This part is not necessary in the .module file, but you can move it to api.php. Note that the first line then shouldn't read "Implementation of.." but give a short explanation of the hook instead.

    Powered by Dreditor.

    StatusFileSize
    new31.59 KB
    PASSED: [[SimpleTest]]: [MySQL] 3,023 pass(es).
    [ View ]

    I have no idea what went wrong with the patch from #28. This time, I tested the patch locally and I was able to patch without an error. I fixed the comment, removed the "non-PHP code", simplified the implementation of module_invoke_all and created the additional api file. Fingers crossed.

    Status:Needs work» Needs review

    Go, testbot!

    Status:Needs review» Fixed

    Thanks, commited with a few modifications:

    - coding style issues (missing spaces after foreach)
    - removed unecessary variable $form_state
    - Refactored the simplify code. For example, there is now only a single, generic, user simplifiying function and the others use this one instead of having to do the whole looping in 3 different functions.

    - Extended the tests a bit to include simplify and access checks for privatemsg.get (Others could use that too, maybe as a follow-up)

    Looking forward to follow-up issues to extend and improve the api (e.g. adding methods to mark messages as read/delete).

    Version:» 7.x-1.x-dev
    Status:Fixed» Patch (to be ported)

    Oh, actually, this needs to be ported to 7.x.

    Really great work. Now that it's committed should I report bug issues here or in the Issues queue?

    privatemsg.get -- if load_full == true it causes white screen on the drupal browse page and seems to return an empty value via the service response

    privatemsg.unreadCount -- Unsure if this is by design -- Returns the number of new threads, but not new messages on existing threads

    Please create new issues, separate issues per bug/feature request. This issue is about porting this to D7 now.

    - privatemsg.get: Not sure if we actually have test coverage for that argument, probably not. It's possible that I broke something during the final refactoring. Please open an issue. It would be awesome if you can help by trying to fix the bug and/or improve the tests but an open issue about it is already a good start :)

    - This is by design. The function returns the number of thread with unread messages. This directly maps how our internal api function works. No matter which way around it is done can be confusing (if it reports that you have 5 new messages, you go to /messages and only see a single unread thread) But feel free to open a feature request if you have any ideas on how to improve it. A possibility might be to offer two API functions.

    *bump

    Hey guys,

    I ended up not using Private Msg for the app that inspired the original interest – but the need is refreshed in a new project I'm on.

    Any chance of starting this up again? We are discussing whether to use Private Msg later today, and completing this service will be a necessity if we do. Depending on the status, we might be able to contribute to the project on our end. Would enjoy hearing from you guys on where you're at with it these days.

    Jase

    Any chance for port to D7? Or anyone to help me with that?

    @czaku - same. I need privatemsg services for a mobile app. I'm happy to help get this moving. Hit me up on IRC and maybe we can get a game plan together.

    Assigned:haagendazs» drnikki

    Just to update, I'm actively developing service methods for 7.x, starting with the ones most useful to me - 1:1 message CRUD functions. C&R are basically done.

    Support for group functionality will come last. Happy to work with anyone who wants to contribute those, though. I'll be making a sandbox shortly.

    Where's the sandbox - I'm happy to help.

    drewshmaltz, hit me up on IRC tomorrow and we can work out division of labor. I'm drnikki there as well.

    Version:7.x-1.x-dev» 7.x-1.3
    StatusFileSize
    new10 KB

    Will this help?

    Can you upload this as a patch?

    Sorry, could you enlighten me in this area?

    Sorry for the late response. In Drupal issue queues, the standard is to upload changes as a Git patch. See the instructions here.

    Do you have any plans for porting this module to D7? Or could you let us know what to do if you don't have time for it?

    Thanks in advance,
    Bekir.

    Hello,

    Are there any updates with this patch for D7?

    Thank you.

    Seong Bae

    Version:7.x-1.3» 7.x-1.4
    Status:Patch (to be ported)» Needs work
    StatusFileSize
    new0 bytes
    PASSED: [[SimpleTest]]: [MySQL] 3,157 pass(es).
    [ View ]

    Pardon me for my extended period of absence. Here is the patch.

    Status:Needs work» Active

    Status:Active» Needs review

    StatusFileSize
    new18.46 KB
    FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/privatemsg/privatemsg_services/pm_service.inc.
    [ View ]

    Whoops. Realized previous patch is empty. Here is the actual one.

    Status:Needs review» Needs work

    The last submitted patch, privatemsg-added_pm_services-433780-55.patch, failed testing.

    Status:Needs work» Needs review
    StatusFileSize
    new10.2 KB
    FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in sites/default/modules/privatemsg/privatemsg_services/pm_service.inc.
    [ View ]

    Status:Needs review» Needs work

    The last submitted patch, privatemsg-added_pm_services-433780-57.patch, failed testing.

    I'm getting some error messages. but I can't figure out where they are coming from

    The last submitted patch, privatemsg-added_pm_services-433780-57.patch, failed testing.

    You should do this below.

    In pm_service.module

    'file' => array(
    - 'file' => 'inc',
    + 'type' => 'inc',
    'module' => 'pm_service',
    'name' => 'pm_service'
    ) ,

    In pm_service.inc

    foreach($query as $thread_id => $msg) {
    - $participants = privatemsg_thread_load($msg->thread_id) ['participants'];
    + $participants = privatemsg_thread_load($msg->thread_id);
    + $participants = $participants['participants'];
    $parts = array();
    foreach($participants as $participant => $usr_obj) {
    $participant_id = substr($participant, 5);
    $org_name = profile2_load_by_user($participant_id, $type_name = 'exhibitor')->field_profile['und'][0]['organisation_name'];
    if ($org_name) {
    $parts[] = $participant_id . ',' . $org_name;
    }
    }

    Now it works.

    @zhongguo999999 Any chance you could re-roll the patch? Privatemsg has a lot of automated tests and providing a patch allows us to automatically run the tests against the new code.

    @ptmkenny
    I'm sorry, I don't really know what you mean.

    After modifying the code, the pm_service module works now.

    @zhongguo999999

    On drupal.org issues that are bugs/feature requests, instead of posting code directly in comments, it is better to upload a patch file; this will help the changes get committed to the dev release of the module faster.

    You can find information on creating a patch here:
    https://drupal.org/node/707484

    Title:Service methodsAdd Service methods
    Version:7.x-1.4» 7.x-2.x-dev
    Assigned:drnikki» Unassigned
    Issue summary:View changes
    Status:Needs work» Needs review
    StatusFileSize
    new9.26 KB
    PASSED: [[SimpleTest]]: [MySQL] 4,913 pass(es).
    [ View ]

    New patch re-rolled for 7.x with zhongguo999999's changes.

    This patch has a dependency on the profile2 module, why?

    $org_name = profile2_load_by_user($participant_id, $type_name = 'exhibitor')->field_profile['und'][0]['organisation_name'];

    A module_exists if test around that statement is a minimum.

    The 'get private messages from remote' permission is a bit scary.
    Any user with that permission is able to read all messages on the site, providing the thread id.
    A new permission should be introduced 'get own private messages from remote'