EWPP-1787: Add extra field displaying event status messages. #997
Conversation
22Alexandra
force-pushed
the
EWPP-1787
branch
from
7ba98a2 to
c4c1686
Compare
| } | ||
|
|
||
| // The event is ongoing but the livestream is yet to start. | ||
| if ($this->requestDateTime >= $event->getStartDate()->getPhpDateTime() && $this->requestDateTime < $event->getOnlineStartDate()->getPhpDateTime()) { |
There was a problem hiding this comment.
We will re-use condition often, so EventNodeWrapper is a good place to store it. You can use isOnlinePeriodYetToCome from openeuropa/oe_content#498.
$this->requestDateTime >= $event->getStartDate()->getPhpDateTime() also can be stored in EventNodeWrapper.
|
|
||
| // If we have a link for livestream the livestream messages have priority | ||
| // over the event status ones. | ||
| if (!$entity->get('oe_event_online_link')->isEmpty()) { |
There was a problem hiding this comment.
In the spec there is a point that livestream info should be visible if Online type != None. So it is checked in EWPP-1802. Would be inconsistent to show message here but not to show livestream due to implementation in EWPP-1802.
Or might be we need a single method in EventNodeWrapper that will check all required fields to enable livestream feature.
| } | ||
|
|
||
| // If the livestream is ongoing, cache it by its end date. | ||
| if ($this->requestDateTime >= $event->getOnlineStartDate()->getPhpDateTime()) { |
There was a problem hiding this comment.
Also isOnlinePeriodOver() can be used.
| // Cache it by the event end date and add the timezone cache contexts. | ||
| $build['#title'] = $this->t('The livestream has ended, but the event is ongoing.'); | ||
| $this->applyHourTag($build, $event->getEndDate()); | ||
| $cacheable->addCacheContexts(['timezone']); |
There was a problem hiding this comment.
| $cacheable->addCacheContexts(['timezone']); |
it was added by $this->applyHourTag()
| protected $dateFormatter; | ||
|
|
||
| /** | ||
| * ProgrammeExtraField constructor. |
| // If the event is not over then apply time-based tags, so that it can be | ||
| // correctly invalidated once the event is over. | ||
| if (!$event->isOver($this->requestDateTime)) { | ||
| $this->applyHourTag($build, $event->getEndDate()); |
There was a problem hiding this comment.
Logic issue, I set the event as planned with the event dates matching the online dates with event cancelled, the message is correct, but when I set it to Rescheduled, the event shows the message "This event has started. You can also watch it via livestream." even when I set the event state to cancelled or postponed, the message doesn't change, even when I'm logged in as admin or logged out. This is due to the fact that you don't exit here, and it continues until the line 147. Please make sure such case is covered by test after fixing it.
There was a problem hiding this comment.
We'll need confirmation from PO.
There was a problem hiding this comment.
Updated the logic.
|
|
||
| // If we have an online type set the livestream messages have priority | ||
| // over the event status ones. | ||
| if ($event->hasOnlineType()) { |
There was a problem hiding this comment.
I think the entity itself should be added as cacheable dependency. Currently the entity is not there.
| $build['#variant'] = 'warning'; | ||
| $build['#icon'] = 'warning'; | ||
| if ($event->isPostponed()) { | ||
| $build['#title'] = $this->t('This event has been postponed.'); | ||
| } | ||
| if ($event->isCancelled()) { | ||
| $build['#title'] = $this->t('This event has been cancelled.'); | ||
| } | ||
| if ($event->isRescheduled()) { | ||
| $build['#title'] = $this->t('This event has been rescheduled.'); | ||
| } |
There was a problem hiding this comment.
These will be never reached if we have online type and dates set even though the event status is set to one of these.
There was a problem hiding this comment.
The livestream messages have priority over these ones, as requested.
Regarding online type/dates/link, according to Evegenii's comment to avoid inconsistency, I'll check the Online type instead of the Online link as requested in the ticket.
There was a problem hiding this comment.
Fixed the logic displaying event status messages (if it's not as planned) over livestream ones.
22Alexandra
force-pushed
the
EWPP-1787
branch
from
41e6b42 to
bd7b1bd
Compare
22Alexandra
dismissed stale reviews from imanoleguskiza and sergepavle
via
e32b1d5
No description provided.