New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UserTemplates image saving too early #2630

Open
vanMeerdervoort opened this Issue Sep 4, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@vanMeerdervoort

vanMeerdervoort commented Sep 4, 2018

  • Non critical bug

Problem description

When editing a UserTemplate on a page, a newly uploaded image immediately replaces the old image. When the user leaves the page without saving his changes, the old image doesn't get restored. This results in missing images and orphaned images in the Frontend Files directory

Steps to reproduce

In admin:
Add a UserTemplate to a page. Save. Edit the UserTemplate. Upload a new image. DO NOT SAVE. Leave the page without saving. Go back to the page and edit the UserTemplate again.

Expected behavior / Proposed solutions

Save a replacement image to a temp directory first, and only replace the original image when the actual save button is pressed.

@carakas

This comment has been minimized.

Show comment
Hide comment
@carakas

carakas Sep 4, 2018

Member

Hello,

Thank you for reporting this issue.
Would you be able to try and create a pull request for this?
If you have any questions you can always ask them here or in our slack channel

Kind regards
Jelmer

Member

carakas commented Sep 4, 2018

Hello,

Thank you for reporting this issue.
Would you be able to try and create a pull request for this?
If you have any questions you can always ask them here or in our slack channel

Kind regards
Jelmer

@vanMeerdervoort

This comment has been minimized.

Show comment
Hide comment
@vanMeerdervoort

vanMeerdervoort Sep 4, 2018

Hi Jelmer,

I currently don't have time for a pull request, but while I was implementing the UserTemplate parser in a custom module, this is what I came up with.

in the addCustomFieldInPlaceholderFor function of the js file

```

	
	// replace image
    if ($element.is('[data-ft-type="image"]')) {
        $placeholder.append(
            jsBackend.content.getImageFieldHtml(
                $element.attr('src'),
                $element.attr('alt'),
                $element.data('ft-label'),
                $element.attr('style') !== 'display: none;',
                $element.data('ft-block-optional'),
                key
            )
        )

        // attach an ajax uploader to the field
        jsBackend.content.uploaders.push(new ss.SimpleUpload({
            button: 'ajax-upload-' + key,
            url: '/backend/ajax?fork[module]=Content&fork[action]=UploadFile&type=UserTemplate',
            name: 'file',
            accept: 'image/*',
            responseType: 'json',
            onComplete: function (filename, response) {
                if (!response) {
                    window.alert(filename + 'upload failed')
                    return false
                }

                $('#user-template-image-' + key + ' img').attr(
                    'src',
                    '/src/Backend/Cache/Content/UserTemplate/' + response.data
                )
            }
        }))

        // handle the "show image" checkbox
        $('#user-template-image-' + key + ' input[type=checkbox]').on('click', function (e) {
            $('#user-template-image-' + key + ' img').toggle($(this).is(':checked'))
        })

        return
    }

    // replace image background
    if ($element.is('[data-ft-type="image-background"]')) {
        $placeholder.append(
            jsBackend.content.getImageBackgroundFieldHtml(
                $element.attr('data-src'),
                $element.data('ft-label'),
                key
            )
        )

        // attach an ajax uploader to the field
        jsBackend.content.uploaders.push(new ss.SimpleUpload({
            button: 'ajax-upload-' + key,
            url: '/backend/ajax?fork[module]=Content&fork[action]=UploadFile&type=UserTemplate',
            name: 'file',
            accept: 'image/*',
            responseType: 'json',
            onComplete: function (filename, response) {
                if (!response) {
                    window.alert(filename + 'upload failed')
                    return false
                }

                $('#user-template-image-background-' + key + ' img').attr(
                    'src',
                    '/src/Backend/Cache/Content/UserTemplate/' + response.data
                )
            }
        }))

        return
    }
	
	
	
	
```

// In the saveCustomField function:

