Skip to content
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

Moved uploaded files doesn't retain new filename. #839

Closed
JakeAi opened this issue Nov 20, 2017 · 3 comments
Closed

Moved uploaded files doesn't retain new filename. #839

JakeAi opened this issue Nov 20, 2017 · 3 comments

Comments

@JakeAi
Copy link
Contributor

JakeAi commented Nov 20, 2017

Once an uploaded file has moved and has an integer appended on the end for allowing multiple of the same files... the file object does not retain the new filename. In the function move() it calls getDestination() which can create a new filename and return a new path to move to. But the new filename does not get saved, it is used to move the file, and then gone. @lonnieezell I don't really know how you'd want to address this, other than reparsing the new path from getDestination... Just seems redundant.

if (!$this->file->move( $path, $this->file->getName(), true )) {
	throw new \ErrorException( "An error occured uploading your file." );
}

$this->file->getName(); // fails

Also I think this ternary operation needs flippe.

/**
 * Returns the destination path for the move operation where overwriting is not expected.
 *
 * First, it checks whether the delimiter is present in the filename, if it is, then it checks whether the
 * last element is an integer as there may be cases that the delimiter may be present in the filename.
 * For the all other cases, it appends an integer starting from zero before the file's extension.
 */

$destination = $overwrite ? $this->getDestination($targetPath . $name) : $targetPath . $name;
//$destination = true ? true : false;
@JakeAi JakeAi changed the title Moved uploaded files don't retain new filename. Moved uploaded files doesn't retain new filename. Nov 20, 2017
@JakeAi
Copy link
Contributor Author

JakeAi commented Nov 20, 2017

We could change from this

	public function move(string $targetPath, string $name = null, bool $overwrite = false)
	{
		.......................
		$destination = $overwrite ? $this->getDestination($targetPath . $name) : $targetPath . $name;
		.......................
		// Success, so store our new information
		$this->path = $targetPath;
		$this->name = $name;
		$this->hasMoved = true;

		return true;
	}

to this

	public function move(string $targetPath, string $name = null, bool $overwrite = false)
	{
		.......................
		//Flip ternary operation
		$destination = !$overwrite ? $this->getDestination($targetPath . $name) : $targetPath . $name;
		.......................
		//**Reparse the new file from the new destination, works on my end**
		$newFile    = pathinfo( $destination );
		$this->path = $newFile['dirname'];
		$this->name = $newFile['basename'];
		$this->hasMoved = true;

		return true;
	}

@lonnieezell
Copy link
Member

For file name we do need to save the name after determining the destination since the docs state:

If the file has been moved, this will return the final name of the moved file

I would update the filename with basename() from the destination variable we get so that it can be saved back to the class. Should take care of it.

As for overwriting and flipping the ternary: it would have been nice if you would have given the logic for why you wanted it flipped :) Took me a second, but yes, you're correct that should be modified otherwise overwriting won't actually work.

@JakeAi
Copy link
Contributor Author

JakeAi commented Nov 20, 2017

After more testing I did read the docs and saw that it returns a new instance of the file. So I ended up not digging into a fix since I am still developing. As for the ternary, I assumed since I was looking at it for an hour that other people would understand xD. But you're right, I didn't document the logic well at all.

Edit: my proposed fix above is completely useless and should be ignored

lonnieezell added a commit that referenced this issue Nov 29, 2017
Fix Issue #839 - Moved uploaded files doesn't retain new filename or overwrite
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants