Opened 18 years ago
Closed 18 years ago
#2386 closed defect (duplicate)
[patch]previous_month and previous_day are non-empty even if there are no objects in previous months/days
Reported by: | Owned by: | Adrian Holovaty | |
---|---|---|---|
Component: | Generic views | Version: | dev |
Severity: | normal | Keywords: | previous_month previous_day genericviews |
Cc: | tyson@…, pete.crosier@…, poj+django@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Yes, I know this is documented behavior, but it is completely illogical.
In the django.views.generic.date_based
generic views, previous_month
and previous_day}} are not empty when there are no objects in previous months/days. {{{next_month
and {{{next_day}} are empty when there are no next months and days, which is logical.
I'd expect previous_month
and {{{previous_day}} to be empty when there are no objects in the previous months/days. How else are you supposed to implement date-based navigation?
Attachments (3)
Change History (15)
by , 18 years ago
Attachment: | test_proj.zip added |
---|
by , 18 years ago
Attachment: | date_based.py.diff added |
---|
comment:1 by , 18 years ago
Cc: | added |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Summary: | previous_month and previous_day are non-empty even if there are no objects in previous months/days → [patch]previous_month and previous_day are non-empty even if there are no objects in previous months/days |
I've put together a patch that I think does what you want.
archive_month
now returns a next_month
and previous_month
only for months with objects, otherwise it returns None
. This takes into account the specified year only, which seems logical (to me).
archive_day
now returns a next_day
and previous_day
only for days with objects, otherwise it returns None
. This takes into account the specified month only, which seems logical (again, to me).
Criticism fully appreciated, and please un-fix this if I shouldn't have marked it so :)
comment:2 by , 18 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Good patch, Pete!
I'm not so hot on the idea of only returning next/prev in the current year/month; seems like that would bite me in the ass when dealing with Jan 1st :). Think you could rework the patch to always return the next/prev month/day (if it exists) reguardless of current year/month?
(Oh, and marking it fixed is a little premature; we still gotta apply the patch :)
comment:3 by , 18 years ago
That was fast, and cheers for re-opening it :) I wasn't sure which way to go with the scope of returning things, but I see where you're coming from.. I'll see what I can conjure up.
comment:4 by , 18 years ago
Just a note, Pete: you rule. Thanks a ton for looking at this. I was going to take a crack at it tonight, but your patch does exactly what I'm looking for (except for what Jacob mentioned, which I agree with).
This is going to make date-based navigation with the generic views a dream.
comment:5 by , 18 years ago
OK, version 2 is sorted.. it works more logically and the code is a bit cleaner too because of it.
Now it returns the closest previous/next months from the queryset from any year, if applicable. And it returns the closest previous/next days for any month/year, if applicable.
Thanks Tyson/Jacob :)
comment:6 by , 18 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
comment:7 by , 18 years ago
Status: | new → assigned |
---|
comment:8 by , 18 years ago
Needs documentation: | set |
---|---|
Triage Stage: | Unreviewed → Ready for checkin |
If the existing behaviour is "documented" as the ticket suggests, this documentation needs changing. Someone care to provide the patch for this too?
Extra credit if you could come up with some tests, but I'm not sure they are necessary.
comment:10 by , 18 years ago
Cc: | added |
---|---|
Patch needs improvement: | set |
comment:12 by , 18 years ago
Resolution: | → duplicate |
---|---|
Status: | assigned → closed |
Marking duplicate (see Per's comments).
Minimal test case