```	
	
	if ($element.is('[data-ft-type="image"]')) {
        $img = $placeholder.find('#user-template-image-' + key + ' img')
        var alt = $placeholder.find('#alt' + key).val()
        var $visible = $placeholder.find('#user-template-image-' + key + ' input[type=checkbox]')


        var replace =  $img.attr('src');
        var cache = "/Cache/"

        if(replace.indexOf(cache) != -1){

            console.log("image in cache - moving to frontend")

            var oldImage = $element.attr('src');

            $.ajax({
                data: {
                    fork: {module: 'Content', action: 'SwitchFiles'},
                    old: oldImage,
                    new: replace,
                    type: 'UserTemplate'
                }
            })

            var newSrc = '/src/Frontend/Files/'
            var oldSrc = '/src/Backend/Cache/'

            replace = replace.replace(oldSrc,newSrc)


        }



        $element.attr('src',replace)

        // go on


        $element.attr('alt', alt)
        if ($visible.is(':checked')) {
            $element.attr('style', 'display: block;')
        } else {
            $element.attr('style', 'display: none;')
        }

        return
    }

    if ($element.is('[data-ft-type="image-background"]')) {
        $img = $placeholder.find('#user-template-image-background-' + key + ' img')

        var replace =  $img.attr('src');
        var cache = "/Cache/"

        if(replace.indexOf(cache) != -1){

            console.log("image in cache - moving to frontend")

            var oldImage = $element.attr('src');

            $.ajax({
                data: {
                    fork: {module: 'Content', action: 'SwitchFiles'},
                    old: oldImage,
                    new: replace,
                    type: 'UserTemplate'
                }
            })

            var newSrc = '/src/Frontend/Files/'
            var oldSrc = '/src/Backend/Cache/'

            replace = replace.replace(oldSrc,newSrc)


        }


        $element.css('background-image', 'url(' + replace + ')')
        $element.attr('data-src',replace);

        return
    }
```		

Add a file to the Ajax actions called SwitchFiles.php

```

	
	<?php

	namespace Backend\Modules\Content\Ajax;

	use Symfony\Component\Filesystem\Filesystem;
	use Backend\Pages\Engine\Base\AjaxAction;
	use Symfony\Component\HttpFoundation\Response;

	/**
	 * This action will move a file from the cache dir to the frontend files dir, and remove the original file.
	 */
	class SwitchFiles extends AjaxAction
	{
		public function execute(): void
		{
			$request = $this->getRequest();
			if (!$request->request->has('type') || !$request->request->has('new') ) {
				$this->output(Response::HTTP_BAD_REQUEST, 'Missing data');

				return;
			}
			if($request->request->has('old')){

				//delete old file
				$file = pathinfo($this->getRequest()->request->get('old'), PATHINFO_BASENAME);
				$directory = $this->getRequest()->request->get('type');

				$path = FRONTEND_FILES_PATH . '/Pages/' . $directory . '/' . $file;

				$filesystem = new Filesystem();
				if ($filesystem->exists($path)) {
					$filesystem->remove($path);
				}else{
					$this->output(Response::HTTP_BAD_REQUEST, 'Cant find old file');
					return;
				}

			}
			//move new file
			$file = pathinfo($this->getRequest()->request->get('new'), PATHINFO_BASENAME);
			$directory = $this->getRequest()->request->get('type');

			$path = BACKEND_CACHE_PATH . '/Pages/' . $directory . '/' . $file;

			$newPath =  FRONTEND_FILES_PATH . '/Pages/' . $directory . '/' . $file;

			$filesystem = new Filesystem();
			if ($filesystem->exists($path)) {
				$filesystem->copy($path,$newPath);
				$filesystem->remove($path);
			}else{
				$this->output(Response::HTTP_BAD_REQUEST, 'error  moving file');
				return;
			}

			$this->output(Response::HTTP_OK);
		}
	}

```

