wecodemore/wcm-avatar

A WordPress plugin to enable custom attachment uploads to be used as user avatars

wcm-avatar's Issues

Check if user already has avatar

When uploading an image and adding it to the user no check if the user might already have an avatar assigned is performed. This means that:

1. The User is assigned the avatar uploaded last.
2. Both attachments are still linked to the same user from the attachment side.

How to reproduce

How does this even happen you might ask? Imagine a user with no avatar. Then you open two tabs with the Profile. And then upload avatars in both open tabs.
The assumption that one can't upload an avatar a second time because the Upload UI disappears after the first upload isn't valid if the UI was already loaded earlier. Also of course this needs to be properly handled server-side.

Possible Solution

On upload check if the user already has an avatar assigned.
If yes then:

• delete the attachment
• or remove the link between avatar attachment and user from both ends
• or outright refuse to upload the second avatar

Move wiki contents to README

As the wiki is hidden and will not be shipped with the plugins code, it would be better to move everything to the README to not hide away important documentation.

Multi Uploader breaks when Upload fails

To reproduce just try uploading an SVG file.

At first glance it seems as if the response from async-upload.php is assumed to always be an image, while this can also be an error message if the upload fails. This is then put in the template where the <img> is expected breaking the template somehow.

AvatarMIMELimitService limits mime types for all uploads, but only for the browser-uploader

I'm not sure how that came to be. Maybe first it was restricted to only the profile page and later was extended to make the multi-uploader work not considering that it now affects all uploads.

So if we really want to restrict the upload here we need to make sure it only happens for avatar uploads which will be hard for async uploads. This is the same problem we'd have in #49

Otherwise we just get rid of this and consider it out of scope for the plugin.

Disable Multi-Selection

Currently Plupload allows multi-selection which also causes all files to be uploaded. This is somewhat related to #24.

Using the plupload_init filter this could be disabled in the UI.
For the server side of things it seems as if each file is uploaded independently via a separate request, so when #24 is done I'd say everything after the first upload would just fail.

Avatar Settings Page ends up weird

As we hijack the output of get_avatarthis also affects the output on wp-admin/options-discussion.php showing the same image (the currently logged in users avatar) for all options.

It seems as if the default_avatar_select filter makes it possible to alter the display of those options.

This means we can probably display our own default here or just completely hide these settings. Also see #30 that already covers the default/fallback issue.

Replace "Logo"-String everywhere

There are still "logo"-string leftovers from conversion from from the old plugin. Those need to get replaced. Search results here on GH.

Edit: A quick cross file search for the logo-string:

grep --exclude=*.min.js --exclude=*.min.css -rnw . -e "logo"

…brings up the following result in the current version:

