Events: end-date shows up, even when it is the same as start date

[jake]

This could be part of a slightly larger project to reformat how we deal with dates/times all together. But for now, we should set it so if the start day and end day are the same, the end day is hidden.

The reason this is failing right now is because in this block:

<ul class="a-blog-item-meta">
  <li class="start-day"><?php echo aDate::dayAndTime($aEvent->getStartDate()) ?></li>
  <li class="start-date"><?php echo aDate::dayMonthYear($aEvent->getStartDate()) ?><?php if ($aEvent->getStartDate() != $aEvent->getEndDate()): ?> &mdash;<?php endif ?></li>
	<?php if ($aEvent->getStartDate() != $aEvent->getEndDate()): ?>
		<li class="end-day"><?php echo aDate::dayAndTime($aEvent->getEndDate()) ?></li>
	  <li class="end-date"><?php echo aDate::dayMonthYear($aEvent->getEndDate()) ?></li>
	<?php endif ?>
</ul>

<?php if ($aEvent->getStartDate() != $aEvent->getEndDate()): ?>

returns both the date and the time. We should be comparing the month and day only here.

I fixed it in a client project with this:

	<?php if ((date('M j', strtotime($a_event->getStartDate()))) == (date('M j', strtotime($a_event->getEndDate())))): ?>

But there's probably a more elegant way to solve the issue in the plugin.

Now that I look at it, it is showing the times in the meta block. We should take the times out.

It should be formatted like this:

--Single-day Event

Saturday, May 29th, 2010

--Multi-day Event

Saturday May 29th, 2010-
Sunday May 30th, 2010

[dordille]

I applied your proposed solution. Now that it is so easy to edit the meta data I going to close this ticket.

[jake]

I redid the Event Meta partial that Dan made to better reflect what we want to see when we first start up a new project.

Dan/Tom/Alex?, can you go over my changes and determine what needs to be cleaned up and whether this can go into the stable branch & trunk. The current event meta partial is not working correctly, so we will need to fix the 1.4 version either way.

current meta partial:

<ul class="a-blog-item-meta">
  <li class="start-day"><?php echo aDate::dayAndTime($aEvent->getStartDate()) ?></li>
  <li class="start-date"><?php echo aDate::dayMonthYear($aEvent->getStartDate()) ?><?php if ($aEvent->getStartDate() != $aEvent->getEndDate()): ?></li>
	<?php if ((date('M j Y', strtotime($aEvent->getStartDate()))) == (date('M j Y', strtotime($aEvent->getEndDate())))): ?>
		<li class="end-day"> &mdash;<?php endif ?><?php echo aDate::dayAndTime($aEvent->getEndDate()) ?></li>
	  <li class="end-date"><?php echo aDate::dayMonthYear($aEvent->getEndDate()) ?></li>
	<?php endif ?>
	<?php if (0): ?>
	<?php // Events authors are not important to end users, turned off for now ?>
  	<li class="author"><?php echo __('Posted By:', array(), 'apostrophe_blog') ?> <?php echo $aEvent->getAuthor() ?></li>   			
	<?php endif ?>
</ul>

My proposed changes:

<?php 
$startDate = aDate::dayMonthYear($aEvent->getStartDate());
$endDate = aDate::dayMonthYear($aEvent->getEndDate());
$startTime = aDate::time($aEvent->getStartDate());
$endTime = aDate::time($aEvent->getEndDate());
?>

<ul class="a-blog-item-meta">
  <li class="start-date"><?php echo $startDate ?></li>
	<?php if ($startDate == $endDate): ?>
		<?php if ($startTime != $endTime): ?>
	  <li class="start-time"><?php echo $startTime ?> &ndash; <?php echo $endTime ?></li>
		<?php endif ?>
	<?php else: ?>
	  <li class="end-date">&ndash; <?php echo $endDate ?></li>
	<?php endif ?>

	<?php if (0): ?>
	<?php // Events authors are not important to end users, turned off for now ?>
  	<li class="author"><?php echo __('Posted By:', array(), 'apostrophe_blog') ?> <?php echo $aEvent->getAuthor() ?></li>   			
	<?php endif ?>