Update the UploadFile.php this also fixes a bug that deletes a file when it has the same name as a file of another UserTemplate item: gives the files unique names.

	```

	<?php

	namespace Backend\Modules\Pages\Ajax;

	use Symfony\Component\HttpFoundation\Request;
	use Symfony\Component\Filesystem\Filesystem;
	use Common\Core\Model;
	use Common\Uri;
	use Backend\Core\Engine\Base\AjaxAction;
	use Backend\Core\Engine\Exception;
	use Symfony\Component\HttpFoundation\Response;

	/**
	 * This action will enable you to upload files trough ajax.
	 * It accepts both XmlHttp requests or file uploads using a form.
	 *
	 * Note that it only accepts single file uploads.
	 * The type $_GET parameter will be used to determine which folder to upload in.
	 */
	class UploadFile extends AjaxAction
	{
		public function execute(): void
		{
			$request = $this->getRequest();

			$fileName = $this->writeFile(
				$this->getFileContentFromRequest($request),
				$this->getFileNameFromRequest($request),
				$request->get('type')
			);

			$this->output(Response::HTTP_OK, $fileName);
		}

		/**
		 * Extracts the uploaded file from a request. It handles both XmlHttpRequest
		 * uploads and form uploads (with files in the $_FILES global)
		 *
		 * @param Request $request
		 *
		 * @throws Exception When no file could be extracted
		 *
		 * @return string The content of the uploaded file
		 */
		private function getFileContentFromRequest(Request $request): string
		{
			// ajax uploaders fallback to submitting a form with the file in the fields.
			$uploadedFiles = $request->files->all();
			if (count($uploadedFiles) === 1) {
				$file = array_values($uploadedFiles)[0];

				return file_get_contents($file->getPathname());
			}

			throw new Exception('The request doesn\'t contain one file.');
		}

		/**
		 * Extracts the uploaded file name from a request. It handles both XmlHttpRequest
		 * uploads and form uploads (with files in the $_FILES global)
		 *
		 * @param Request $request
		 *
		 * @throws Exception When no file could be extracted
		 *
		 * @return string The content of the uploaded file
		 */
		private function getFileNameFromRequest(Request $request): string
		{
			// ajax uploaders fallback to submitting a form with the file in the fields.
			$uploadedFiles = $request->files->all();
			if (count($uploadedFiles) === 1) {
				$file = array_values($uploadedFiles)[0];

				return $file->getClientOriginalName();
			}

			throw new Exception('The request doesn\'t contain one file.');
		}

		/**
		 * Writes some content to a file in a given folder
		 *
		 * @param string $content
		 * @param string $fileName
		 * @param string $destinationFolder
		 *
		 * @return string The filename of the written file.
		 */
		private function writeFile(string $content, string $fileName, string $destinationFolder): string
		{
		   // $path = FRONTEND_FILES_PATH . '/Pages/' . $destinationFolder;

			$path = BACKEND_CACHE_PATH . '/Pages/' . $destinationFolder;

			// create the needed folder if it doesn't exist
			$filesystem = new Filesystem();
			if (!$filesystem->exists($path)) {
				$filesystem->mkdir($path);
			}

			$uniqueId = time();

			// convert the filename to url friendly version
			$baseName = Uri::getUrl(pathinfo($fileName, PATHINFO_FILENAME));
			$extension = pathinfo($fileName, PATHINFO_EXTENSION);
			$fileName = $baseName.'-'.$uniqueId. '.' . $extension;

			// generate a non-existing filename
			while ($filesystem->exists($path . '/' . $fileName)) {
				$baseName = Model::addNumber($baseName);
				$fileName = $baseName . '.' . $extension;
			}

			// save the content of the file
			$filesystem->dumpFile(
				$path . '/' . $fileName,
				$content
			);

			return $fileName;
		}
	}
```

Hope this helps!
Cheers, Vincent

vanMeerdervoort commented Sep 4, 2018

Hi Jelmer,

I currently don't have time for a pull request, but while I was implementing the UserTemplate parser in a custom module, this is what I came up with.

in the addCustomFieldInPlaceholderFor function of the js file

```

	
	// replace image
    if ($element.is('[data-ft-type="image"]')) {
        $placeholder.append(
            jsBackend.content.getImageFieldHtml(
                $element.attr('src'),
                $element.attr('alt'),
                $element.data('ft-label'),
                $element.attr('style') !== 'display: none;',
                $element.data('ft-block-optional'),
                key
            )
        )

        // attach an ajax uploader to the field
        jsBackend.content.uploaders.push(new ss.SimpleUpload({
            button: 'ajax-upload-' + key,
            url: '/backend/ajax?fork[module]=Content&fork[action]=UploadFile&type=UserTemplate',
            name: 'file',
            accept: 'image/*',
            responseType: 'json',
            onComplete: function (filename, response) {
                if (!response) {
                    window.alert(filename + 'upload failed')
                    return false
                }

                $('#user-template-image-' + key + ' img').attr(
                    'src',
                    '/src/Backend/Cache/Content/UserTemplate/' + response.data
                )
            }
        }))

        // handle the "show image" checkbox
        $('#user-template-image-' + key + ' input[type=checkbox]').on('click', function (e) {
            $('#user-template-image-' + key + ' img').toggle($(this).is(':checked'))
        })

        return
    }

    // replace image background
    if ($element.is('[data-ft-type="image-background"]')) {
        $placeholder.append(
            jsBackend.content.getImageBackgroundFieldHtml(
                $element.attr('data-src'),
                $element.data('ft-label'),
                key
            )
        )

        // attach an ajax uploader to the field
        jsBackend.content.uploaders.push(new ss.SimpleUpload({
            button: 'ajax-upload-' + key,
            url: '/backend/ajax?fork[module]=Content&fork[action]=UploadFile&type=UserTemplate',
            name: 'file',
            accept: 'image/*',
            responseType: 'json',
            onComplete: function (filename, response) {
                if (!response) {
                    window.alert(filename + 'upload failed')
                    return false
                }

                $('#user-template-image-background-' + key + ' img').attr(
                    'src',
                    '/src/Backend/Cache/Content/UserTemplate/' + response.data
                )
            }
        }))

        return
    }
	
	
	
	
```

