Skip to:
Content

Opened 4 years ago

Closed 3 years ago

Last modified 3 years ago

#2203 closed defect (bug) (fixed)

GMT offset bug for date_sent in bp-messages-classes.php [Has Patch]

Reported by: rnaegle Owned by: DJPaul
Milestone: 1.5 Priority: major
Severity: Version:
Component: Messaging Keywords: has-patch needs-testing
Cc: email@…, piphut, paulhastings0@…, hashmore@…

Description

I found that newly created messages were being displayed as if they had been sent at an incorrect time relative to my system timezone. In my case the local timezone is EDT; messages that were just sent appeared to have been sent 4 hours ago.

The problem persists after updating to 1.2.2.1

It appears the culprit was the MySQL function FROM_UNIXTIME returns a timestamp relative to the system timezone, but bp_core_time_since expects a timestamp relative to GMT.

Attached is a patch that works on my instance of BuddyPress.

Attachments (5)

bp-messages-classes.patch (853 bytes) - added by rnaegle 4 years ago.
2203.001.diff (7.3 KB) - added by cnorris23 4 years ago.
update based on r-a-y's tip
2203.002.diff (8.0 KB) - added by cnorris23 4 years ago.
fix-wonky-time.patch (19.0 KB) - added by johnjamesjacoby 4 years ago.
Wonky Time
omgpizza.patch (1.5 KB) - added by DJPaul 3 years ago.

Download all attachments as: .zip

Change History (45)

comment:1 rnaegle4 years ago

  • Owner set to rnaegle
  • Status changed from new to assigned
  • Summary changed from GMT offset bug for date_sent in bp-messages-classes.php to GMT offset bug for date_sent in bp-messages-classes.php [Has Patch]

comment:2 cnorris234 years ago

  • Keywords has-patch added; messages timezone GMT removed
  • Milestone set to 1.2.4
  • Priority changed from trivial to normal

IIRC, there are two other instances (three, including this one) of FROM_UNIXTIME being used. Have you taken a look at these?

comment:3 21cdb4 years ago

I just discoered this issue too. It's still not working in 1.2.3, i'll hope the fix will make it into 1.2.4 besides the 3.0 fixes.

comment:4 21cdb4 years ago

  • Cc email@… added

comment:5 r-a-y4 years ago

Check out changeset [2424] for clues.

Seems to be the same issue as what is listed here, but changeset applied to the groups component.

comment:6 cnorris234 years ago

  • Keywords needs-testing added
  • Milestone changed from 1.2.4 to 1.3

Based on r-a-y's tip, I've gone through and updated the patch to line up with what has already been commited. I've also fixed the other references to FROM_UNIXTIME, and any related references to time(). I have not tested this at the moment, but I will.

@rnaegle and 21cdb,
could you, both, give this a test please?

cnorris234 years ago

update based on r-a-y's tip

comment:7 rnaegle4 years ago

I've made the changes based on 2203.001.diff--the patch fixes incorrect times expressed in relative terms (based on bp_core_time_since). I've noticed that messages displayed with explicit date and time don't get converted from GMT to local time.

For example, suppose I send a message to another user today a 23:00 GMT. When viewing the thread 5 minutes later, the relative time will display "Sent 5 minutes ago." However, when viewing the list of all messages sent the time sent will be displayed as "April 15, 2010 at 11:00 pm" instead of displaying the time relative to my current timezone.

My GMT offset is configured for both PHP and WPMU. When experimenting with bp-messages-templatetags.php I noticed that date_default_timezone_get() returns "UTC" instead of the correct timezone. I have bbPress installed via BuddyPress

comment:8 cnorris234 years ago

@rnaegle

When experimenting with bp-messages-templatetags.php I noticed that
date_default_timezone_get() returns "UTC" instead of the correct timezone.

