EntityMetadataWrapper missing "picture", "timezone" & "signature" properties for user entity

I guess that would actually be a good way. You can combine that with an upload (file_save_data() ) as well.

  ..
  $file = file_save_data($data, $destination, $replace);
  $wrapper->picture->set($file);
  ..

I've patched a simple extension against the user module including picture, password, signature & timezone.

The only thing I'm not sure of is setting the password. It injects the plaintext in the user database, so it doesn't get hashed. Should the module handle that or the implementation of it?

Added the properties to the callbacks as well.

Thanks dsdeiz, that does indeed seem to work.
I'll manually test all fields to validate they indeed work as expected, but simpletest is still throwing an exception for the picture I don't understand.

I thought the type 'file' would be better then using 'struct' and I was under the impression that file was a valid entity type like node and user. But it is not yet behaving as I expected. Requesting $user->picture->uri->value() for example fails, while $node->author->mail->value() does work. Perhaps any of you has an idea on this?

  $properties['picture'] = array(
    'label' => t("Picture"),
    'description' => t("The user's picture."),
    'getter callback' => 'entity_metadata_user_get_properties',
    'setter callback' => 'entity_property_verbatim_set',
    'type' => 'file',
    'access callback' => 'entity_metadata_user_properties_access',
    'schema field' => 'picture',
  );

Oke, it took me a while to realise I had to update the test as well. So here it is.

The test kept failing when I attempted to get and set the picture property, but then I found that the node test does not include the author property as well. So I suppose the test is incompatible with the entity metadatawrapper chaining.
The picture property return test is working and it returns a created file, so I guess this is already tested elsewhere. Perhaps someone could confirm this?

I've tested most of the properties successfully. The only thing I'm worried about is when a user picture is uploaded to another folder than the default user pictures folder, it might not pass hook_file_download(). Would it be the responsibility of this module to move the file or should the user do that?

You are right. I was under the impression Drupal emptied the password after loading, but it is in the global user object as well so I'll just return the hashed pass (although I don't see a use case for it).

Do you perhaps have an idea on the picture folder as well?

Yeah sure.

Since the image is a file now, it's access is checked by entity_metadata_file_access(), which in turn calls hook_file_download() to check if the user can access the file. That code looks like this:

function hook_file_download($uri) {
  // Check if the file is controlled by the current module.
  if (!file_prepare_directory($uri)) {
    $uri = FALSE;
  }
  if (strpos(file_uri_target($uri), variable_get('user_picture_path', 'pictures') . '/picture-') === 0) {
    if (!user_access('access user profiles')) {
      // Access to the file is denied.
      return -1;
    }
    else {
      $info = image_get_info($uri);
      return array('Content-Type' => $info['mime_type']);
    }
  }
}

So it basically checks if the image is in the user_picture_path (the default location for the user form to upload images). And returns true if the user profile is visbile. However with the patch provided any managed file can be set as a user image which might not even be visible.
Therefor, should I attempt to move the file to the correct folder in entity_metadata_user_set_properties(), should I throw an exception or should I do nothing and leave this responsibility to the person using the wrapper.

I suppose it would be best to implement a validation callback just like the mail property has. And check if the picture path is valid. Then we should document somewhere that the files have to be like: public://pictures/picture-<uid>-<timestamp>.<ext>
At that same validation we can check if the file is actually an image.

Moving files seems like a bad idea as we would to load the file with file_load(), then move it with file_move and save it again. Where the user might have already have used entity_load or an entity metadata wrapper to attach it to the user, so we would have to clear those caches as well to be sure. Seems like a potential lot of overhead to me.

Thanks for your reply. I forgot to create a local commit last time, so I don't have an interdiff, but the callback is added and the test has been edited a bit.