// In the saveCustomField function:

```	
	
	if ($element.is('[data-ft-type="image"]')) {
        $img = $placeholder.find('#user-template-image-' + key + ' img')
        var alt = $placeholder.find('#alt' + key).val()
        var $visible = $placeholder.find('#user-template-image-' + key + ' input[type=checkbox]')


        var replace =  $img.attr('src');
        var cache = "/Cache/"

        if(replace.indexOf(cache) != -1){

            console.log("image in cache - moving to frontend")

            var oldImage = $element.attr('src');

            $.ajax({
                data: {
                    fork: {module: 'Content', action: 'SwitchFiles'},
                    old: oldImage,
                    new: replace,
                    type: 'UserTemplate'
                }
            })

            var newSrc = '/src/Frontend/Files/'
            var oldSrc = '/src/Backend/Cache/'

            replace = replace.replace(oldSrc,newSrc)


        }



        $element.attr('src',replace)

        // go on


        $element.attr('alt', alt)
        if ($visible.is(':checked')) {
            $element.attr('style', 'display: block;')
        } else {
            $element.attr('style', 'display: none;')
        }

        return
    }

    if ($element.is('[data-ft-type="image-background"]')) {
        $img = $placeholder.find('#user-template-image-background-' + key + ' img')

        var replace =  $img.attr('src');
        var cache = "/Cache/"

        if(replace.indexOf(cache) != -1){

            console.log("image in cache - moving to frontend")

            var oldImage = $element.attr('src');

            $.ajax({
                data: {
                    fork: {module: 'Content', action: 'SwitchFiles'},
                    old: oldImage,
                    new: replace,
                    type: 'UserTemplate'
                }
            })

            var newSrc = '/src/Frontend/Files/'
            var oldSrc = '/src/Backend/Cache/'

            replace = replace.replace(oldSrc,newSrc)


        }


        $element.css('background-image', 'url(' + replace + ')')
        $element.attr('data-src',replace);

        return
    }
```		

Add a file to the Ajax actions called SwitchFiles.php

```

	
	<?php

	namespace Backend\Modules\Content\Ajax;

	use Symfony\Component\Filesystem\Filesystem;
	use Backend\Pages\Engine\Base\AjaxAction;
	use Symfony\Component\HttpFoundation\Response;

	/**
	 * This action will move a file from the cache dir to the frontend files dir, and remove the original file.
	 */
	class SwitchFiles extends AjaxAction
	{
		public function execute(): void
		{
			$request = $this->getRequest();
			if (!$request->request->has('type') || !$request->request->has('new') ) {
				$this->output(Response::HTTP_BAD_REQUEST, 'Missing data');

				return;
			}
			if($request->request->has('old')){

				//delete old file
				$file = pathinfo($this->getRequest()->request->get('old'), PATHINFO_BASENAME);
				$directory = $this->getRequest()->request->get('type');

				$path = FRONTEND_FILES_PATH . '/Pages/' . $directory . '/' . $file;

				$filesystem = new Filesystem();
				if ($filesystem->exists($path)) {
					$filesystem->remove($path);
				}else{
					$this->output(Response::HTTP_BAD_REQUEST, 'Cant find old file');
					return;
				}

			}
			//move new file
			$file = pathinfo($this->getRequest()->request->get('new'), PATHINFO_BASENAME);
			$directory = $this->getRequest()->request->get('type');

			$path = BACKEND_CACHE_PATH . '/Pages/' . $directory . '/' . $file;

			$newPath =  FRONTEND_FILES_PATH . '/Pages/' . $directory . '/' . $file;

			$filesystem = new Filesystem();
			if ($filesystem->exists($path)) {
				$filesystem->copy($path,$newPath);
				$filesystem->remove($path);
			}else{
				$this->output(Response::HTTP_BAD_REQUEST, 'error  moving file');
				return;
			}

			$this->output(Response::HTTP_OK);
		}
	}

```

