Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stella’s picture

Hi,

If you have access to the server, you could ftp/scp the files to your "avatar_selection" directory. They should be automatically picked up by the module. Just ensure they have the correct file permissions. However, at present it's not possible to batch edit or delete the avatar images, which means that some development work will need to be done in order to allow you to configure which roles or organic groups are associated with each image without doing them one by one.

Cheers,
Stella

JJacobsson’s picture

Yay, that worked! Thanks! If I had thought longer than my nose I would have tried to upload one image to see where it ended up. I assumed it would work the way you described but I had put the images in the wrong directory.

But.

Having 3735 avatar images total *kills* site performance because the module relies on file_scan_directory. This becomes death.

You could provide an option to scan the directory from one of the admin pages (chugga chugga chugga chugga *ding* 3 minutes later in my case) and add a record to the table for each image it finds, then at all other times when images are to be presented, do a db_range_query and let the database deal with selecting the appropriate rows.... maybe? Not to familiar with what "organic groups" is so maybe this would in some way be incompatible with that...

Also, the pager makes 125 pages for 3735 images at 30 images per page and lists all of them. That becomes a very long list that will pretty much totaly wrek whatever theme you throw at it.

[edit] I tell a lie. It's the pager-javascript thingy that kills the site. Not the file_scan_directory. [/edit]

JJacobsson’s picture

I did some ugly hacks that adds use of the pager_query and theme_pager, a "Scan filesystem" administration page, and changed _avatar_selection_image_list so that it only relies on what's in the database table.

See attached patch for some ugly code if you want.

I have no responsibility what so ever. Just so we are clear ;)

JJacobsson’s picture

I did a lot of changes.

* Avatar Images are now organized into groups.
* Groups have weight as do Avatars for sorting. This only applies to sorting of avatar images though (first on group weight then avatar weight).
* Groups now have the access permission stuff, so you no longer set this per avatar.
* When scan the avatar directory, only new images are added and you can set what weight and group the should be added to.
* When uploading an image, you get to set what weight and group that image should belong to.
* Only images actually present in the avatar_selection table will be served.
* The old javascript is still there and still handles the mouse-over/selection stuff, but the built-in regular page-reload drupal pager is in use instead.

Phew. All in a patch file at your leisure. Not tested with postgres.

JJacobsson’s picture

http://drupal.org/node/196209 <- I completely broke this.

But that fix kinda completely breaks a site when that site has many avatars (many being somewhere in the low hundreds I would guess).

Don't know what to do for avatar selection on user registration. Put it on it's own page I guess, after the other stuff has been processed.

stella’s picture

Assigned: Unassigned » stella
Status: Active » Fixed
FileSize
7.82 KB
8.11 KB

Try the appropriate attached patch for your version of Drupal or the latest dev release (available later today).

raffi’s picture

Status: Fixed » Postponed (maintainer needs more info)

I used the patches you created and still the same performance problem. I also used the latest dev release and still the same.

stella’s picture

There were some issues with the latest dev release - wasn't possible to change avatars or select them from any page other than 1 (pretty broken really!), so I'd recommend trying the new one (available later today).

As for the original issue, ensure you clear your browser cache before testing. If the performance issue still exists, I'm not sure what I can do. The new version doesn't load the images until they are needed, but it does store the name of the file in the javascript header. This is different from the old version where all the images were loaded (ugh). The only alternative is to go back to the server each time for the next set of files but that causes any form information entered to be lost when they switch pages.

I'll look into changing how the module retrieves the list of avatars to show which might speed things up a bit further.

Cheers,
Stella

stella’s picture

Try out the latest dev release (available later today). I've changed it so that the list of images is now retrieved from the database rather than scanning the directory. However, there's a new option on the upload page that allows you to bulk upload new images by forcing a scan of the directory.

Don't forget to clear your browser cache and disable & re-enable the module (to clear drupal cache) before testing.

Cheers,
Stella

stella’s picture

Status: Postponed (maintainer needs more info) » Fixed

