Skip to content
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

Hide unpublished info from the ICS feed #1236

Merged
merged 2 commits into from
Jan 8, 2016
Merged

Hide unpublished info from the ICS feed #1236

merged 2 commits into from
Jan 8, 2016

Conversation

jrjohnson
Copy link
Member

Don’t show anything that isn’t published and display only title and
date for scheduled stuff.

Fixes #1235

Don’t show anything that isn’t published and display only title and
date for scheduled stuff.
$events = $userHandler->addInstructorsToEvents($userHandler->findEventsForUser($user->getId(), $from, $to));

//Only show published events
$events = array_filter($events, function ($event) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suggest moving this up. filter out un-published events first before loading/attaching additional data points, such as instructors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call - refactored a bit to not add details to non-published or scheduled events.

Do this before filtering is a waste as unpublished events get dropped
anyway.
@jrjohnson jrjohnson assigned stopfstedt and unassigned thecoolestguy Jan 7, 2016
stopfstedt added a commit that referenced this pull request Jan 8, 2016
Hide unpublished info from the ICS feed
@stopfstedt stopfstedt merged commit 229cc7d into ilios:master Jan 8, 2016
@jrjohnson jrjohnson deleted the 1235-publishedinfeed branch January 8, 2016 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants