313726 - Relative dates should work more sanely: e.g. -1w should mean 7 days and -1ws mean starting this week

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Gervase Markham [:gerv]]

When you are searching using relative dates, -1d currently means 00:00PST on the previous day. It should mean "the last 24 hours". Similarly, -1M means the first of the month; it should mean 30 days ago. That's a more logical meaning, and leads to the ability to make more useful queries (like a buglist one only looks at once a week on a Wednesday).

This could be achieved by some magic with a Perl date module, and converting to elapsed hours or something like that.

Gerv

Comment 1

[Gervase Markham [:gerv]]

I think this _may_ be fixed now...

Gerv

Comment 2

[Gervase Markham [:gerv]]

-1d on b.m.o. today now appears to mean between 24 and 48 hours!

Gerv

Comment 3

[Biju]

initially I was irritated to see -1d not equal to -24h
but I feel -1w not equal to -7d is good.
This way user have 2 choices.  
* if he want really 7day he can use -7d
* or just this week, he can use -1w

so do for month and year (three !!! -365d, -12m -1y)

Comment 4

[Marc Schumann [:Wurblzap]]

I find the way it is very useful and flexible. I have searches for "today" (0d) and "this month" (0m), for example.

Comment 5

[Gervase Markham [:gerv]]

Marc: where are you in the world? Such searches only make sense if you are in the same or a similar timezone as the Bugzilla server. If I search for "0d" on b.m.o., then it doesn't do anything like produce all bugs filed in my "today".

-0d should be meaningless, -1d should be 24 hours, so it works wherever you are in the world, -2d should be 48 hours and so on.

(I guess we could special-case -0d if it made you happy.)

Gerv

Comment 6

[Marc Schumann [:Wurblzap]]

Most Bugzillas I work with are local Bugzillas and are set to the same timezone I'm in. I find it very useful to have searches about this day, this day and last, this week, this week and last, this month, this month and last, this year. I use these all the time, and I can do so easily with the current setup. I don't want to lose this ability.

I can see that Bugzilla's behaviour may need some documentation, though.

Comment 8

[miketosh]

At least this functionality should be described to users.

Entering 	Translates to
NOW 	Right now
0H 	The start of the current hour
-2H 	2 hours or 120 minutes ago
-3D 	3 days or 72 hours ago
-1M 	The start of last month. If today is 3/23, then -1m would use 2/1
-3W 	The start of 3 weeks ago. If today is Friday, then it uses four Sundays ago
-1y 	January 1 of last year. 

I agree with Gerv on this.  If I want to see all bugs since this day (the 28th for example) last month, my search will be different each month.  In February it would be -31d, in March it could be -28d or -29d, in May it would be -30d.  These values should at a minimum be documented, and a better idea would be to fix these values, but still provide the functionality that Marc finds useful.

Perhaps -1sw, could be the start of 1 week ago, and -1w would be EXACTLY 1 week ago.  Same for -1sy and -1y.  -1sm would be the first of last month, and -1m would be the 28th of last month.

It is hard to describe to users that if they put in -1m they could get up to 60 days worth of bugs back in their search.

Comment 9

[Marc Schumann [:Wurblzap]]

Yeah, the s modifier sounds like a sensible idea. You can get the intuitive result using -1w, and if you want the beginning of last week, then you can do it using the modifier. I like that.

Comment 10

[miketosh]

Attachment: Code Patch Ver 1

Here is a patch against trunk (as of today, 2011-04-05) that will use a modifier 's' to make a search use the start of a time unit, as opposed to EXACTLY this day/hour/minute/second so many time units ago.

To search for all issues opened in the last year, you'd enter a -1y, instead of a -365d or -366d depending on the leap-year.

To search for all issues opened since January 1 last year, you'd enter a -1sy for the start of one year in the past.  Obviously this would require a note to users in the Release Notes, but I think this fixes Bugzilla to follow how most people would expect a search to behave.

The strangest part is that the relative dates don't all follow the same logic.  Some go to the start of the previous time unit, while others are precise.  Again, this is fixed by my proposed patch.

Comment 11

[Max Kanat-Alexander]

This sounds like a pretty sensible solution to me, actually. My only concern is whether or not we should be changing the default behavior.

Comment 12