</ul>

The screenshot shows the resulting date formats.

[agilbert]

<?php 
$startDate = aDate::dayMonthYear($aEvent->getStartDate());
$endDate = aDate::dayMonthYear($aEvent->getEndDate());
$startTime = aDate::time($aEvent->getStartDate());
$endTime = aDate::time($aEvent->getEndDate());
?>

This shouldn't be in a template. Since this partial doesn't get rendered through a component action, the aDate calls should be refactored to the model level.

$aEvent->getStartDayMonthYear();
$aEvent->getEndDayMontyYear();
$aEvent->getStartTime();
$aEvent->getEndTime();

or maybe:

$aEvent->getStartDate('d m y');
$aEvent->getEndDate('d m y');
$aEvent->getStartDate('t');
$aEvent->getEndDate('t');

[tboutell]

Hang on a minute, aDate:: calls don't belong in the model level, they are about presentation, totally view stuff. Think of them as date helpers. The model gives you a datestamp and that's the end of its responsibility.

[jake]

Yeah, alex, I figured that would be something that shouldn't be in the template. i did it mainly to clean up the code and make it clearer for this ticket. It would be nice to have an international-friendly way to customize the date format, though.

[agilbert]

This is a gray area. In my opinion it makes sense for date formatting capabilities to be in the model layer. Propel always had the ability to pass date() parameters to any method that returned a timestamp. That was really useful and never felt like it violated MVC principles. Refactoring this to the model level would ensure that properly i18n date strings could be accessed by things other than templates.

[tboutell]

It violates MVC principles, but it may not be a big enough deal to fuss about. The workhorse code that does the actual formatting job is already someplace it can be accessed from outside the model (aDate), that's the important bit. I guess having a little wrapper code in the model layer to get at it when you're using an object of that type doesn't hurt.

[johnnybenson]

It's downright painful formatting dates.

Jake or I will get a design from Rick or a third party design firm where we have even less flexibility -- the design's date formats that look nothing like any of the formats that aDate has "pre-defined"

This is only tangentially related to this ticket, but what is the most flexibile, and programmatically "correct" way to format dates when we create blog or event templates?

[tboutell]

I think you can do:

date('whatever', strtotime($event->getStartDate()));

And specify whatever formatting parameter you wish to the PHP date function.

The strtotime() call should parse the MySQL date/time string and turn it into a proper Unix timestamp so date() can eat it. (The Date class methods automatically accept either one.)

Alex might know of some sneaky way to get Doctrine to do the conversion for you.

[tboutell]

(The formats in the Date class represent house styles that Rick and I came up with, and also standard internationalized styles for various lengths if you don't set our app.yml flag to use our house styles. Neither is appropriate if the client has chosen a format, sure)

[tboutell]

Wait, I'm a dumbass! Do this:

date('whatever', Date::normalize($event->getStartDate()));

Date::normalize is the method that parses MySQL dates for the other Date:: methods. Should always return something that the date() function likes.

[jake]

We usually do use

date('whatever', strtotime($event->getStartDate()));

if we need to override the aDate formats. I don't mind using the aDate stuff in the sandbox project in the format that i've illustrated above. I think it looks pretty decent. I'm not sure why we started down this path in this conversation. I really just wanted someone to clean up my php and give me something i could commit back to the plugin to fix a bug. The larger date conversation can happen elsewhere.

[tboutell]

Jake, your partial looks totally fine to me. Calling the date formatters from the partial is totally acceptable and certainly consistent with code we have elsewhere. This is not the place or time for a fight over whether we need to move some stuff to the model layer someday... we can sweat that in 2.0. Jake please just commit and move on

[geoffd]

We should check to make sure this works once we fix the issue with the "all day" box.

If you click all day and then select a range of days, the information should display correctly in the front-end view.

If it is an event that goes over multiple days, it should show up if browsing in the day view.

[rickybanister]

We fixed the all day checkbox, there is now logic that hides the time entirely if it's an all day event.

If you select a range of days it displays correctly.

Events now span multiple days in the single day view.

I'm opening a new ticket (see #792) that addressed an issue in the Event admin where the time shown is wrong.