Update the UploadFile.php this also fixes a bug that deletes a file when it has the same name as a file of another UserTemplate item: gives the files unique names.

	```

	<?php

	namespace Backend\Modules\Pages\Ajax;

	use Symfony\Component\HttpFoundation\Request;
	use Symfony\Component\Filesystem\Filesystem;
	use Common\Core\Model;
	use Common\Uri;
	use Backend\Core\Engine\Base\AjaxAction;
	use Backend\Core\Engine\Exception;
	use Symfony\Component\HttpFoundation\Response;

	/**
	 * This action will enable you to upload files trough ajax.
	 * It accepts both XmlHttp requests or file uploads using a form.
	 *
	 * Note that it only accepts single file uploads.
	 * The type $_GET parameter will be used to determine which folder to upload in.
	 */
	class UploadFile extends AjaxAction
	{
		public function execute(): void
		{
			$request = $this->getRequest();

			$fileName = $this->writeFile(
				$this->getFileContentFromRequest($request),
				$this->getFileNameFromRequest($request),
				$request->get('type')
			);

			$this->output(Response::HTTP_OK, $fileName);
		}

		/**
		 * Extracts the uploaded file from a request. It handles both XmlHttpRequest
		 * uploads and form uploads (with files in the $_FILES global)
		 *
		 * @param Request $request
		 *
		 * @throws Exception When no file could be extracted
		 *
		 * @return string The content of the uploaded file
		 */
		private function getFileContentFromRequest(Request $request): string
		{
			// ajax uploaders fallback to submitting a form with the file in the fields.
			$uploadedFiles = $request->files->all();
			if (count($uploadedFiles) === 1) {
				$file = array_values($uploadedFiles)[0];

				return file_get_contents($file->getPathname());
			}

			throw new Exception('The request doesn\'t contain one file.');
		}

		/**
		 * Extracts the uploaded file name from a request. It handles both XmlHttpRequest
		 * uploads and form uploads (with files in the $_FILES global)
		 *
		 * @param Request $request
		 *
		 * @throws Exception When no file could be extracted
		 *
		 * @return string The content of the uploaded file
		 */
		private function getFileNameFromRequest(Request $request): string
		{
			// ajax uploaders fallback to submitting a form with the file in the fields.
			$uploadedFiles = $request->files->all();
			if (count($uploadedFiles) === 1) {
				$file = array_values($uploadedFiles)[0];

				return $file->getClientOriginalName();
			}

			throw new Exception('The request doesn\'t contain one file.');
		}

		/**
		 * Writes some content to a file in a given folder
		 *
		 * @param string $content
		 * @param string $fileName
		 * @param string $destinationFolder
		 *
		 * @return string The filename of the written file.
		 */
		private function writeFile(string $content, string $fileName, string $destinationFolder): string
		{
		   // $path = FRONTEND_FILES_PATH . '/Pages/' . $destinationFolder;

			$path = BACKEND_CACHE_PATH . '/Pages/' . $destinationFolder;

			// create the needed folder if it doesn't exist
			$filesystem = new Filesystem();
			if (!$filesystem->exists($path)) {
				$filesystem->mkdir($path);
			}

			$uniqueId = time();

			// convert the filename to url friendly version
			$baseName = Uri::getUrl(pathinfo($fileName, PATHINFO_FILENAME));
			$extension = pathinfo($fileName, PATHINFO_EXTENSION);
			$fileName = $baseName.'-'.$uniqueId. '.' . $extension;

			// generate a non-existing filename
			while ($filesystem->exists($path . '/' . $fileName)) {
				$baseName = Model::addNumber($baseName);
				$fileName = $baseName . '.' . $extension;
			}

			// save the content of the file
			$filesystem->dumpFile(
				$path . '/' . $fileName,
				$content
			);

			return $fileName;
		}
	}
```

Hope this helps!
Cheers, Vincent

@carakas

This comment has been minimized.

Show comment
Hide comment
@carakas

carakas Sep 5, 2018

Member

Thanks

Member

carakas commented Sep 5, 2018

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment