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: Tyson Tate <tyson@…> 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)

test_proj.zip (8.5 KB ) - added by Tyson Tate <tyson@…> 18 years ago.
Minimal test case
date_based.py.diff (3.8 KB ) - added by Pete Crosier 18 years ago.
date_based.py.2.diff (3.3 KB ) - added by Pete Crosier 18 years ago.
Works a bit more logically.

Download all attachments as: .zip

Change History (15)

by Tyson Tate <tyson@…>, 18 years ago

Attachment: test_proj.zip added

Minimal test case

by Pete Crosier, 18 years ago

Attachment: date_based.py.diff added

comment:1 by Pete Crosier, 18 years ago

Cc: pete.crosier@… added
Resolution: fixed
Status: newclosed
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 Jacob, 18 years ago

Resolution: fixed
Status: closedreopened

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 Pete Crosier, 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 Tyson Tate <tyson@…>, 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 Pete Crosier, 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 :)

by Pete Crosier, 18 years ago

Attachment: date_based.py.2.diff added

Works a bit more logically.

comment:6 by Adrian Holovaty, 18 years ago

Owner: changed from Jacob to Adrian Holovaty
Status: reopenednew

comment:7 by Adrian Holovaty, 18 years ago

Status: newassigned

comment:8 by Chris Beaven, 18 years ago

Needs documentation: set
Triage Stage: UnreviewedReady 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:9 by Chris Beaven, 18 years ago

Triage Stage: Ready for checkinAccepted

Oops, I meant "accepted"

comment:10 by Per Jonsson <poj@…>, 18 years ago

Cc: poj+django@… added
Patch needs improvement: set

The patch no longer applies, as it's changes conflicts with changes introduced when fixing #2433.
The allow_future parameter introduced by #2433 is not honored. So patch needs improvement.

comment:11 by Per Jonsson <poj@…>, 18 years ago

Ticket #3585 has an alternate solution to this problem.

comment:12 by Jacob, 18 years ago

Resolution: duplicate
Status: assignedclosed

Marking duplicate (see Per's comments).

Note: See TracTickets for help on using tickets.
Back to Top