Django

Code

Ticket #1282 (new)

Opened 2 years ago

Last modified 3 months ago

archive_today generic view should accept a month_format parameter instead of hardcoding '%b'

Reported by: anonymous Assigned to: nickefford
Component: Generic views Version: SVN
Keywords: sprintsept14 easy-pickings Cc:
Triage Stage: Accepted Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 1

Description

builtin archive_day generic view assumes to get a 3 char abbreviated month as month-parameter. MONTHS_3 defines the english values, like "may" or "dec".

when setting the LANGUAGE_CODE to 'de-de' for example (which sets locale?), the abbreviations you get when running strftime('%b'), differ from that and archive_day will fail.

Attachments

archive_today.diff (0.8 kB) - added by Jannis Leidel <jl@websushi.org> on 03/18/07 13:28:17.
make archive_today() respect month_format
archive_today_new.diff (1.4 kB) - added by nickefford on 09/14/07 14:42:48.
Improved version of original patch
generic-views.zip (2.1 kB) - added by nickefford on 09/14/07 14:43:37.
Beginnings of some unit tests for generic views

Change History

01/27/06 08:30:54 changed by anonymous

btw, while archive_day takes month_format parameter (and respects it), archive_today doesn't do so. it hardcodes strftime('%b') which is wrong.

05/16/06 04:54:03 changed by derelm

  • version set to SVN.
  • milestone set to Version 1.0.

revision 2912 introduces translation hooks for MONTHS_3.

the issue with archive_day not respecting month_format is still valid though.

01/17/07 16:12:17 changed by

  • milestone deleted.

Milestone Version 1.0 deleted

01/20/07 16:27:08 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Accepted.

03/18/07 13:28:17 changed by Jannis Leidel <jl@websushi.org>

  • attachment archive_today.diff added.

make archive_today() respect month_format

03/18/07 13:28:35 changed by Jannis Leidel <jl@websushi.org>

  • has_patch set to 1.
  • summary changed from untranslated MONTHS_3 breaks generic date based views for some locales when not modifying month_format to [patch] untranslated MONTHS_3 breaks generic date based views for some locales when not modifying month_format.

08/20/07 05:04:29 changed by Simon G. <dev@simon.net.nz>

  • summary changed from [patch] untranslated MONTHS_3 breaks generic date based views for some locales when not modifying month_format to untranslated MONTHS_3 breaks generic date based views for some locales when not modifying month_format.
  • stage changed from Accepted to Ready for checkin.

08/25/07 16:24:33 changed by gwilson

  • needs_better_patch set to 1.
  • summary changed from untranslated MONTHS_3 breaks generic date based views for some locales when not modifying month_format to archive_today generic view should accept a month_format parameter instead of hardcoding '%b'.
  • needs_tests set to 1.
  • stage changed from Ready for checkin to Accepted.

The original intent of this ticket was fixed in [2912], so changing the title to reflect the still valid problem of the month_format hardcoding.

Also, I believe month_format should be passed along to achive_day, otherwise it will use %b by default. And we also should have some tests for this generic view.

09/14/07 09:25:23 changed by nickefford

  • owner changed from nobody to nickefford.
  • status changed from new to assigned.

09/14/07 12:39:37 changed by nickefford

  • owner changed from nickefford to nobody.
  • status changed from assigned to new.

Dropping this so that someone who understands better how to write tests for generic views can have a go.

09/14/07 14:38:04 changed by nickefford

  • owner changed from nobody to nickefford.

Reclaiming, because I've managed to figure it out.

The attached patch makes the suggested changes. I also include a zip file containing the beginnings of regression tests for generic views. Unzipping in the top-level directory should install these under tests/regressiontests/generic_views. One of the core devs should probably have a close look at these to make sure I'm doing it sensibly, but the tests pass :)

09/14/07 14:42:48 changed by nickefford

  • attachment archive_today_new.diff added.

Improved version of original patch

09/14/07 14:43:37 changed by nickefford

  • attachment generic-views.zip added.

Beginnings of some unit tests for generic views

09/14/07 14:43:57 changed by nickefford

  • needs_better_patch deleted.
  • needs_tests deleted.

09/14/07 15:40:26 changed by Rob Hunter <robertjhunter@gmail.com>

  • keywords set to sprintsept14.
  • needs_better_patch set to 1.

A few comments:

  • The "application" being loaded for the test is called "generic views" but the URLconf uses date_based explicitly. I'd feel more comfortable with URLs like /generic_views/date_based/archive_today/... so that other generic views have a place to put their tests.
  • The test case inherits from UnitTestCase and instantiates its own client. Did you have a particular reason to avoid subclassing django.test.TestCase class which does just that?
  • The test case sets up several articles with different dates (past present and future) but calls them just Article 1, 2, 3 etc. It would be more illustrative (maybe enough to remove the comment) to call them "Past article", "Today's article 1" and "Article from the Future". (or something)
  • Is counting the number of articles returned a sufficient test? It probably is.
  • The methods on the test case class have camelCase names, instead of Django's normal lowercase_with_underscores standard.
  • I see you had to use the real now() method, which could cause spurious errors if buildbot runs the test just before midnight. With the current code, I don't think there's a nice way around this. :-(
  • Your patch contains a HTML 4 "transitional" template, with content that is never used. How about just an empty file?

09/14/07 15:42:27 changed by Rob Hunter <robertjhunter@gmail.com>

By the way congratulations writing the first test for generic views! I'll have to settle for second place ;-)

02/21/08 15:33:04 changed by SmileyChris

  • keywords changed from sprintsept14 to sprintsept14 easy-pickings.

Add/Change #1282 (archive_today generic view should accept a month_format parameter instead of hardcoding '%b')




Change Properties
Action