date_default_timezone_get() returns UTC because WP now explicitly sets the TZ to UTC for those using PHP 5.1+. This was to fix a problem that was partly experienced in this ticket. You can read more about the change here (http://wpdevel.wordpress.com/2010/05/29/plugins-using-dateu-or-time-may/). Dates are still converted to the TZ set in your WP options when needed.

I've updated the patch to fix the "time sent" display when viewing your message list. Could you please test?

cnorris234 years ago

comment:9 piphut4 years ago

  • Cc piphut added
  • Component changed from Activity to Messaging

comment:10 follow-up: cnorris234 years ago

@piphut
Can you try the provided patch? It won't fix previous messages, but it should address future message timestamps.

comment:11 in reply to: ↑ 10 pisanojm4 years ago

Replying to cnorris23:

@piphut
Can you try the provided patch? It won't fix previous messages, but it should address future message timestamps.

@pisanojm

I tried this patch.... it worked the very first time I was in a message and sent. Subsequently it was found that I was not receiving private messages with this patch/mod. I reverted back to the old/original and then I was able to receive private messages again.

Never used this ticket system before, so I hope this is right...

comment:12 pisanojm4 years ago

Tried this in 2.9.2 WP (cnorris23 -2203.002.diff). With Buddypres 1.2.4.1.
Double Checked to make sure coded correct. Ended up stating that new private messages were now sent 10 years and 6 months ago. Changed files back and it is back to stating new private messages were sent 5 hours ago.

comment:13 cnorris234 years ago

@pisanojm
This patch was written to run on WP 3.0 (trunk) and BP 1.3 (trunk), not WP 2.9.2/BP 1.2.4.1. The patch depends on changes made in WP 3.0, as I stated here, and, consequently, does not produce the desired results in WP 2.9.2/BP 1.2.4.1. Any messages sent before the code change will still report the wrong time, but they should only be a few hours off, not 10 years. I can't reproduce the 10 year issue, nor can I reproduce the inability to receive messages on BP 1.3 (trunk) or BP 1.2.4.1. Your first comment says that you got it to work, but you couldn't receive messages. Your second says you checked to see if the patch was coded correctly, and you end up with the 10 year issue. Did you change something the second time? How are you applying the patch? Please try the patch running the most recent WP 3.0-RC2 and BP 1.3 (latest trunk) and tell me what you get.

comment:14 pisanojm4 years ago

@cnorris23
Ok, Sorry I wasn't clear. Let me see if I can clear up some of this in a longer reply. First off, when I first attempted this I used the bp-messages-classes.patch.
When I did this simple edit. I opened my inbox and sent a few messages, all of these messages sent with the correct timestamp. However, about an hour later, I noted that I was not receiving messages in my INBOX. Yet, I was receiving emails and Private Message Notifications. Next, I undid the bp-messages-classes.patch edit. While I didn't receive the messages that said were sent to me during the running of the above patch, it all went back to "normal"... With normal being a 5 hour off timestamp.

The, I posted a number of buddypress.org questions... Which is where it was suggested I run your latest .diff patch. Again this was done to WP 2.9.2 and 1.2.4.1. When I said checked to see if "coded correct" above, I simply meant that I double checked to make SURE THAT I coded it correctly in the change using PSPAD. Which I did. This literally stated that all messages sent were 10 years and 6 months ago... But private messages were being sent and received.

I then went to update my site to Wordpress 3.0 RC2 to try the timestamp edit again, but afer installing it was noted that the Activity Stream (which is sent to the "home" page") was simpy missing.... You can see this conversation here:

http://buddypress.org/community/groups/how-to-and-troubleshooting/forum/topic/activity-stream-as-home-page-results-in-page-not-found/

So, because we are in the process of getting this thing up-and-running (we are in pre-launch, and I'm not much of a coder), I simply did a cpanel fullbackup restore of the site back to 2.9.2 BP 1.2.4.1 because there was no sense in trying to fix the timestamp issue at that point if no-one can see the activity stream. I don't have a "test BP" site, it's a production site that I'm willing to take some risks on to make it work... Hope this clears this up... When I said I got it to work, it was with the original .patch and BEFORE I realized that private messages were not longer being received. Here is the group thread about that:
http://buddypress.org/community/groups/how-to-and-troubleshooting/forum/topic/message-timestamps-are-wrong/

Would love to see this fixed...

comment:15 pisanojm4 years ago

This issue is still happening on WP 3.0 and BP 1.2.5.2.
I just re-applied this patch and I truly get a sent 40 years, 6 months ago... Again, I double checked to see if I entered the codes correctly on all of the files...

You can see this here:
http://mustech.net/miscpicts/40years.jpg

I have made sure that my Linux Server time/date is correct as well as my Time Zone/Date in WP.

comment:16 cnorris234 years ago

@pisanojm
The attached patch isn't for the 1.2.x branch which is why it's not working. I'll look into getting a patch for the 1.2.x branch, and see if we can get a fix in.

comment:17 paulhastings04 years ago

  • Cc paulhastings0@… added
  • Milestone changed from 1.3 to 1.2.6

Well, taking a looking at the issue here I think @nuprn1 may have narrowed down the problem in [Trac ticket #2504 http://trac.buddypress.org/ticket/2504].

comment:18 cnorris234 years ago

I've literally tried to replicate the "years" problem multiple ways, but I can't reproduce this at all. For those with the issue, are you running into this bug on messages you've just sent, or, is this problem occurring on old messages in the thread?

Also, I was mistaken, the patch, attachment:2203.002.diff does apply cleanly for the 1.2.x branch, and it's the one I'm using. If anyone could give some me some steps to reproduce the issue that would be magnificent. This is really starting to bother me now :/

comment:19 pisanojm4 years ago

Please contact me directly.... info [at] jpisano.com Wish there was PM on BP.org site...

comment:20 pisanojm4 years ago

When I send the message it STATES sent 0 seconds ago. When I refresh the Page it states Sent 4 hours ago.

Also NOTE that the WHEN viewing the Sent Messages list after clicking on Messages --> sent messages THIS LIST DOES HAVE THE CORRECT TIME.

It's not until you OPEN a sent message that you see SENT 4 hours, X minutes ago.

Pict 1. Correct Here:
http://mustech.net/miscpicts/timestamp1.jpg

Pict 2. Not Correct Here:
http://mustech.net/miscpicts/timestamp2.jpg


comment:21 pisanojm4 years ago

Any new thoughts on this guys? I'd be happy to test something again.

comment:22 johnjamesjacoby4 years ago

BuddyPress needs to switch to using current_time('mysql') instead of FROM_UNIXTIME.

Working on a fix now.

comment:23 cnorris234 years ago

@jjj
My patch accomplishes the same thing, albeit, using a PHP function rather than the WordPress wrapper function. Apparently, however, I'm missing something somewhere, and haven't had time to investigate.

johnjamesjacoby4 years ago

Wonky Time

comment:24 johnjamesjacoby4 years ago

If anyone experiencing these issues would please test my wonky-time fix and submit some feedback, would be much appreciated to get this into 1.2.6.

comment:25 paulhastings04 years ago

Ugh... is the only way to test it by manually finding and replacing the strings in each of the 15 files? Or is there some simpler way of "running" a .patch file? E.g. upload it to some kind of script processor that automatically finds/replaces all of the changes.

comment:26 johnjamesjacoby4 years ago

  • Priority changed from normal to major

You would use an svn checkout and then apply this patch.

Are you familiar with SVN?

comment:27 paulhastings04 years ago

Unfortunately I'm not, but I guess I will be within the next few days. ;)

I'll have some free time on Wednesday so I'll try to get back to you with my results by that evening.

Is there any certain SVN program/client that you would recommend?

comment:28 hnla4 years ago

  • Cc hashmore@… added

@paulhastings

What platform do you work in ?
For windows download Tortoise:
http://tortoisesvn.tigris.org/

It makes all commands available on right click context menus and provides handy interfaces for operations such as revision graphs etc. You can run svn from command line of course, good luck on that one ;)

Having checked out branch 1.2 you should have a working copy of the head of the branch which you can then 'SVN Update' when required.

The next step is my warped approach I create a folder in the checkedout folder called 'diff' - scroll back up to the attachments for this ticket click on the download link for John James's patch for this ticket you will download a file called 'fix-wonky-time.patch'

right click in your working copy folder and select 'tortoiseSVN -> 'apply patch' you will then be asked to supply path to patch file or diff file navigate to that downloaded file and select it, now you should have a new window named 'TortoiseMerge'
on the left edge of this is a list of the patched files, 15 in this instance, right clicking on one can show a preview in a split window of the original file (your working copy) and the patched version.

Select 'Patch All' and you should now have your working copy files updated to the patched versions

Hope this is of some small help if you're not on Windows perhaps it isn't :(

There may well be better ways to do things than I have described, but I'm no great fan of SVN finding all too confusing at times so not really the best person to describing SVN procedures :) (I vote we change to GIT! :p )

@all core and mods - a little help with getting to grips with using SVN might be helpful, we have acknowledged a need for patchers and testers and if any impediments can be removed it might help people?

comment:29 johnjamesjacoby4 years ago

Looks like it works to me. I'm itching to get this committed because it unifies all of the current_time/last_active/date-time functions. :)

comment:30 hnla4 years ago

JJJ I would go for it, however as much as I've been aware of this issue with sent message time stamps and having prepared to merge the patch and run a quick test. I went to check that I had the issue on test install of WP 3.0 MS and couldn't replicate the it which threw me out a bit not really sure why that was but have run the patch regardless on that install and it doesn't appear to have caused any issues.

comment:31 johnjamesjacoby4 years ago

Cool thnx.

comment:32 johnjamesjacoby4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [3142]) Fixes #2203 and #2497. Introduces bp_core_current_time function as central handler for all time related functions. Removes unused bp_core_format_time in lieu of currently used bp_format_time.

comment:33 Sadr4 years ago

Looks like this will get fixed in 1.2.6, yay'''

In the future, it would be greatly appreciated if minor yet incredibly disruptive bugs like this one could be fixed in smaller, frequently released maintenance patches.

comment:34 johnjamesjacoby4 years ago

Agreed. The dev cycle for 1.2.6 was far too long and there are plans to improve that in the future.

comment:35 DJPaul3 years ago

  • Milestone changed from 1.2.6 to 1.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

We missed one on the private messaging component's index screen as reported in the forums -- http://buddypress.org/community/groups/how-to-and-troubleshooting/forum/topic/time-offset-still-present-in-bp-1-2-6/

DJPaul3 years ago

comment:36 DJPaul3 years ago

  • Owner changed from rnaegle to DJPaul
  • Status changed from reopened to assigned

Problem was as get_date_from_gmt() already applies the date offset, we were applying it twice. It seems to work but I am waiting from hnla and paulhastings0 on the forum before I commit.

comment:37 hnla3 years ago

And I've confirmed works for me. Had got it finally narrowed down to:

get_date_from_gmt()

But couldn't see that final strtotime solution, thanks Paul.

So with a bug like this can we not release point bug fixes for the community? It's ok for some of us who are comfortable making a core change manually or applying a patch but for the wider community?

comment:38 djpaul3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [3299]) Fixed #2203 again (branch)

comment:39 DJPaul3 years ago

I will put this into trunk post-merge.

comment:40 hnla3 years ago

I'm running the patch on a live site so will be keeping an eye on it to ensure it behaves.

Note: See TracTickets for help on using tickets.