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

[anonymous]

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.

[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.

[derelm]

revision 2912 introduces translation hooks for MONTHS_3.

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

[(none)]

Milestone Version 1.0 deleted

[Jannis Leidel <jl@…>]

make archive_today() respect month_format

[Gary Wilson]

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.

[Nick Efford]

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

[Nick Efford]

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 :)

[Nick Efford]

Improved version of original patch

[Nick Efford]

Beginnings of some unit tests for generic views

[Rob Hunter <robertjhunter@…>]

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?

[Rob Hunter <robertjhunter@…>]

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

[Nick Efford]

I've refreshed the patch for latest (post nfa merge) trunk, so that the tests now build on the existing test infrastructure for generic views.

Note: in addition to applying this patch, it is also necessary to create the template file article_archive_day.html in tests/templates/views. The file can be empty.

[Nick Efford]

Updated patch

[Nick Efford]

Hmm, patch seems to be confusing Trac. Inspecting the diff, I can see the following, which might be the source of the problem:

\ No newline at end of file

[Chris Beaven]

Nick, it looks like you forgot to add views/article_archive_day.html in your patch. I'll put a new one up for you

[Chris Beaven]

I should really read notes - I see that you mentioned that already in your previous patch. I just did a few other minor tweaks to the patch so mine is still good (and contains the new template file too).

So in review, the patch is good to go.

[Chris Beaven]

The tests in this patch would be useful for #7944, too.

[Adam Nelson]

Patch is missing (is this a problem with all the older uploads?) and anyway, I'm sure in the intervening 2 years this patch would not apply cleanly.

[Karen Tracey]

What do you mean by patch is missing? If you mean it looks empty when clicked on, that's a bug in trac (I believe) where patches with "\ No newline at end of file" don't display properly. You can still download the patch and it's fine. I also would not necessarily assume that an old patch won't apply any more; depending on the area of code touched it may still be OK. It's probably not helpful to move things back from "Ready for checkin" to accepted just because the patch is old, unless you actually verify that the patch no longer applies.

[Adam Nelson]

Hm, wasn't aware of that bug in Trac. Anyway, the patch doesn't apply cleanly on the regressiontests, django/views/generic/date_based.py applies fine.

[Adam Nelson]

Patch updated as of rev:13294

[Eric Holscher]

This seems like a useful patch. I think the only thing missing is documentation in the generic views docs that it accepts this parameter and it should be RFC.

[Jannis Leidel]

This is probably going to be fixed by the latest proposed patch for #6735.

[Russell Keith-Magee]

Function-based generic views were deprecated by the introduction of class-based views in [14254]. Class-based views should solve this problem.