We are using gmdate() which is meant to *always* provide an english string (because it's meant for web server response headers and other standards). I think using drupal's format_date() is a proper almost-drop-in approach since it can be told to use timezone 0 (equivalent to GMT) and also uses gmdate itself (also using t() in proper spots for localization).

Patch attached.

CommentFileSizeAuthor
#12 2010-03-26_165121.png3.02 KBjanusman
#12 2010-03-26_165209.png2.95 KBjanusman
#12 2010-03-26_165834.png2.31 KBjanusman
#12 2010-03-26_171454.png2.21 KBjanusman
#12 2010-03-26_171619.png2.84 KBjanusman
#10 apachesolr-708424-10.patch1.14 KBjanusman
#7 apachesolr_date_format.patch1.25 KBmeba
apachesolr_date_format.patch1.26 KBjanusman

Comments

meba’s picture

meba commented
Status: Active » Needs review

Applied without a problem, then tried browsing the site - works

janusman’s picture

janusman commented

Meba: would you say it's Ready to be committed? =)

meba’s picture

meba commented

Not sure, I am not familiar with apachesolr enough (yet :-)

robertdouglass’s picture

robertdouglass commented
Status: Needs review » Reviewed & tested by the community

Looks good to me. Thanks.

janusman’s picture

janusman commented
Status: Reviewed & tested by the community » Fixed

Ok, see commits for 6.x-1.x, 6.x-2.x and 5.x-2.x:

http://drupal.org/cvs?commit=339236
http://drupal.org/cvs?commit=339240
http://drupal.org/cvs?commit=339244

pwolanin’s picture

pwolanin commented
Status: Fixed » Active

No, this was not right. I think the timezone should be NULL not zero so that the user sees the time in their own TZ (or the site TZ).

http://api.drupal.org/api/function/format_date/6

meba’s picture

meba commented
Status: Active » Needs review
StatusFileSize
new apachesolr_date_format.patch1.25 KB
pwolanin’s picture

pwolanin commented
Status: Needs review » Needs work

I think the 1st patch is already committed, so please roll any new patches from the latest CVS.

janusman’s picture

janusman commented

@pwolanin: IIUC, the original code always showed dates in the GMT timezone, ignoring the site's/user's timezone settings. So I think your comment is more about the original code than my patch? =)

Either way, I agree the dates should be shown in the site's/user's timezone settings.

janusman’s picture

janusman commented
Status: Needs work » Needs review
StatusFileSize
new apachesolr-708424-10.patch1.14 KB

Would this do? This patch is for the current 6.x-2.x-DEV.

pwolanin’s picture

pwolanin commented

I think that's all that's required, assuming you tested it

janusman’s picture

janusman commented
StatusFileSize
new 2010-03-26_171619.png2.84 KB
new 2010-03-26_171454.png2.21 KB
new 2010-03-26_165834.png2.31 KB
new 2010-03-26_165209.png2.95 KB
new 2010-03-26_165121.png3.02 KB

I looked a bit into this and it won't work =(

*In a sense* date handling is a bit broken since before the patch:

  1. Rolled back and using gmdate() instead of format_date()
  2. Set the site's timezone to +0000 (GMT)
  3. Erased all site content and created TWO nodes only, with a post dates of:
    • 2010-02-28 23:59:59 +0000
    • 2010-01-01 00:00:00 +0000
  4. Deleted the index, re-indexed, waited for everything to be processed.
  5. Searched for them (they both have the same unique keyword in the title) ...

Produces these results::

Both show up in the month "february" in the "FIlter by post date" block, and one of them under march:
2010-03-26_165834.png

Clicking on "february" now shows both (!) in Feb. 28 and one in march 1st:
2010-03-26_165121.png

Clicking on "february 28th" shows one at 11PM, the other at 12AM...
2010-03-26_165209.png

Note: Making the second node's time from 00:00:00 to 00:00:01 +0000 fixes this (shows a count of (1) under february, and (1) under march)

Now, still with the rolled-back patch (currently using gmdate()) I change the site's timezone to -0600 and leave the nodes. The same filters show as above.
Now, when I introduce the first patch, the behavior is the same as the earlier cases, BUT the dates are now localized (which was my intention in the first place).
If I use the patch from #10 (use format_date with a timezone of NULL instead of 0) things get *screwy*... this shows up after searching by keyword for the nodes:

2010-03-26_171454.png

clicking on "January" (yes, january!??) now shows:

2010-03-26_171619.png

etc. etc.

Soooo...to close out:

conclusion

I move to use the original patch which doesn't try to fix anything regarding the timezones, just localizes the dates =) The timezone issue is (I bet) a more complex issue since we'd have to issue Solr queries in GTM shifted by the timezone the user is viewing (e.g. so a node posted as 28-feb-2010 +0000 will show up to a user in CST timezone (-0600) as "March > March 1 > March 1, 06:00" ... AND if he wants to filter to "March" it would mean sending to Solr a time-shifted query (instead of the current simplistic [2010-03-01 00:00:00Z TO 2010-04-01 00:00:00Z])

Thoughts?

janusman’s picture

janusman commented

BTW my proposal to "use the original patch" in effect means "do nothing and mark this issue fixed" as that patch was already committed.

pwolanin’s picture

pwolanin commented

@janusman - I was wondering about the very timezone issue you bring up. It would be especially bad if you are a lon g ways from GMT - but as is it will be bad since your result set may include dats that are listed as half a day off from your apparent query.

So, I think we cannot mark this as closed, but perhaps you are right that we should not change 0 to NULL for now.

janusman’s picture

janusman commented

I'm just suggesting to close the original issue (localize existing functionality). IMO the timezone problem should be a whole different issue.

Can I somehow convince you to close this out and open a different issue for the [more complex] timezone issue? =) If not, just adjust the issue's title =)

pwolanin’s picture

pwolanin commented
Status: Needs review » Fixed

Sure, let's open a new issue.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.