On my test environment the file move in the following part failed, but that might be due a misconfigured wamp. (Simpletest doesn't work on my server due to a basedir restriction) I can't figure out what goes wrong and the code looks good to me:

    // For the user picture property we pass an image file
    if ($info['name'] == 'picture') {
      $file = $this->createFile('image');
      // Fix name, filename and uri
      $file = file_move($file, 'public://' . variable_get('user_picture_path', 'pictures') . '/picture-' . $file->uid . '-' . $file->timestamp . '.' . pathinfo($file->filename, PATHINFO_EXTENSION));
      return $file;
    }

Apparently I needed some extra streamwrapper functions, that I found in the user_save() function. Attached is a working patch and interdiff.

I recently figured there are still some issues with settings pictues and signatures. I will try to fix this in the upcoming days.

Hi doors,

I've initially written this patch so the fields became available in Entity metatadata Wrappers and RestWS. So I have not tested it with other API's. I'd however expect that it should work. Could you describe a bit more what you are trying to do and with what modules?
have you cleared the caches? Have you enabled the picture field in admin/config/people/accounts ?

Please be as detailed as possible.

It was not showing because I have not thought of it. However when you change line 398 in entity/modules/callbacks.inc from

return isset($account->picture) ? $account->picture : NULL;

to

return isset($account->picture) ? $account->picture : variable_get('user_picture_default', '');

it should work.

Oh wait, probably not. I'll have to write some more code to do that. I'll do some tests and get back to you.

After looking into this I believe it is not related to this patch.

Drupal generates the picture in template_preprocess_user_picture() and then it displays it in user-picture.tpl.

These templates and processors are only used when displaying either the 'Rendered user', or the 'Rendered user picture'. (The last one in Views is called: 'User: Picture The user's picture, if allowed.' and I expect that one only to be available when a full user object is loaded (Search API loads only a selection of fields)).

What you could do is use Views' no result behavior, and when the user's picture is empty, rewrite the output to <img src="path/to/default/picture" alt="Default user picture">. You could use some replacement patterns to get the user's name in the alt attribute.

Hope this helps.

Well, I guess I finally have them all working.

The following is now possible:

$user = entity_metadata_wrapper('user', $uid);

$user->password->set("newpassword");
$user->timezone->set("Europe/Amsterdam");
//
// Signatures work like node bodies;
$user->signature->value->set("signature");
$user->signature->format->set("filtered_html");
//
// Create a picture here
$data = 'binary image datastring';

$picture = file_save_data($data, 'temporary://picture.png', FILE_EXISTS_RENAME);
$picture->status = 0; // Set the file temporary so the user_save function will take it.
$picture = file_save($picture);
//
// Save the picture
$user->picture->set($picture);
//
// Save the user
$user->save();

Turned out the picture validation was still commented out due debugging. Also fixed some trailing whitespaces.

Thanks for the feedback, however I don't quite get what you mean. The signature as you describe here is like I have it. Or do you want me to remove the password?

Good point. Since the module is built with ternary operators, I've decided to use those here as well.

Just found out that my small change from 'pass' to 'password' was causing all passwords to get double hashed, so nobody could log in anymore. So that has been reverted in this patch.

Did you test this and confirm it worked? Then the status of the ticket can be set to 'reviewed and tested by the community' and we might get it in.

You can do it in function entity_metadata_user_save in modules/callbacks.inc
I once had this there, but my project did not require pictures anymore. Guess I rolled the patch from the wrong repository.

I'll reroll a patch including this in a few days.

/**
 * Callback to save a user account.
 */
function entity_metadata_user_save($account) {
  // Load the user picture file object
  $account->picture = file_load($account->picture);
  
  $edit = (array) $account;
  // Don't save the hashed password as password.
  unset($edit['pass']);
  user_save($account, $edit);
}

I noticed that earlier, and I suppose it is a flaw in Drupal (you can't set a picture during registratration as well). We could also handle that in entity_metadata_user_save, but I guess it is a bit hackish... It would be user friendly though...

I suppose it would become something like this, but I can't test right now:

/**
 * Callback to save a user account.
 */
function entity_metadata_user_save($account) {
  $picture = $account->picture;
  if($account->is_new) {
    $account->picture = '';
    $edit = (array) $account;
    // Don't save the hashed password as password.
    unset($edit['pass']);
    user_save($account, $edit);
  }

  // Load the user picture file object
  $account->picture = file_load($picture);
  
  $edit = (array) $account;
  // Don't save the hashed password as password.
  unset($edit['pass']);
  user_save($account, $edit);
}

It should be working without the additional save now.

The full example from #32 should be working now.

Thanks for taking the time to correct some issues edwardaa. The patch attached to #47 was my latest attempt and that seems to be working (though our suggestions could improve it).
Did you create your patch based on that one?

If I am not mistaken, the user_save function already moved the image to public://pictures (and renamed them if they existed). This was the reason for me to keep the pictures in temp and let user_save handle the rest, so the pictures folder won't contain two duplicates of every file.

In my opinion the tests should follow this behavior. But you might want to check above, as it has been a while sinds I worked on this.

Great, it would be nice to hear something from the maintainer(s) about wheter or not they would want to commit this.