Skip to content

Commit

Permalink
Also show future news items if the "show all news items" option is se…
Browse files Browse the repository at this point in the history 
…lected (see #419)
  • Loading branch information
@leofeyer
leofeyer committed Apr 18, 2019
1 parent d883272 commit 2137caa
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
@@ -1,5 +1,9 @@
# Change log

## DEV

* Also show future news items if the "show all news items" option is selected (see #419).

## 4.4.38 (2019-04-10)

* Correctly copy multiple events into an empty calendar (see #427).
Expand Down
4 changes: 2 additions & 2 deletions news-bundle/src/Resources/contao/modules/ModuleNewsArchive.php
View file
Expand Up @@ -142,8 +142,8 @@ protected function compile()
}
elseif ($this->news_jumpToCurrent == 'all_items')
{
$intBegin = 0;
$intEnd = time();
$intBegin = 0; // 1970-01-01 00:00:00
$intEnd = 2145913200; // 2038-01-01 00:00:00

This comment has been minimized.

Sign in to view

Copy link
@Toflar

Toflar Apr 23, 2019

Member

Why only 2038? We don't have to limit to 32bit, do we? Shouldn't that just be PHP_INT_MAX?

This comment has been minimized.

Sign in to view

Copy link
@fritzmg

fritzmg Apr 23, 2019

Contributor

If you have recurring events (that recur forever), you'll likely run into your memory_limit on 64bit PHP environments. I noticed that via inspiredminds/contao-sibling-navigation#3

This comment has been minimized.

Sign in to view

Copy link
@Toflar

Toflar Apr 23, 2019

Member

Yeah but that's a different issue. You might also run into that one with 2145913200 depending on the number of repetitions 😄

This comment has been minimized.

Sign in to view

Copy link
@fritzmg

fritzmg Apr 23, 2019 •
edited

Contributor

Yes of course. But with PHP_INT_MAX you are far more likely to run into that issue, when running 64bit. The eventlist module also uses 2038-01-01 00:00:00.

With the current implementation, I don't think there is any solution other than arbitrarily limiting recurring events.

And I don't know if there is any other way of calculating all events for whatever purpose. If you want to display all events, you need to get all events somehow. And if you have a recurring event (without a limitation), then there will be infinite events of course - and thus you need an infinite amount of memory.

The calendar module is not future proof in various ways currently 😁

This comment has been minimized.

Sign in to view

Copy link
@leofeyer

leofeyer Apr 23, 2019

Author Member

I'm ok with using PHP_INT_MAX, however, we do need to find a solution for repeated events first.

This comment has been minimized.

Sign in to view

Copy link
@Toflar

Toflar Apr 23, 2019

Member

Yeah, imho that's a configuration value in the module settings (Maximum repetitions for repeated events - The list will grow incredibly long if you set this value too high and will eventually hit server resource limits. Choose a reasonable value here.). And we should just set it to a reasonable default amount which imho is something like 10 or so. It has to be configurable because you don't know if people have 1 event repeating every week or 200 events of which 5 are repeating every day. It might also depend on other filter settings such as only today's events or this year's events etc.

This comment has been minimized.

Sign in to view

Copy link
@fritzmg

fritzmg Apr 23, 2019

Contributor

Wouldn't it also make sense to be able to set an end date for repeated events?

This comment has been minimized.

Sign in to view

Copy link
@leofeyer

leofeyer Jun 6, 2019

Author Member

See #510.
}
}
catch (\OutOfBoundsException $e)
Expand Down

0 comments on commit 2137caa

Please sign in to comment.