JJacobsson: I reviewed your patch. Obviously I couldn't implement it since it broke #196209 and didn't work with postgres. However, I did incorporate some of your ideas into the latest dev version.

  • The list of available avatars are now retrieved from the database, rather than using file_scan_directory() which probably was slowing things down.
  • There's a new "scan for new images" option on the "upload" configuration page which allows new avatars copied manually into the directory to be added to the database.
  • I've modified the paging javascript so that it doesn't load the images which shouldn't be visible.
  • You implemented an additional "grouping" which I didn't really like (didn't add any new functionality) but then still liked it because it made management of large numbers of avatars easier. It added yet another grouping mechanism when we already have roles and organic groups. So I've completely change the "manage avatars" screen so that it first shows the number of avatars per role/group, and by clicking on "edit" you can just retrieve the list of avatars assigned to that role/group and edit those.

All of these changes are present in the latest dev release (available later today) and should hopefully improve things for sites where a large number of avatars are being used.

Cheers,
Stella

raffi’s picture

Status: Fixed » Needs review
FileSize
4.86 KB

I was thinking about a way to completely get rid of the performance issue when we have few hundreds of avatar images.

The patch attached adds an option to admin/settings/avatar_selection page that enables you to choose whether to view all avatars + paging in the user/*/edit page or to view only 5 avatars on the user page then provide a link to an external page that displays all avatars.

Please snpower take a look at this.

raffi’s picture

Status: Needs review » Active

Maybe active status is more eye catching.

stella’s picture

Status: Active » Needs work

Hi raffi,

I reviewed your patch and there are a number of drupal coding standards not followed (no tabs, spacing, etc) and doesn't create the directory paths correctly. I applied the patch, fixed the drupal coding standard issues, but sorry to say the code just does not work.

The new option the settings form works fine. However the "popup" window does not. The images in the popup are not paged (the new paging util should help performance) and when I click on the image, I get a "404 not found" error. The selected image is not saved to the database. Hovering over the images and selecting an image does not change the border colour of the image. These are all to do with the fact that images in the popups are just links and not part of a form, and also because the css and js files/settings are not loaded in the popup window.

Assuming you got the above working, then there's still a couple more of big problems. First of all, the selected image is not shown on the user's account edit page (which is already open). This could cause major confusion. In addition, if the user had previously selected one the avatars on the edit page before opening the popup and selecting one there, the avatar chosen in the popup will be overwritten by the previously selected one on the main edit page when they submit the edit form.

Secondly, it doesn't work for user registrations, and can't work in its current form, since the code tries to update the users table but there is no entry for the user in the database at this point.

Finally, I don't see how it improves performance. The paging utility itself (dev version) shouldn't be the cause of any performance issues as it only loads X images. Your solution would load all the images in a popup, which could take a while to load, depending on the number of images.

I'm sorry if the above is a bit harsh, but I'm afraid that patch isn't up to a standard where I can commit it to CVS. I'm also away for the next month, so I don't have the time to make changes to get it to work. And other than having a pop-up feature, I don't see how it improves performance or what benefit it adds. Sorry!

Cheers,
Stella

raffi’s picture

Status: Needs work » Active

thanks snpower ,

I just wanted your idea regarding the popup addition. When it has all the issues you mentioned fixed does it have the chance to be applied on the original module? I have already fixed some of the issues and installed it on a working site it is working fine for my client now.
I'll create a final patch and submit it here again.

Thanks again,

stella’s picture

I'll consider it. Be sure to run your modified module through the "coder" module and fix any issues it identifies before resubmitting. Also use file_create_path() to create the path to the avatar_selection directory (not everyone uses files/avatar_selection, e.g. multisite installations).

You would need to get the issues identified above working before I will include it, including the two big ones (user registration, edit form not updated). :S

If you can get another patch submitted in the next 2-3 hours, I'll do my best to review it and get it checked in today. Otherwise, it will be April, as I'm away for the next month.

Cheers,
Stella

stella’s picture

Status: Active » Fixed

raffi: open a separate issue for the popup window.

The original issue should be greatly improved in avatar selection 5.x-2.4 and avatar selection 6.x-1.2.

Cheers,
Stella

JJacobsson’s picture

Awesome stuff! I will give it a twirl when I'm done re-theming my site ;)

I dident really like the grouping I did either... I just had to have some way to manage that many avatars and I needed it 5 hours before I installed the module so....

And to be honest, after doing some further tests (at least on my setup) I was pretty far of with my claim that file_scan_directory() was any kind of culprit. Of course, that was with just me opening the page. On a live site with a larger number of visitors the SQL server might be able to do some clever caching that the file system isent aware of.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.