Hotfix/display 1019 feed errors #166
Conversation
There was a problem hiding this comment.
Looks good. However, we should use placeholders in log messages, cf. https://symfony.com/doc/5.4/logging.html#logging-a-message.
| } | ||
| } catch (\Throwable $throwable) { | ||
| // If the content does not exist anymore, unpublished the slide. | ||
| if ($throwable instanceof ClientException && 404 == $throwable->getCode()) { |
There was a problem hiding this comment.
| if ($throwable instanceof ClientException && 404 == $throwable->getCode()) { | |
| if ($throwable instanceof ClientException && Response::HTTP_NOT_FOUND == $throwable->getCode()) { |
(cf. https://github.com/symfony/symfony/blob/6.3/src/Symfony/Component/HttpFoundation/Response.php#L51)
| // If the content does not exist anymore, unpublished the slide. | ||
| if ($throwable instanceof ClientException && 404 == $throwable->getCode()) { | ||
| try { | ||
| $this->logger->info('Feed with id: '.$feed->getId().' depends on an item that does not exist in Event Database. Unpublished slide with id: '.$feed->getSlide()->getId()); |
There was a problem hiding this comment.
You should use placeholders in the log message: https://symfony.com/doc/5.4/logging.html#logging-a-message
And probably move the logging to after unpublishing the slide (cf. “Unpublished …” (past tense)).
| if ($throwable instanceof ClientException && 404 == $throwable->getCode()) { | ||
| try { | ||
| $this->logger->info('Feed with id: '.$feed->getId().' depends on an item that does not exist in Event Database. Unpublished slide with id: '.$feed->getSlide()->getId()); | ||
| $feed->getSlide()->setPublishedTo(new \DateTime('now', new \DateTimeZone('UTC'))); |
There was a problem hiding this comment.
Does this unpublish a slide? To the untrained eye, it seems to publish a slide.
There was a problem hiding this comment.
Added a comment.
A slide has publishedFrom, publishedTo. By setting publishedTo to now, the slide becomes unpublished.
There was a problem hiding this comment.
Ah, the “To” had escaped my untrained eye …
| $feed->getSlide()->setPublishedTo(new \DateTime('now', new \DateTimeZone('UTC'))); | ||
| $this->entityManager->flush(); | ||
| } catch (\Exception $exception) { | ||
| $this->logger->error($exception->getCode().': '.$exception->getMessage()); |
There was a problem hiding this comment.
Use message placeholders.
| $this->logger->error($exception->getCode().': '.$exception->getMessage()); | ||
| } | ||
| } else { | ||
| $this->logger->error($throwable->getCode().': '.$throwable->getMessage()); |
There was a problem hiding this comment.
Use message placeholders.
| return $result; | ||
| } | ||
| } catch (\Throwable $throwable) { | ||
| $this->logger->error($throwable->getCode().': '.$throwable->getMessage()); |
There was a problem hiding this comment.
Use message placeholders.
| 'occurrences.place.id' => array_map(function ($place) { return str_replace('/api/places/', '', $place['value']); }, $places), | ||
| 'organizer.id' => array_map(function ($organizer) { return str_replace('/api/organizers/', '', $organizer['value']); }, $organizers), | ||
| 'tags' => array_map(function ($tag) { return str_replace('/api/tags/', '', $tag['value']); }, $tags), |
There was a problem hiding this comment.
https://www.php.net/manual/en/functions.arrow.php are designed for this. Consider using them, i.e.
| 'occurrences.place.id' => array_map(function ($place) { return str_replace('/api/places/', '', $place['value']); }, $places), | |
| 'organizer.id' => array_map(function ($organizer) { return str_replace('/api/organizers/', '', $organizer['value']); }, $organizers), | |
| 'tags' => array_map(function ($tag) { return str_replace('/api/tags/', '', $tag['value']); }, $tags), | |
| 'occurrences.place.id' => array_map(static fn($place) => str_replace('/api/places/', '', $place['value']), $places), | |
| 'organizer.id' => array_map(static fn($organizer) => str_replace('/api/organizers/', '', $organizer['value']), $organizers), | |
| 'tags' => array_map(static fn($tag) => str_replace('/api/tags/', '', $tag['value']), $tags), |
Link to ticket
https://jira.itkdev.dk/browse/DISPLAY-1019
Description
Checklist