./assets/src/logo/all.js:96:    window.wcm.logo = window.wcm.logo || {};
./assets/src/logo/all.js:97:    window.wcm.logo.models = models;
./assets/src/logo/all.js:109:       //template : wp.template( plugin.templates.logo ),
./assets/src/logo/all.js:110:       template : _.template( $( plugin.templates.logo ).html(), null, { variable : 'data' } ),
./assets/src/logo/all.js:213:   window.wcm.logo = window.wcm.logo || {};
./assets/src/logo/all.js:214:   window.wcm.logo.views = views;
./assets/src/logo/all.js:220:   var logo = new models.Logo(),
./assets/src/logo/all.js:224:   logo.set( _.extend( plugin.data, {
./assets/src/logo/all.js:230:   logo.on( 'change', logo.fetch );
./assets/src/logo/all.js:235:       model : logo
./assets/src/logo/all.js:241:       model : logo
./assets/src/logo/all.js:247:       model : logo
./assets/src/logo/all.js:253:       model : logo
./assets/src/logo/all.js:268:           logo.set( { 'att_id' : response.response } );
./assets/src/logo/all.js:271:} )( window, jQuery, _, logoUploader, window.wcm.logo.models, window.wcm.logo.views );
./assets/src/logo/controller.js:5:  var logo = new models.Logo(),
./assets/src/logo/controller.js:9:  logo.set( _.extend( plugin.data, {
./assets/src/logo/controller.js:15: logo.on( 'change', logo.fetch );
./assets/src/logo/controller.js:20:     model : logo
./assets/src/logo/controller.js:26:     model : logo
./assets/src/logo/controller.js:32:     model : logo
./assets/src/logo/controller.js:38:     model : logo
./assets/src/logo/controller.js:53:         logo.set( { 'att_id' : response.response } );
./assets/src/logo/controller.js:56:} )( window, jQuery, _, logoUploader, window.wcm.logo.models, window.wcm.logo.views );
./assets/src/logo/models.js:97: window.wcm.logo = window.wcm.logo || {};
./assets/src/logo/models.js:98: window.wcm.logo.models = models;
./assets/src/logo/views.js:11:      //template : wp.template( plugin.templates.logo ),
./assets/src/logo/views.js:12:      template : _.template( $( plugin.templates.logo ).html(), null, { variable : 'data' } ),
./assets/src/logo/views.js:115: window.wcm.logo = window.wcm.logo || {};
./assets/src/logo/views.js:116: window.wcm.logo.views = views;
./assets/style.css:26:#tmpl-main--container .logo--caption {
./assets/style.css:42:.logo--caption .edit-attachment {
./assets/templates/caption.tmpl:1:<p class="logo--caption" style="background-color: <%= data.color1 %>;">
./scripts/scripts:2:concat-cli -f assets/src/logo/models.js assets/src/logo/views.js assets/src/logo/controller.js -o assets/src/logo/all.js
./scripts/scripts:4:minify assets/src/logo/models.js     > assets/src/logo/models.min.js
./scripts/scripts:5:minify assets/src/logo/views.js      > assets/src/logo/views.min.js
./scripts/scripts:6:minify assets/src/logo/controller.js > assets/src/logo/controller.min.js
./scripts/scripts:7:minify assets/src/logo/all.js        > assets/src/logo/all.min.js
./src/Services/AvatarScriptsService.php:69:     $url  = "{$this->url}assets/src/logo";
./src/Services/AvatarScriptsService.php:70:     $path = "{$this->root}assets/src/logo";
./src/Services/AvatarScriptsService.php:82:         ? 'logo-upload'
./src/Services/AvatarScriptsService.php:83:         : 'logo-models';
./src/Services/AvatarScriptsService.php:90:             'logo'    => '#tmpl-logo',
./src/Services/AvatarScriptsService.php:114:            'logo',
./src/Services/AvatarScriptsService.php:141:            'logo-upload',
./src/Services/AvatarScriptsService.php:163:            'logo-models',
./src/Services/AvatarScriptsService.php:175:            'logo-views',
./src/Services/AvatarScriptsService.php:177:            [ 'logo-models', ],
./src/Services/AvatarScriptsService.php:182:            'logo-controller',
./src/Services/AvatarScriptsService.php:184:            [ 'logo-views', ],
./src/Templates/AvatarDisplayTemplate.php:75:                       <p class="logo--caption" style="background-color: {$scheme[1]}">
./src/Templates/AvatarUploadTemplate.php:84:                    <tr class="company-logo-upload-wrap">

Do not filter MIME type during cron

No one will upload manually during cronjob runs.

Set minimum image dimension for upload

The upload size should not just be restricted by a max. The sizes should be determined by the image sizes that are set with the filter mentioned in #7 and fetched via the max and min sizes provided there.

Add front end/ theme uploader

The uploader should mostly work like the one in the admin UI user profile screen. This is needed to reuse the same filters everywhere.

The uploaders setup process as well as the rendering logic should get attached to a filter. This is to allow setting the uploader up while still being able to turn off the rendering later on with a switch that might not be available early on.

Fix Avatar Replacement filter name

The wcm.avatar.size filter is meant to allow changing the avatar arguments. Therefore it's named wrong. Should be changed now.

Avatar Upload Template breaks rendering

The AvatarUploadTemplate::display() method breaks rendering. This needs to be simply rendered.

Do not check for capability if avatar is for own profile

This check should only occur for others profiles.

Avatar in admin bar breaks out

The currently logged in user avatar currently breaks out of the admin bar if s/he has a custom avatar. The desired Markup as used with the Gravatar service is:

<a class="ab-item" aria-haspopup="true" href="https://example.com/wp/wp-admin/profile.php">
    Howdy, admin
    <img alt="" src="https://1.gravatar.com/avatar/d402074cd4c1b928f1a3489d83b8ce15?s=26&amp;d=mm&amp;r=g" srcset="https://1.gravatar.com/avatar/d402074cd4c1b928f1a3489d83b8ce15?s=52&amp;d=mm&amp;r=g 2x" class="avatar avatar-26 photo" height="26" width="26">
</a>

… while the user has the following Markup with a custom avatar:

<a class="ab-item" aria-haspopup="true" href="https://nzz.wp/wp/wp-admin/profile.php">
    Howdy, admin
</a>
<a href="https://nzz.wp/wp/wp-admin/post.php?post=1988&amp;action=edit">
    <img width="26" height="26" src="https://nzz.wp/wp-content/uploads/2015/08/avatar.jpeg" class="" alt="avatar" user_avatar="1">
</a>

Reconsider positioning of UI on profile page

I am not sure it is reasonable that the Avatar UI on the profile page is on the very top, somewhat hackishly implemented via the all_admin_noticeshook. show_user_profile and edit_user_profile might be more reasonable choices.

Good news: Just switching the hook to one of these two works flawlessly. Once #26 is done this might be an easy fix.

Reconsider using file_get_contents() in UnderscoreTemplateScripts

Is it really necessary to use file_get_contents with a URL here? Can't we get this from the local filesystem?

Reason why I'm asking is that I've just run into an issue while working on a dev site with a self-signed certificate where this fails as the server (somewhat ironically) isn't aware of his own self-signed certificate.

Don't skip classes by name

In the ServiceDispatcher there is a filter that skips all classes with the same name. Let's remove that filter until there is a need for that.

AvatarScriptsService lacks check for permissions

The check does only check if we are on a profile screen, but not if we have the proper rights.

This causes a JS error TypeError: uploader is undefined when editing another users profile while not having upload_files permission.

Should probably be something like here.

Use correct text domain

Looks like C&P mistakes:

client_domain vs wcmavatar: here and here.

Fix avatar not displayed on user profile screen

There's some wrong switch somewhere that denies showing the avatar in the user profile (or edit) screen. The avatar itself gets uploaded and stored correctly.

Add branch alias for composer.json

Add branch alias to dev

Fix/ Add docblocks

Some docblocks and such aren't complete.

Remove branch-alias from Composer during dev

The branch-alias currently doesn't help. Should be moved in after there's a stable release.

Fix wrong tag in composer.json

Composer tags are not allowed to use & in the name

AvatarBrowserUploaderSaveService breaks on non-english locale

This check for Upload checks for a string that gets translated. So in German this ends up as Hochladen and always fails.

Limit image sizes to custom sizes defined in a filter

Currently the attachment gets generated in every size that is registered. This is not desired. There needs to be a filter that allows restricting the sizes – and falls back to all sizes if nothing is registered.

Use the post_thumbnail_size filter for that.

AddUserMetaService seems to be unused

Maybe this was originally intended to be used in places like AvatarAddMetaService.php:69-74 ?

As developer I want to have a complete build during installation

Currently we have a Git hook to run the NPM build process, which then uses bash scripts to call the proper commands. We also have a .po-file generator that sits in the root. All this should better be summoned and run during a Composer install script as this is when the package gets pulled and set up. In addition, we should continue to ship the assets in minified versions to have a fallback.

Create *.pot file

Whatever this is it doesn't seem to have anything to do with this plugin.

Gravatar Urls for Highres

If there's no image/ attachment/ user photo uploaded and found, the get_avatar() function falls back to just the call to Gravatar. Now the srcset adds the 2x at the end of the Url. This should get changed to just double the size for this request. See here for arguments.

Remove (apparently) unused AdminStylesService

Services/AdminStylesService.php adds Devicons which as far as I can tell aren't used anywhere. Probably just remove it.

AvatarUploadSizeLimit only affects multi-uploader (plupload)

The upload_size_limit filter used for AvatarUploadSizeLimit was never meant to be a strict server side check. Instead it is only a hint for plupload to already decline files to big for (by default) the php.inisettings on the client side.

So if we really want to restrict the upload size we'd need to hook into wp_handle_upload_prefilter just like we do for AvatarDimensionLimitService and AvatarExistsLimitService.

Limit Underscore template to user profile screens only

The template placeholders are printed to each admin screen. Should be limited to user profile screen only.

Default/Fallback avatar

In contrast to default WP behaviour nothing (an empty string) is returned when the user doesn't have an avatar assigned. This means that you can't have a default avatar as the default argument of get_avatar is ignored.

There are multiple options to handle this:

1. Fall back to default behaviour (Gravatar) when no local avatar is set

This is probably expected behaviour when this plugin is seen as an extension of default WP behaviour.

2. Only fall back to default if it is a URL

This is probably expected behaviour if this plugin is seen as a replacement of default WP behaviour. Especially since one of the benefits of this plugin is/can be not to leak any information to Gravatar this should in any case be an option.

---

Personally I'd prefer option 2 as it is the safe default. But option 1 is probably more aligned with user expectations. A filter for this should exist in any case.

AvatarBrowserUploaderSaveService nests errors one level to deep

media_handle_upload() only returns one WP_Error object which then might contain multiple errors. So we shouldn't store this error as an array but as a plain object.

$this->error[] = $result;

should instead be

$this->error = $result;

Otherwise the error won't be displayed inside media_upload_form() as the filter returns an array containing a WP_Error instead of the object itself.

AvatarDimensionLimitService and AvatarExistsLimitService only affect Multi-Uploader

As we only check for the POST variables which are not present when using the single-file browser uploader both always bail out early.

wcm-avatar/src/Services/AvatarDimensionLimitService.php

Lines 29 to 30 in 24292fc

	$screen = filter_input( INPUT_POST, 'screen_id', FILTER_SANITIZE_STRING );
	if ( ! in_array( $screen, [ 'profile', 'user-edit' ], TRUE ) )

and

wcm-avatar/src/Services/AvatarExistsLimitService.php

Lines 28 to 29 in 24292fc

	$screen = filter_input( INPUT_POST, 'screen_id', FILTER_SANITIZE_STRING );
	if ( ! in_array( $screen, [ 'profile', 'user-edit' ], TRUE ) )

Uploader broken

Currently there's a problem with the uploader where the uploader believes there already is a custom avatar present. When an existing image is deleted, the error message is broken as well and shows as in the second image.

There are no errors in the console.

Consider comments as input argument

Yes. Comments. Honestly. See: Dashboard "Activity" meta box. No where it is used. But hey!

Make Plugin work for users without `upload_files` permission

Currently the plugin checks for upload_fileseverywhere.
Also when using the multi-uploader everything sits on top of wp-admin/async-upload.php which also checks for this permission.

So to make this work we'd need to find a safe way to enable that. Especially for async-upload.php this also needs to make sure that we only enable the upload for the avatar and not for everything. Maybe something like a nonce (a real one, not a WP nonce).

Also this would probably be the point where we should really have some central helpers to determine these kind of things. See #37.

Keep references to callbacks in Bootstrap

It is great that the plugin is modular so in theory you could replace bits and pieces. But not keeping a reference to the objects hooked in Bootstrap.php makes it hard to actually do it without resorting to black magic.

Add better labels for GitHub

The current labels are less than optimal.

Avatar filter callback expects ID or email

The avatar filter callback should not get converted to int in case an email is provided (instead of an ID).

Helpers for common checks

There are plenty of places where things like "Does the user have an avatar set?" or "Is this upload on/from the profile page?" are checked. Currently a lot of these checks are just C&P. Maybe it is worth having some central helper that does this in a reusable way.

Use Uploaded Custom Avatar instead of Gravatar in User List screen

There's by now an additional column in the admin screen that shows the uploaded attachment. We should replace the original Gravatar when there's already an attachment uploaded.

Browser Uploader is broken

...because $_REQUEST['user_id'] is actually never set therefore this condition always fails.

The sad thing also is that the upload itself works, just the linking up with the user is skipped leaving an abandoned upload in the Media Library.

This could probably be fixed with a hidden user_idfield somewhere here but I haven't thought this through yet...

As a user who uploaded a custom avatar, I do not want to get fooled about using Gravatar

With WP 4.4, a description below a users avatar on its profile page was introduced. It is advertising for Gravatar and of no use if the user already uploaded a custom avatar. As we already properly display a custom avatar there, we should change this.

It only shows when IS_PROFILE_PAGE is set and true. The user_profile_picture_description-filter allows changing this string. We should do that:

add_filter( 'user_profile_picture_description', function( $descr ) {

    if ( 
        defined( 'IS_PROFILE_PAGE' ) and IS_PROFILE_PAGE
        && apply_filters( 'wcm.avatar.enable_custom_avatar', TRUE )
    ) {
        $has = get_user_meta( 
            get_current_user_id(), 
            apply_filters( 'wcm.avatar.meta_key', 'user_avatar' ),
            TRUE 
        );
        $descr = $has
            ? 'You can change your avatar by defining a new one <a href="#tmpl-main--container">here</a>.'
            : 'You can upload an avatar <a href="#tmpl-main--container">on top</a>.';
    }

    return $descr;
} );

/Cc @stephenharris Maybe you got some input on how to phrase the description?