[Max Kanat-Alexander]

  Also, I sort of think the "s" should come after the normal letter, not before it.

Comment 13

[miketosh]

In order to keep it uniform, I would suggest changing the default behavior.  I would guess (although probably wrongly) that people expecting the 'start of' behavior are advanced types and would be easier to adapt to this type of change.

But I can code it however you want if it means it would get accepted.  Max, would you prefer to do a more thorough code review of this patch, and then I can post the 'after' change?

Comment 14

[Max Kanat-Alexander]

Yeah, I agree with you about the default behavior, it makes more sense for those to be a full X weeks or years or months, etc. I'll do a review, sure.

Comment 15

[Max Kanat-Alexander]

Comment on attachment 524110 [details] [diff] [review]
Code Patch Ver 1

>         if ($unit eq 'd') {
>             $date -= $sec + 60*$min + 3600*$hour + 24*3600*$amount;
>-            return time2str("%Y-%m-%d %H:%M:%S", $date);
>+            my $fmt = $startof ? "%Y-%m-%d 00:00:00" : "%Y-%m-%d %H:%M:%S";
>+            return time2str($fmt, $date);

  I think the -= line is normalizing this to the beginning of the day, so you have to do an "if $startof" there too.

>         elsif ($unit eq 'h') {
>+            my $fmt = "%Y-%m-%d %H:%M:%S";

  Since you keep using that all over this function, you should probably just write it once and re-use it.


  Also, $startof needs to always be true when the user specifies "0" as the amount (that's the current behavior and seems sensible enough to me).

Comment 16

[miketosh]

Attachment: Code Patch Ver 2

Better?  I made 0-amount be a starting amount, moved the modifier to the end, made $fmt a little more global in this function, and fixed the day's  functionality.

Comment 17

[Max Kanat-Alexander]

Comment on attachment 524113 [details] [diff] [review]
Code Patch Ver 2

I think this is the same patch as before.

Comment 18

[miketosh]

Attachment: Code Patch Ver 2 (Actual updated patch this time)

(In reply to comment #17)
> I think this is the same patch as before.

That it was.  I, uh, meant to do that.  Yeah.  Just wanted to see if you were paying attention! ;-)

Comment 19

[Max Kanat-Alexander]

Comment on attachment 524115 [details] [diff] [review]
Code Patch Ver 2 (Actual updated patch this time)

>+        if ($amount == 0) { $startof = 1; }  

  Nit: this is better written as:

  $startof = 1 if $amount == 0;

  Otherwise this looks great! I assume that you've tested this pretty thoroughly?

Comment 20

[miketosh]

Attachment: Code Patch Ver 3

(In reply to comment #19)
> >+        if ($amount == 0) { $startof = 1; }  
> 
>   Nit: this is better written as:
> 
>   $startof = 1 if $amount == 0;

Totallly agree.  This was how it read the first time I wrote it, but I rewrote it to match the if below it.  This new patch corrects this nit, as well as a comment in the function.

>   Otherwise this looks great! I assume that you've tested this pretty
> thoroughly?

Yes, tested this same patch against 3.7.3 even, and it worked really well there too. The patch didn't apply, but manually editing the file to make the same changes did the trick.  -3ms translated to 2011-01-01 00:00:00, where -3m translated to 2011-01-05 22:10:12.

Max, can you please apply this patch for me?  I still, appropriately, cannot check in patches.

Comment 21

[Max Kanat-Alexander]

Comment on attachment 524117 [details] [diff] [review]
Code Patch Ver 3

Yeah, I can check it in. Is there any special reason you need it checked in now, or can I wait a bit?

Comment 22

[miketosh]

Whenever is fine.  I'm not waiting for it.

Comment 23

[Max Kanat-Alexander]

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Search.pm
Committed revision 7772.

Comment 24

[Frédéric Buclin]

(In reply to comment #20)
> changes did the trick.  -3ms translated to 2011-01-01 00:00:00

I'm impressed how long 3 milliseconds is. :)

Comment 25

[miketosh]

(In reply to comment #12)
>   Also, I sort of think the "s" should come after the normal letter, not before
> it.

I'm updating the new title to match this.

Comment 26

[Frédéric Buclin]

Added to relnotes in bug 713346.