diff --git a/resources/templates/users/_photo.twig b/resources/templates/users/_photo.twig deleted file mode 100644 index c54facd65b2..00000000000 --- a/resources/templates/users/_photo.twig +++ /dev/null @@ -1,33 +0,0 @@ -{% set volume = Volumes.getUserPhotoVolume() %} -{% if volume %} - {% set id = id ?? "userphoto-#{random()}" %} -
-
- {% if user.photoId %} - {{ user.getThumbHtml(100)|raw }} - {% else %} - {{ svg('@resources/images/thumbs/user.svg')|prepend(tag('title', { - text: user.getName(), - })) }} - {% endif %} -
-
- - {% if user.photo %} -
- - -
-
- -
- {% else %} -
- -
- {% endif %} -
-
-{% else %} -

{{ 'Please set a valid volume for storing the user photos in user settings page first.'|t('app') }}

-{% endif %} diff --git a/routes/actions.php b/routes/actions.php index 5c1694b8ba1..f4bb1e1e0f1 100644 --- a/routes/actions.php +++ b/routes/actions.php @@ -86,7 +86,6 @@ use CraftCms\Cms\Http\Controllers\Users\PasskeysController as UserPasskeysController; use CraftCms\Cms\Http\Controllers\Users\PasswordController; use CraftCms\Cms\Http\Controllers\Users\PermissionsController; -use CraftCms\Cms\Http\Controllers\Users\PhotoController; use CraftCms\Cms\Http\Controllers\Users\PreferencesController; use CraftCms\Cms\Http\Controllers\Users\RecoveryCodesController; use CraftCms\Cms\Http\Controllers\Users\SaveUserController; @@ -512,9 +511,6 @@ Route::post('users/save-preferences', [PreferencesController::class, 'store']); Route::post('users/delete-user', [UsersController::class, 'destroy']); Route::post('users/user-content-summary', [UsersController::class, 'contentSummary']); - Route::post('users/render-photo-input', [PhotoController::class, 'renderInput']); - Route::post('users/upload-user-photo', [PhotoController::class, 'upload']); - Route::post('users/delete-user-photo', [PhotoController::class, 'destroy']); Route::post('users/require-password-reset', [PasswordController::class, 'requireReset']); Route::post('users/remove-password-reset-requirement', [PasswordController::class, 'removeResetRequirement']); Route::post('users/verify-password', [PasswordController::class, 'verifyPassword']); diff --git a/src/FieldLayout/LayoutElements/Users/PhotoField.php b/src/FieldLayout/LayoutElements/Users/PhotoField.php index 8eaf327a18a..c8d83b47762 100644 --- a/src/FieldLayout/LayoutElements/Users/PhotoField.php +++ b/src/FieldLayout/LayoutElements/Users/PhotoField.php @@ -4,20 +4,29 @@ namespace CraftCms\Cms\FieldLayout\LayoutElements\Users; +use CraftCms\Cms\Asset\Data\VolumeFolder; +use CraftCms\Cms\Asset\Elements\Asset; use CraftCms\Cms\Element\Contracts\ElementInterface; use CraftCms\Cms\FieldLayout\LayoutElements\BaseNativeField; +use CraftCms\Cms\Filesystem\Exceptions\InvalidSubpathException; +use CraftCms\Cms\Filesystem\Filesystems\Temp; use CraftCms\Cms\Support\Arr; +use CraftCms\Cms\Support\Facades\Folders; use CraftCms\Cms\Support\Facades\HtmlStack; use CraftCms\Cms\Support\Facades\InputNamespace; use CraftCms\Cms\Support\Facades\ProjectConfig; +use CraftCms\Cms\Support\Facades\Sites; use CraftCms\Cms\Support\Facades\Volumes; use CraftCms\Cms\User\Elements\User; +use CraftCms\Cms\View\LegacyAssets\CpAsset; use CraftCms\Cms\View\LegacyAssets\InternalAssetRegistry; -use CraftCms\Cms\View\LegacyAssets\UserPhotoAsset; use Illuminate\Support\Facades\Auth; +use Illuminate\Support\Facades\Gate; use InvalidArgumentException; use Override; +use Throwable; +use function CraftCms\Cms\renderObjectTemplate; use function CraftCms\Cms\t; use function CraftCms\Cms\template; @@ -75,25 +84,88 @@ protected function inputHtml(?ElementInterface $element = null, bool $static = f return null; } - app(InternalAssetRegistry::class)->register(UserPhotoAsset::class); - $inputId = sprintf('user-photo-%s', mt_rand()); + $folder = $this->userPhotoFolder($element); - HtmlStack::jsWithVars(fn ($userId, $inputId, $isCurrentUser) => <<register(CpAsset::class); + $inputId = sprintf('user-photo-%s', mt_rand()); + $namespacedInputId = InputNamespace::namespaceId($inputId); + $uploadFs = $volume->getFs(); + $canUpload = ! $uploadFs instanceof Temp && Gate::check("saveAssets:$volume->uid"); + + if ($canUpload) { + HtmlStack::jsWithVars(fn ($inputId, $folderId) => << { + const userPhotoInput = $('#' + $inputId).data('elementSelect'); + if (userPhotoInput?.uploader) { + userPhotoInput.uploader.setParams({folderId: $folderId}); + } else if (!userPhotoInput || userPhotoInput.canUpload) { + setTimeout(setUserPhotoUploadFolder, 0); + } else { + return; + } +} +setUserPhotoUploadFolder(); JS, [ - $element->id, - InputNamespace::namespaceId($inputId), - $element->getIsCurrent(), - ]); + $namespacedInputId, + $folder->id, + ]); + } - return template('users/_photo', [ + return template('_components/fieldtypes/Assets/input', [ 'id' => $inputId, - 'user' => $element, + 'name' => 'photoId', + 'jsClass' => 'Craft.AssetSelectInput', + 'elementType' => Asset::class, + 'elements' => $element->getPhoto() ? [$element->getPhoto()] : [], + 'condition' => null, + 'criteria' => [ + 'kind' => 'image', + 'folderId' => $folder->id, + 'siteId' => $element->siteId ?? Sites::getCurrentSite()->id, + ], + 'sources' => ["volume:$volume->uid"], + 'storageKey' => 'userPhoto', + 'fieldId' => null, + 'single' => true, + 'limit' => 1, + 'defaultPlacement' => 'end', + 'selectionLabel' => t('Choose a photo'), + 'viewMode' => 'large', + 'showActionMenu' => true, + 'canUpload' => $canUpload, + 'fsType' => $uploadFs::class, + 'defaultFieldLayoutId' => $volume->fieldLayoutId ?? null, + 'defaultSource' => "volume:$volume->uid", + 'defaultSourcePath' => $folder->getSourcePathInfo() ? [$folder->getSourcePathInfo()] : null, + 'showFolders' => true, + 'showSourcePath' => true, + 'sourceElementId' => $element->id, + 'modalSettings' => [ + 'defaultSiteId' => $element->siteId ?? null, + ], ]); } + private function userPhotoFolder(User $user): VolumeFolder + { + $volume = Volumes::getUserPhotoVolume(); + if (! $volume) { + throw new InvalidArgumentException('Invalid user photo volume.'); + } + + $subpath = (string) ProjectConfig::get('users.photoSubpath'); + + if ($subpath !== '') { + try { + $subpath = renderObjectTemplate($subpath, $user); + } catch (Throwable) { + throw new InvalidSubpathException($subpath); + } + } + + return Folders::ensureFolderByFullPathAndVolume($subpath, $volume); + } + #[Override] protected function actionMenuItems(?ElementInterface $element = null, bool $static = false): array { diff --git a/src/Http/Controllers/Users/PhotoController.php b/src/Http/Controllers/Users/PhotoController.php deleted file mode 100644 index f1ebcd8ac60..00000000000 --- a/src/Http/Controllers/Users/PhotoController.php +++ /dev/null @@ -1,129 +0,0 @@ -validate([ - 'userId' => ['required', 'integer'], - ]); - - $user = $this->users->getUserById($request->integer('userId')); - - abort_if(! $user, 400, 'Invalid user ID: '.$request->integer('userId')); - - return $this->renderPhotoTemplate($request, $user); - } - - public function upload(Request $request): Response - { - $request->validate([ - 'userId' => ['required', 'integer'], - ]); - - if ($request->integer('userId') !== $request->user()->id) { - $this->authorize('editUsers'); - } - - if (! $request->hasFile('photo')) { - return new JsonResponse; - } - - try { - $uploadedFile = $request->file('photo'); - - $user = $this->users->getUserById($request->integer('userId')); - - abort_if(! $user, 400, 'Invalid user ID: '.$request->integer('userId')); - - // Move to our own temp location - $fileLocation = AssetsHelper::tempFilePath($uploadedFile->extension()); - $file = $uploadedFile->move(dirname($fileLocation), basename($fileLocation)); - $this->users->saveUserPhoto($fileLocation, $user, $uploadedFile->getClientOriginalName(), $file->getMimeType()); - - return $this->renderPhotoTemplate($request, $user); - } catch (Throwable $exception) { - if (isset($fileLocation) && file_exists($fileLocation)) { - File::delete($fileLocation); - } - - Log::error('There was an error uploading the photo: '.$exception->getMessage()); - - return $this->asFailure(t('There was an error uploading your photo: {error}', [ - 'error' => $exception->getMessage(), - ])); - } - - } - - public function destroy(Request $request, Elements $elements): JsonResponse - { - $request->validate([ - 'userId' => ['required', 'integer'], - ]); - - $user = $this->users->getUserById($request->integer('userId')); - - abort_if(! $user, 400, 'Invalid user ID: '.$request->integer('userId')); - - if ($user->photoId) { - $elements->deleteElementById($user->photoId, Asset::class); - } - - $user->photoId = null; - $elements->saveElement($user, false); - - return $this->renderPhotoTemplate($request, $user); - } - - private function renderPhotoTemplate(Request $request, User $user): JsonResponse - { - $templateMode = TemplateMode::get(); - if (TemplateMode::is(TemplateMode::Site) && ! app(TemplateResolver::class)->exists('users/_photo.twig')) { - $templateMode = TemplateMode::Cp; - } - - $data = [ - 'html' => template('users/_photo', [ - 'user' => $user, - ], templateMode: $templateMode), - 'photoId' => $user->photoId, - ]; - - if ($user->getIsCurrent() && $request->isCpRequest()) { - $data['headerPhotoHtml'] = template('_layouts/components/header-photo'); - } - - return new JsonResponse($data); - } -} diff --git a/src/Http/Controllers/Users/SaveUserController.php b/src/Http/Controllers/Users/SaveUserController.php index 863d53f7d76..d7c862443b1 100644 --- a/src/Http/Controllers/Users/SaveUserController.php +++ b/src/Http/Controllers/Users/SaveUserController.php @@ -5,6 +5,7 @@ namespace CraftCms\Cms\Http\Controllers\Users; use CraftCms\Cms\Asset\AssetsHelper; +use CraftCms\Cms\Asset\Elements\Asset; use CraftCms\Cms\Auth\AuthMethods; use CraftCms\Cms\Auth\Concerns\ConfirmsPasswords; use CraftCms\Cms\Config\GeneralConfig; @@ -250,11 +251,17 @@ public function __invoke(Request $request): Response // --------------------------------------------------------------------- $photo = $request->file('photo'); + $photoIdSubmitted = $request->exists('photoId'); + $directPhotoSubmitted = $this->hasDirectPhotoPayload($request); if ($photo && ! ImageHelper::canManipulateAsImage($photo->extension())) { $user->errors()->add('photo', t('The user photo provided is not an image.')); } + if ($photoIdSubmitted && ! $directPhotoSubmitted) { + $this->validatePostedPhotoId($request, $user); + } + // Don't validate required custom fields if it's public registration if (! $isPublicRegistration || ($userSettings['validateOnPublicRegistration'] ?? false)) { $user->ruleset->useScenario(ElementRules::SCENARIO_LIVE); @@ -304,6 +311,10 @@ public function __invoke(Request $request): Response // Save the user’s photo, if it was submitted $this->processUserPhoto($request, app(Elements::class), $user); + if ($photoIdSubmitted && ! $directPhotoSubmitted) { + $this->processPostedPhotoId($request, app(Elements::class), $user); + } + // If this is public registration, assign the user to the default user group if (Edition::isAtLeast(Edition::Pro) && $isPublicRegistration) { // Assign them to the default user group @@ -425,6 +436,54 @@ private function processUserPhoto(Request $request, Elements $elements, User $us } } + private function hasDirectPhotoPayload(Request $request): bool + { + if ($request->boolean('deletePhoto')) { + return true; + } + if ($request->hasFile('photo')) { + return true; + } + + return is_array($request->input('photo')); + } + + private function validatePostedPhotoId(Request $request, User $user): void + { + $photoId = $this->postedPhotoId($request); + + if ($photoId === null) { + return; + } + + if (! Asset::find()->id($photoId)->kind('image')->one()) { + $user->errors()->add('photoId', t('The user photo provided is not an image.')); + } + } + + private function processPostedPhotoId(Request $request, Elements $elements, User $user): void + { + $photoId = $this->postedPhotoId($request); + $user->setPhoto($photoId ? Asset::find()->id($photoId)->one() : null); + + $elements->saveElement($user, false); + } + + private function postedPhotoId(Request $request): ?int + { + $photoId = $request->input('photoId'); + + if (is_array($photoId)) { + $photoId = end($photoId); + } + + if ($photoId === null || $photoId === '') { + return null; + } + + return (int) $photoId; + } + /** * Possibly log a user in right after they activated their account (not when they reset their password), * if Craft is configured to do so. diff --git a/src/User/Elements/User.php b/src/User/Elements/User.php index e286e184a5c..8ca5f7a8615 100644 --- a/src/User/Elements/User.php +++ b/src/User/Elements/User.php @@ -867,17 +867,15 @@ public function attributeLabels(): array return $labels; } - #[Override] - public function safeAttributes(): array - { - return Arr::except(parent::safeAttributes(), ['photoId']); - } - #[Override] public function setAttributesFromRequest($values): void { unset($values['unverifiedEmail']); + if (array_key_exists('photoId', $values)) { + $values['photoId'] = $this->normalizePhotoId($values['photoId']); + } + if (isset($values['email'])) { $values['email'] = trim($values['email']); if ($values['email'] === '' || $values['email'] === $this->email) { @@ -926,6 +924,19 @@ public function setAttributes($values): void parent::setAttributes($values); } + private function normalizePhotoId(mixed $photoId): ?int + { + if (is_array($photoId)) { + $photoId = end($photoId); + } + + if ($photoId === null || $photoId === '') { + return null; + } + + return (int) $photoId; + } + /** * Returns whether the user account can be logged into. */ @@ -2010,11 +2021,6 @@ public function afterSave(bool $isNew): void $model->save(); - // Make sure that the photo is located in the right place - if (! $isNew && $this->photoId) { - Users::relocateUserPhoto($this); - } - $this->setDirtyAttributes($dirtyAttributes); parent::afterSave($isNew); diff --git a/tests/Feature/Http/Controllers/Assets/UploadControllerTest.php b/tests/Feature/Http/Controllers/Assets/UploadControllerTest.php index e7665ebab73..af0b0435ee1 100644 --- a/tests/Feature/Http/Controllers/Assets/UploadControllerTest.php +++ b/tests/Feature/Http/Controllers/Assets/UploadControllerTest.php @@ -6,6 +6,7 @@ use CraftCms\Cms\Asset\Models\VolumeFolder as VolumeFolderModel; use CraftCms\Cms\Http\Controllers\Assets\UploadController; use CraftCms\Cms\User\Elements\User; +use Illuminate\Http\UploadedFile; use function Pest\Laravel\actingAs; use function Pest\Laravel\postJson; @@ -35,6 +36,15 @@ ])->assertStatus(400); }); +it('uploads to a posted folder id', function () { + postJson(action([UploadController::class, 'upload']), [ + 'folderId' => $this->folder->id, + 'assets-upload' => UploadedFile::fake()->image('avatar.jpg'), + ]) + ->assertOk() + ->assertJsonPath('assetId', fn ($assetId) => is_int($assetId)); +}); + it('requires authentication for replace file', function () { auth()->logout(); diff --git a/tests/Feature/Http/Controllers/Elements/UpdateFieldLayoutControllerTest.php b/tests/Feature/Http/Controllers/Elements/UpdateFieldLayoutControllerTest.php index 364119de5f2..21fc2daa91f 100644 --- a/tests/Feature/Http/Controllers/Elements/UpdateFieldLayoutControllerTest.php +++ b/tests/Feature/Http/Controllers/Elements/UpdateFieldLayoutControllerTest.php @@ -2,6 +2,9 @@ declare(strict_types=1); +use CraftCms\Cms\Asset\Models\Asset as AssetModel; +use CraftCms\Cms\Asset\Models\Volume; +use CraftCms\Cms\Asset\Models\VolumeFolder as VolumeFolderModel; use CraftCms\Cms\Element\Revisions; use CraftCms\Cms\Entry\Elements\Entry; use CraftCms\Cms\Entry\Models\Entry as EntryModel; @@ -131,3 +134,29 @@ ->etc() ); }); + +it('normalizes user photoId arrays from element select inputs', function () { + config()->set('filesystems.disks.update-field-layout-test', [ + 'driver' => 'local', + 'root' => storage_path('framework/testing/update-field-layout-test'), + ]); + + $user = User::findOne(); + $volume = Volume::factory()->create(['fs' => 'disk:update-field-layout-test']); + $folder = VolumeFolderModel::factory()->create(['volumeId' => $volume->id]); + $asset = AssetModel::factory()->createElement([ + 'volumeId' => $volume->id, + 'folderId' => $folder->id, + 'kind' => 'image', + 'filename' => 'avatar.jpg', + ]); + + postJson(action(UpdateFieldLayoutController::class), [ + 'elementType' => User::class, + 'elementId' => $user->id, + 'siteId' => $user->siteId, + 'photoId' => [$asset->id], + ]) + ->assertOk() + ->assertJsonPath('element.photoId', $asset->id); +}); diff --git a/tests/Feature/Http/Controllers/User/PhotoControllerTest.php b/tests/Feature/Http/Controllers/User/PhotoControllerTest.php deleted file mode 100644 index 546f01aa049..00000000000 --- a/tests/Feature/Http/Controllers/User/PhotoControllerTest.php +++ /dev/null @@ -1,38 +0,0 @@ -logout(); - - postJson(action([PhotoController::class, 'renderInput']))->assertUnauthorized(); - postJson(action([PhotoController::class, 'upload']))->assertUnauthorized(); - postJson(action([PhotoController::class, 'destroy']))->assertUnauthorized(); -}); - -test('userId is required', function () { - postJson(action([PhotoController::class, 'renderInput']))->assertJsonValidationErrorFor('userId'); - postJson(action([PhotoController::class, 'upload']))->assertJsonValidationErrorFor('userId'); - postJson(action([PhotoController::class, 'destroy']))->assertJsonValidationErrorFor('userId'); -}); - -test('renderInput', function () { - postJson(action([PhotoController::class, 'renderInput'], [ - 'userId' => auth()->id(), - ]))->assertJsonStructure([ - 'html', - 'photoId', - 'headerPhotoHtml', - ]); -}); - -test('upload', function () {})->todo('Add when volumes have been ported'); -test('destroy', function () {})->todo('Add when volumes have been ported'); diff --git a/tests/Feature/Http/Controllers/User/SaveUserController/UserEditTest.php b/tests/Feature/Http/Controllers/User/SaveUserController/UserEditTest.php index 9887c0afecc..b87cf99a341 100644 --- a/tests/Feature/Http/Controllers/User/SaveUserController/UserEditTest.php +++ b/tests/Feature/Http/Controllers/User/SaveUserController/UserEditTest.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use CraftCms\Cms\Asset\Elements\Asset; +use CraftCms\Cms\Asset\Models\Asset as AssetModel; use CraftCms\Cms\Asset\Models\Volume; use CraftCms\Cms\Database\Factories\UserFactory; use CraftCms\Cms\Filesystem\Filesystems\Local; @@ -178,6 +180,92 @@ expect($updatedUser->getPhoto())->not->toBeNull(); }); +it('can select an existing image asset as another users photo', function () { + $admin = UserFactory::new()->admin()->createElement(); + $targetUser = UserFactory::new()->createElement(); + $asset = AssetModel::factory()->createElement([ + 'kind' => 'image', + 'filename' => 'avatar.jpg', + ]); + + actingAs($admin)->post(action(SaveUserController::class), [ + 'userId' => $targetUser->id, + 'photoId' => $asset->id, + ]) + ->assertRedirect() + ->assertSessionHasNoErrors(); + + $updatedUser = User::find()->id($targetUser->id)->one(); + expect($updatedUser)->not->toBeNull(); + expect($updatedUser->photoId)->toBe($asset->id); +}); + +it('can clear another users selected photo without deleting the asset', function () { + $admin = UserFactory::new()->admin()->createElement(); + $asset = AssetModel::factory()->createElement([ + 'kind' => 'image', + 'filename' => 'avatar.jpg', + ]); + $targetUser = UserFactory::new()->createElement([ + 'photoId' => $asset->id, + ]); + + actingAs($admin)->post(action(SaveUserController::class), [ + 'userId' => $targetUser->id, + 'photoId' => '', + ]) + ->assertRedirect() + ->assertSessionHasNoErrors(); + + $updatedUser = User::find()->id($targetUser->id)->one(); + expect($updatedUser)->not->toBeNull(); + expect($updatedUser->photoId)->toBeNull(); + expect(Asset::find()->id($asset->id)->status(null)->one())->not->toBeNull(); +}); + +it('preserves another users selected photo when photoId is not submitted', function () { + $admin = UserFactory::new()->admin()->createElement(); + $asset = AssetModel::factory()->createElement([ + 'kind' => 'image', + 'filename' => 'avatar.jpg', + ]); + $targetUser = UserFactory::new()->createElement([ + 'photoId' => $asset->id, + ]); + + actingAs($admin)->post(action(SaveUserController::class), [ + 'userId' => $targetUser->id, + 'firstName' => 'Updated', + ]) + ->assertRedirect() + ->assertSessionHasNoErrors(); + + $updatedUser = User::find()->id($targetUser->id)->one(); + expect($updatedUser)->not->toBeNull(); + expect($updatedUser->photoId)->toBe($asset->id); +}); + +it('rejects non-image assets selected as another users photo', function () { + $admin = UserFactory::new()->admin()->createElement(); + $targetUser = UserFactory::new()->createElement(); + $asset = AssetModel::factory()->createElement([ + 'kind' => 'pdf', + 'filename' => 'document.pdf', + ]); + + $response = actingAs($admin)->postJson(action(SaveUserController::class), [ + 'userId' => $targetUser->id, + 'photoId' => $asset->id, + ]); + + $response->assertStatus(400); + $response->assertJsonValidationErrorFor('photoId'); + + $updatedUser = User::find()->id($targetUser->id)->one(); + expect($updatedUser)->not->toBeNull(); + expect($updatedUser->photoId)->toBeNull(); +}); + it('returns proper error message on validation failure when editing another user', function () { $admin = UserFactory::new()->admin()->createElement(); $targetUser = UserFactory::new()->createElement(); diff --git a/tests/Feature/Http/Controllers/User/UsersControllerTest.php b/tests/Feature/Http/Controllers/User/UsersControllerTest.php index be0cd984f88..f31fd44099d 100644 --- a/tests/Feature/Http/Controllers/User/UsersControllerTest.php +++ b/tests/Feature/Http/Controllers/User/UsersControllerTest.php @@ -1,9 +1,14 @@ User::findOne()->id]))->assertOk(); }); + + test('edit renders user photo asset picker when a user photo volume is configured', function () { + ProjectConfig::set('fs.test', [ + 'hasUrls' => true, + 'name' => 'Test', + 'settings' => [ + 'path' => public_path('test'), + ], + 'type' => Local::class, + 'url' => '/test', + ]); + + $volume = Volume::factory()->create([ + 'fs' => 'test', + ]); + + ProjectConfig::set('users.photoVolumeUid', $volume->uid); + $folder = Folders::ensureFolderByFullPathAndVolume('', Volumes::getVolumeByUid($volume->uid)); + + get(action([UsersController::class, 'edit'])) + ->assertOk() + ->assertSee('Craft.AssetSelectInput', false) + ->assertSee('photoId', false) + ->assertSee("folderId: $folder->id", false) + ->assertSee('"siteId":'.User::findOne()->siteId, false); + }); }); describe('destroy', function () {