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

[3.5] Add feature to fetch placeholder images from remote URLs #7368

Merged
merged 1 commit into from Mar 14, 2018

Conversation

Projects
None yet
2 participants
@bobdenotter
Member

bobdenotter commented Mar 8, 2018

Tired of the same random placeholder images? Too few kittens? This PR will allow you to add new placeholder images to your contenttypes, when using the "add loripsum content":

screen shot 2018-03-08 at 19 27 36

    fields:
        title:
            type: text
            class: large
            group: content
        …
        image:
            type: image
            group: media
            placeholder: https://source.unsplash.com/1600x900/?nature,water/__random__

or https://source.unsplash.com/1600x900/?kitten/__random__ for more kittens.

Should be trivial to make work with other placeholder services, like pexels, visualhunt or placekitten. Or we could even write our own, if we don't want to be reliant on a 3rd party service.

@bobdenotter bobdenotter changed the title from Add feature to fetch placeholder images from remote URLs to [3.5] Add feature to fetch placeholder images from remote URLs Mar 8, 2018

$filename = sprintf('placeholder_%s.jpg', substr(md5(microtime()), 0, 12));
$this->filesystem->write('files://' . $filename, $image);
return [$pathKey => $filename, 'title' => 'placeholder', 'alt' => 'placeholder'];
} catch (\Exception $e) {

This comment has been minimized.

@GawainLynch

GawainLynch Mar 9, 2018

Contributor

This is not a good idea, you don't want to catch everything silently, it makes it extremely hard to debug.

So …

  • getImage() can throw a GuzzleHttp\Exception\RequestException if there is a problem handling the call
  • write() will throw a Bolt\Filesystem\Exception\FileExistsException that can be avoided by calling put()

If other exceptions are being thrown, we want to know about them as something else is fundamentally wrong.

$image = $this->apiClient->getImage($placeholder);
$filename = sprintf('placeholder_%s.jpg', substr(md5(microtime()), 0, 12));
$this->filesystem->write('files://' . $filename, $image);
return [$pathKey => $filename, 'title' => 'placeholder', 'alt' => 'placeholder'];

This comment has been minimized.

@GawainLynch

GawainLynch Mar 9, 2018

Contributor

PSR-2 requires a newline before then return

public function getImage($url)
{
$url = str_replace('__random__', rand(100000, 999999), $url);
return $this->client->get($url, ['timeout' => 10])->getBody();

This comment has been minimized.

@GawainLynch

GawainLynch Mar 9, 2018

Contributor

PSR-2 requires a newline before then return

@@ -278,12 +278,15 @@ private function addText(Entity\Content $contentEntity, $fieldName, $type, array
*/
private function addJson(Entity\Content $contentEntity, $fieldName, $type)
{
$contentType = $this->repository->getEntityManager()->getContentType($contentEntity->getContenttype());
$placeholder = isset($contentType['fields'][$fieldName]['placeholder']) ? $contentType['fields'][$fieldName]['placeholder'] : null;
$value = null;

This comment has been minimized.

@GawainLynch

GawainLynch Mar 9, 2018

Contributor

While I am nagging, drop the newline before this one 😜

*
* @param string $url URL of the remote image to fetch
*
* @return StreamInterface

This comment has been minimized.

@GawainLynch

GawainLynch Mar 9, 2018

Contributor

Needs a use Psr\Http\Message\StreamInterface; added

This comment has been minimized.

@bobdenotter
@@ -453,9 +456,21 @@ private function getRandomTags($count = 5)
*
* @return array|null
*/
private function getRandomImage($type)
private function getRandomImage($type, $placeholder = null)

This comment has been minimized.

@GawainLynch

GawainLynch Mar 9, 2018

Contributor

$placeholder doesn't need a default, and does need a @param string|null $placeholder in the docblock

@GawainLynch GawainLynch merged commit 75126cc into 3.5 Mar 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@GawainLynch GawainLynch deleted the feature/random-images branch Mar 14, 2018

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