Your application offers members a set of images to use as their profile photo — avatars you supply, badges, team crests, anything shared. It works. Then one member picks a different one, and the image disappears for everyone else who chose it. No error, no log entry; just a broken avatar on other people's posts, discovered days later.
This is not a bug in your code so much as a consequence of how Invision Community stores profile photos, and it is easy to walk into because the obvious implementation is the one that breaks.
Why it happens
A member's photo is two columns on core_members: pp_photo_type and pp_main_photo. The obvious thing to do is set the type to custom — that is what an uploaded photo uses — and point pp_main_photo at your file.
The problem is Member::deletePhoto(), which runs whenever a photo is replaced. It deletes the underlying file when the outgoing type is custom, letter, or begins sync-. That is correct for an uploaded photo, which belongs to exactly one member. It is destructive for an image ten members are sharing.
The supported way
Core provides for this explicitly, and documents it in a comment in Member::photoUrl():
Other - This allows an app (such as Gallery) to set the pp_photo_type to a storage container to support custom images without duplicating them
So set pp_photo_type to your file storage container key rather than to custom. Core resolves the photo with File::get( pp_photo_type, pp_main_photo ) for any type containing an underscore, and the image then appears everywhere a profile photo appears — posts, profiles, hovercards, the member list, notification emails — with no theme hook of any kind.
The container key is <app>_<FileStorage extension class>. An app myapp with extensions/core/FileStorage/Photos.php gives myapp_Photos:
/* Order matters — see below. */
$member->pp_photo_type = 'myapp_Photos';
$member->pp_main_photo = $sharedFilePath;
$member->pp_thumb_photo = NULL;
$member->photo_last_update = time();
$member->save();
Because the outgoing type is now your container rather than custom, deletePhoto() leaves the file alone, and one copy serves everyone who chose it.
Set the type before the photo
This looks like a style preference and is not. set_pp_photo_type() records the previous type, and set_pp_main_photo() then consults that record to decide whether the outgoing file should be deleted. Core notes it in a comment on deletePhoto(): "It is common to update pp_photo_type before pp_main_photo."
Assign them the other way round and a member switching from their own uploaded photo to one of yours leaves their upload orphaned in storage forever. Nothing breaks visibly; storage just grows.
The FileStorage extension is not optional here
Normally a FileStorage extension is housekeeping — it lets an administrator move your files to S3 and lets IPS account for them. When the container key is the photo type, it becomes load-bearing: remove or rename the extension and every member wearing one of your images loses their avatar, because File::get() can no longer resolve the container.
Two things follow. Assert the key rather than trusting yourself to keep two names in sync:
/* CONTAINER must equal <app>_<extension class> */
assert( MyApp::CONTAINER === 'myapp_Photos' );
And implement move() properly. Moving a file to another storage method rewrites your own table, but members store the path, not your row id — so the same pass has to update them:
Db::i()->update( 'core_members',
array( 'pp_main_photo' => $newPath ),
array( 'pp_photo_type=? AND pp_main_photo=?', 'myapp_Photos', $oldPath )
);
Miss that and the storage move reports success while every affected member gets a broken image.
Deleting one of your images
Release the members first, then remove the file:
Db::i()->update( 'core_members',
array( 'pp_photo_type' => NULL, 'pp_main_photo' => NULL, 'pp_thumb_photo' => NULL ),
array( 'pp_photo_type=? AND pp_main_photo=?', 'myapp_Photos', $file )
);
In that order, a failure part-way leaves members with no photo, which falls back to their letter avatar. In the other order it leaves them pointing at storage that no longer exists, which renders as a broken image. Tell the administrator how many members a deletion will affect before they confirm it.
DELETE FROM on your table skips whatever cleanup that path does, leaving orphaned files in storage and, worse, members still pointing at them. Reach for the application's own delete path even when a query would be faster.Counting who is wearing what
Resist keeping your own tally. A member can change their photo through core's own form at any time without telling your application, so a stored count drifts and quietly becomes fiction. Read it from the source instead:
SELECT pp_main_photo, COUNT(*) FROM core_members
WHERE pp_photo_type = 'myapp_Photos' GROUP BY pp_main_photo
One grouped query answers it for a whole screen, and it cannot be wrong.
Related
The pattern worth taking away: when core's own comments describe a mechanism — as photoUrl() does here — that is usually the supported path, and the obvious alternative is obvious because it is what a single-owner file would do. Ask who else might be pointing at the thing you are about to modify.
Related application: Profile Photo Gallery — Profile Photo Gallery uses exactly this mechanism to offer members a curated set of avatars, so one copy of each image is stored no matter how many members choose it.
Recommended Comments