Recurring event: end of #600
Conversation
...ons/2023_02_09_152218_update_entity_events_add_recurring_until_month_recurring_until_day.php
Show resolved
Hide resolved
|
Doesn't seem to work on the yearly view. I have an event set to end in May 2023. On the yearly calendar, it's still showing in december 2023 On the calendar dashboard, it's not seeing upcoming events happening in a few days |
| (!empty($reminder->recurring_until) && empty($reminder->recurring_until_month) && $reminder->recurring_until < $this->currentYear()) || | ||
| (!empty($reminder->recurring_until_month) && empty($reminder->recurring_until_day) && $reminder->recurring_until_month < $this->currentMonth() && $reminder->recurring_until < $this->currentYear()) || | ||
| (!empty($reminder->recurring_until_day) && ($reminder->recurring_until_day < $this->currentDay() && $reminder->recurring_until_month == $this->currentMonth() && $reminder->recurring_until == $this->currentYear() || | ||
| $reminder->recurring_until_month < $this->currentMonth() && $reminder->recurring_until <= $this->currentYear()) |
There was a problem hiding this comment.
Note for later: I'm confused by the logic here, since it's doing the same logic here compared to in the "upcoming" events loop. Shouldn't it do the reverse? And if it's the same, we should remove the copy-pasting and make it a function
| // Recurring? We need to switch around the data a bit to figure out the most recent date | ||
| if (!empty($this->is_recurring)) { | ||
| if ($this->recurringMonthly()) { | ||
| //dump('monthly'); | ||
| $reminderMonth = $month; |
There was a problem hiding this comment.
Why remove this line? If it's recurring monthly but doesn't have more days, it will be the wrong value, no?
| @@ -478,7 +484,9 @@ public function mostRecentOccurrence(int $year, int $month, int $day, array $mon | |||
| if ($days > 1 && $this->length > 1) { | |||
| $days -= $this->length - 1; | |||
| } | |||
|
|
|||
| if ($overDate) { | |||
| $days = -2; | |||
There was a problem hiding this comment.
Why are we doing -2 here? It should be detailed in the code what's happening here, and why two.
| @@ -576,6 +588,10 @@ public function nextUpcomingOccurrence(int $calendarYear, int $calendarMonth, in | |||
| } | |||
|
|
|||
| $days += ($reminderDay - $day); | |||
|
|
|||
| if ($overDate) { | |||
There was a problem hiding this comment.
Same thing here, why -2 specifically?
| //Does the event end this month? if so what day? | ||
| //dump($extraDate, $reminder->recurring_until_month); | ||
|
|
||
| if (!empty($reminder->recurring_until) && $reminder->recurring_until == $y && !empty($reminder->recurring_until_month) && $reminder->recurring_until_month <= $m) { |
There was a problem hiding this comment.
Copy-pasted code, should be a single function if
| <i class="fa-solid fa-calendar" title="{{ $reminder->readableDate() }}" data-toggle="tooltip" data-placement="bottom"></i> | ||
| </div> | ||
| {{ link_to($reminder->entity->url(), $reminder->entity->name) }} | ||
| @if ($reminder->daysAgo() > -1) |
There was a problem hiding this comment.
Nested ifs make the code more difficult to read. Instead of
@if ($reminder->daysAgo() <= 0 || !$reminder->entity)
@continue
@endif| <i class="fa-solid fa-calendar-check" data-toggle="tooltip" title="{{ __('calendars.actions.today') }}"></i> | ||
| @else | ||
| <i class="fa-solid fa-calendar" title="{{ $reminder->readableDate() }}" data-toggle="tooltip" data-placement="bottom"></i> | ||
| @if ($reminder->inDays() > -1) |
There was a problem hiding this comment.
Same story here, both ifs should be merged into an early continue statement
|
Closing this as no one has requested it again |
No description provided.