[drupal-devel] [feature] Replace core archive.module w/ codemonkeyx
archive.module
Morbus Iff
drupal-devel at drupal.org
Wed Sep 14 14:19:16 UTC 2005
Issue status update for
http://drupal.org/node/29676
Post a follow up:
http://drupal.org/project/comments/add/29676
Project: Drupal
Version: cvs
Component: archive.module
Category: feature requests
Priority: normal
Assigned to: Morbus Iff
Reported by: Morbus Iff
Updated by: Morbus Iff
Status: patch (code needs work)
Attachment: http://drupal.org/files/issues/archive_5.module (11.18 KB)
Grr.
Morbus Iff
Previous comments:
------------------------------------------------------------------------
Thu, 25 Aug 2005 21:08:49 +0000 : Morbus Iff
Over at http://drupal.org/node/8287, Berkes mentions that the core
archive.module was considered being removed, per a discussion at the
Drupal Sprint. Kjartan also mentions he would "love to have the archive
module improved in general." In chatting with chx about this, he
mentioned codemonkeyx's rewrite sitting in contrib/modules/archive/.
I'll be doing some work with the archive.module over the next few days,
and will be basing my changes around codemonkeyx's version, and making
it compatible with HEAD. This general Issue is to move codemonkeyx's
version into HEAD as a replacement to the existing archive.module. An
unknown version of his replacement can be seen at
http://www.codemonkeyx.net/archive. I'll be running a live HEAD version
soon as well.
These patches were made during the customization of Drupal by
http://www.NHPR.org. In loving support of open source software,
http://www.NHPR.org will continue to contribute patches they feel the
community will benefit from. Questions about this patch should be
directed to morbus at disobey.com.
------------------------------------------------------------------------
Fri, 26 Aug 2005 19:45:59 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues_2 (9.56 KB)
As an example of a very early revision, see the attached, with the
following changes from the current contrib CVS:
* removed the year offset from theme_archive_navigation_years, which
controlled how many year links to show at once in the top nav. For
those with sites with more than five years, they'll WANT people to
notice that they have five years, not to have to click on the earliest
date and then have their expectations changed.
* made the "created > $date" in archive_buildQuery "created >= $date"
instead, to allow posts that were created at exactly midnight that day
(like me, by design).
* since there's no block, I made the menu item visible upon first load.
this menu item is given "access content" permissions.
More to come, including doxygen and gmt considerations.
------------------------------------------------------------------------
Fri, 26 Aug 2005 19:47:41 +0000 : Morbus Iff
Might as well start getting a review of it so I can fix 'em as they come
in.
------------------------------------------------------------------------
Fri, 26 Aug 2005 19:56:49 +0000 : Tobias Maier
cant you provide a patch file?
thanks for your work
------------------------------------------------------------------------
Fri, 26 Aug 2005 20:04:01 +0000 : Morbus Iff
The codemonkeyx version is a complete rewrite of the core
archive.module. If I were to create a patch file against core, every
line would be deleted, and every line would be new. Once I finish my
revisions to codemonkey's version, I'll post the final version here, as
well as a patch against his current CVS.
------------------------------------------------------------------------
Fri, 26 Aug 2005 20:09:13 +0000 : Tobias Maier
ok, thanks again :D
------------------------------------------------------------------------
Mon, 29 Aug 2005 13:41:43 +0000 : Junyor
+1 for this change. The archive.module in core is dead.
------------------------------------------------------------------------
Mon, 29 Aug 2005 16:14:30 +0000 : adrian
What is the progress on this morbus ?
------------------------------------------------------------------------
Mon, 29 Aug 2005 16:29:13 +0000 : Morbus Iff
Adrian - I'll be attaching a new version either later today or tomorrow,
with a CHANGELOG. I'll also be running a live version of it over on
NHPR.org for people to play with. The three major things I'm worried
about right now are a) doxygen, b) variable/function naming, c) GMT
considerations. After those, I'll be exploring a patch for my own
needs: the ability to get archives for particular term.
------------------------------------------------------------------------
Mon, 29 Aug 2005 18:53:34 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/archive.module (9.93 KB)
Here's the latest, with the following changelog:
* reordered some routines to be a little more workflowish.
* renamed archive_buildQuery to archive_build_query.
* general whitespace and formatting cleanup.
* HEADish update: returning $output, not page templating it.
* removed the reference of &$ad in archive_build_query.
* test for the existence of arg(#)'s before validating them.
* archive_validSomething changed to archive_valid_something.
* removed unused vars: cur_date, cur_date_end.
* renamed archive_buildURL to archive_build_url.
* removed the HTML whitespace from the theming.
* twiddled a lot of quotes and apostraphes.
* removed 'future' CSS class. ill-defined.
* reordered/renamed the CSS classes.
------------------------------------------------------------------------
Mon, 29 Aug 2005 18:54:08 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/_p_29676_archive_css.patch (1.56 KB)
And the drupal.css patch.
------------------------------------------------------------------------
Mon, 29 Aug 2005 18:56:29 +0000 : Morbus Iff
This version of the module is currently running live at
http://www.nhpr.org/archive/.
------------------------------------------------------------------------
Mon, 29 Aug 2005 19:59:57 +0000 : Tobias Maier
if i click for example on 2003 it would be good if this would go to
january or december
and marks them that this one will be shown now
as you can see it if you click on january 2003.
it has to select
* on the first:
the first month of writing
* on the last:
the last month of writing
* on every else:
january
I hope you can understand what I mean...
greets tobias
------------------------------------------------------------------------
Mon, 29 Aug 2005 20:17:40 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/archive_0.module (11.28 KB)
Alright. I've attached another new version that adds a new feature that
wasn't part of the original codemonkeyx CVS, but was chatted about on
the devel list back in April. If this particular feature has bad code
or needs heavy refactoring, certainly consider ONLY the version in
comment #9 (and the matching drupal.css patch in #10).
This new version supports dated archives based on taxonomy tids. It was
a quick addition which NHPR.org needed (for the date nav; the normal tid
archive pager wasn't strong enough for our needs). Since it was a quick
addition, it supports only ONE tid at a time - the 'and/or' syntax for
the taxonomy.module was not brought over. If that syntax was desired,
it'd make more sense to create some sort of API for archive.module so
that other nodes can take advantage of the dated nav in their normal
pages (like node types, users, forums, etc.)
The added code supports term matches at any granularity:
# all node types that match tid 15000 ('The Front Porch')
http://www.nhpr.org/archive/term/15000
# all 2005 node types that match tid 20 ('Health')
http://www.nhpr.org/archive/2005/term/20
# all March, 2003 node types that match tid 9 ('Education')
http://www.nhpr.org/archive/2003/3/term/9
# all July 11, 2003 node types that match tid 49 ('Economy')
http://morbus.totalnetnh.net/nhpr/archive/2002/7/11/term/49
------------------------------------------------------------------------
Mon, 29 Aug 2005 20:27:26 +0000 : Tobias Maier
what does this mean?
"Story Archives of 'archives'
"
on http://www.nhpr.org/archive/term/15000
should this maybe named?
"Archive of 'The Front Porch'"
if I go to http://www.nhpr.org/archive/term/20
I can only read "archives"...
why is there a difference?
- I never tested this module on my test site, because I'm not at home
-
------------------------------------------------------------------------
Mon, 29 Aug 2005 20:29:34 +0000 : Morbus Iff
Tobias: that wouldn't be possible, at least not accurately. The new
archive.module supports browsing by year, month, and day, as you know.
archive/2005 loads up all the data from a particular year and starts
creating a pager out of it. Consider if you have 3 posts in December,
and 15 posts in November. It wouldn't be "right" to highlight December
because the pager display for the entire year would also include some
of November's entries (since 3 is less than the pager increment).
Likewise, if we ONLY showed the items from December, then we wouldn't
have a "pager by year" feature, only a "pager by month (defaulting to
December when none is selected)" feature.
------------------------------------------------------------------------
Mon, 29 Aug 2005 20:30:41 +0000 : Morbus Iff
Tobias: regarding #14, that's an artifact of the templates that I'm
using for NHPR, and has nothing to do with archive.module itself (in
fact, once the anonymous cache expires, you'll see that little oopsie
go away).
------------------------------------------------------------------------
Mon, 29 Aug 2005 20:31:47 +0000 : Tobias Maier
I can see your right :)
I hope it comes in HEAD before tomorrow :D
------------------------------------------------------------------------
Tue, 30 Aug 2005 00:20:12 +0000 : dtan
I apologize if this is already a known issue.
http://www.nhpr.org/archive/2005/9 does not create a link for september
1st, even though there are 2 nodes listed
(http://www.nhpr.org/archive/2005/9/1)
------------------------------------------------------------------------
Tue, 30 Aug 2005 17:11:21 +0000 : Morbus Iff
dtan: I'm pretty sure I know what this is - I'll address it either later
today or tomorrow.
------------------------------------------------------------------------
Tue, 30 Aug 2005 20:49:58 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/archive_1.module (11.53 KB)
Alright, I've attached a new archive.module - this is the version WITH
term filtering enabled. I can make one without terms if necessary -
otherwise, I'll just work from this one for now. This version fixes the
bug that dtan saw, as a well as a bunch of other off-by-one errors. Of
primary importance, however, is that all mktime's that mattered have
been switched to gmmktime, which was one of the oft-reported Issues
with the old archive.module. I want to eyeball them all again and make
sure they're right though.
The URLs from #13 are still operational and the CSS from #10 is still
required.
------------------------------------------------------------------------
Wed, 31 Aug 2005 00:02:06 +0000 : Morbus Iff
In testing with a few people in #drupal, we've discovered a much bigger
problem, which affects this rewritten module as well as the current
core archive.module. In a nutshell, the node.created time is stored
with time(). PHP's time() bases itself on the server time, NOT on GMT.
Thus, for archive.module to work correctly, it must ALWAYS use mktime
(relative to server time) and never consider the $user->timezone
(relative to GMT). For archive.module, this would cause dates to always
be considered via server time, which isn't good, but is better than the
craziness going on now. Alternatively, we could try to convert server
time to GMT first, and then work with that.
The proper solution is to fix node.module to use gmmktime without any
arguments for node.created, then have an update path that modifies all
node .created and .modified values to GMT, not server time.
------------------------------------------------------------------------
Wed, 31 Aug 2005 05:53:51 +0000 : Junyor
Since that is also a problem with the old archive.module, I don't see
why it should stop this from getting into core.
------------------------------------------------------------------------
Wed, 31 Aug 2005 06:37:28 +0000 : stefan nagtegaal
"Since that is also a problem with the old archive.module, I don't see
why it should stop this from getting into core.
"
Well, I think it's better to only accept the best code rather than
accepting bugs getting in core.
------------------------------------------------------------------------
Wed, 31 Aug 2005 07:05:51 +0000 : Kobus
I am with Junyor on this. If this can be fixed, great, but if not, it's
not a train smash, as the old one exhibits the same problem.
I say add the new archive module, if there are no other ciritical bugs
with it. It is much more robust and usable than the old one. We
desperately need a new archive module.
I couldn't find any other bugs while testing with the links Morbus
posted. I don't have a HEAD installation anywhere, so can't test it
locally at this moment.
Regards,
Kobus
------------------------------------------------------------------------
Wed, 31 Aug 2005 07:07:49 +0000 : Kobus
BTW, code freeze means no new code added, right?
Can't this module put in Core as is for the code freeze and the bug
sorted out before the official release? Or is that just mean of me to
suggest that?
Kobus
------------------------------------------------------------------------
Wed, 31 Aug 2005 07:13:09 +0000 : Junyor
Stefan: The bug is already in core, since node.module is in core.
Archive.module (old and new!) just shows that bug.
------------------------------------------------------------------------
Wed, 31 Aug 2005 07:20:35 +0000 : Tobias Maier
I want to have it in core for 4.7, too!!!
------------------------------------------------------------------------
Wed, 31 Aug 2005 07:25:24 +0000 : stefan nagtegaal
Junyor: I know that.. Though I think it's not good to accept code which
we are aware of that it has bugs in it..
Offcourse this is very double, because drupal contains (probably) a lot
of bugs, only they weren't spotted yet..
But, accepting code which has bugs in unacceptable imo..
For example, the node revisions patch had almost 40 reviews/rewritings
from Gerhard and several others before it was accepted to be in core..
If we do not allow bugs to go into core, we don't have to bughunt and
fix later which is a good thing..
So, imo we should first sort out the problem, then discuss what the
best way is to fix the problem, and after that Squeeze that moron! ;-)
------------------------------------------------------------------------
Wed, 31 Aug 2005 09:11:06 +0000 : adrian
I vote we include this module, and open up a new issue for the bug.
It's not archive's fault that this occurs, it is just showing the data
it has access to. The bug already exists in core too.
------------------------------------------------------------------------
Wed, 31 Aug 2005 13:50:59 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/archive_2.module (10.63 KB)
Attached is a new, and most likely final, version of the archive.module.
I've removed all user->timezone references - all date determinations are
based on server time, which is also the time used in the "created"
column of the node table. This is "more accurate" than what the core
archive.module currently does (which'll always be wrong because it's
treating server time as GMT, which isn't always the case). When
node.module does start saving times as GMT properly, archive.module
will have be to be tweaked with timezones and blah blah blah. I did
fiddle around with determining the server offset in an attempt to get
to a GMT base, but I didn't have much luck with that. The NHPR.org URLs
above are still valid for testing.
I need testers and reviewers.
------------------------------------------------------------------------
Wed, 31 Aug 2005 14:55:16 +0000 : m3avrck
I get these two PHP errors when I have *no* content in my Drupal install
(just did a clean install to test it):
warning: mktime() [<a href='function.mktime'>function.mktime</a>]:
Windows does not support negative values for this function in
websites\drupal_cvs\drupal\modules\archive.module on line 274.
warning: date() [<a href='function.date'>function.date</a>]: Windows
does not support dates prior to midnight (00:00:00), January 1, 1970 in
websites\drupal_cvs\drupal\modules\archive.module on line 274.
Once I add content, these go away. Need some better checks to make sure
if there is *no* content you don't get weird errors and what not :)
------------------------------------------------------------------------
Wed, 31 Aug 2005 14:55:45 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/_p_29676_archive_css_0.patch (1.56 KB)
Updated drupal.css patch for HEAD.
------------------------------------------------------------------------
Wed, 31 Aug 2005 15:26:01 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/archive_3.module (10.75 KB)
Attached is the complete module, fixed for m3avrck's #31.
------------------------------------------------------------------------
Wed, 31 Aug 2005 17:30:36 +0000 : m3avrck
Reviewed patch and further test, running great over here! Definetly +1
for this one!
------------------------------------------------------------------------
Wed, 31 Aug 2005 20:03:36 +0000 : Morbus Iff
Note: The NHPR folks preferred their archives to be sorted from earliest
to latest (SQL ASC vs. DESC) for month/day listings, so no longer use
the NHPR.org URLs above as representative of the module itself.
------------------------------------------------------------------------
Wed, 31 Aug 2005 21:26:37 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/drupal.css (10.35 KB)
CVS updated for HEAD again.
------------------------------------------------------------------------
Wed, 31 Aug 2005 21:27:00 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/_p_29676_archive_css_1.patch (1.56 KB)
GRRR.
------------------------------------------------------------------------
Thu, 08 Sep 2005 20:18:48 +0000 : m3avrck
Attachment: http://drupal.org/files/issues/archive_0.patch (20.72 KB)
Rerolled patch against head. Also created one patch that fixes both
archive.module and drupal.css so it's a clean and simple process now :)
------------------------------------------------------------------------
Thu, 08 Sep 2005 20:23:55 +0000 : m3avrck
Just retested with the latest patch I made, applies cleanly against HEAD
and *everything* works great and looks great too. No errors of any kind,
even works on PHP 5.0.5 with no call by reference errors, :D Let's get
this baby in!
------------------------------------------------------------------------
Thu, 08 Sep 2005 21:26:06 +0000 : Souvent22
+1. I like the new patch, makes things easier to patch. Very slick. I
like the break down of years, to months, to days. Very nice. Ready to
go I say.
------------------------------------------------------------------------
Mon, 12 Sep 2005 18:55:56 +0000 : m3avrck
Bump to the top, freeze coming soon!!!
------------------------------------------------------------------------
Tue, 13 Sep 2005 18:05:21 +0000 : Cvbge
So, pgsql does not have FROM_UNIXTIME nor MONTH functions...
I'd prefer, if it is possibile, to change the query to not use this
mysql-specific functions, then writing pgsql specific equivalents...
------------------------------------------------------------------------
Tue, 13 Sep 2005 18:39:20 +0000 : Dries
It might be me, but visually, I like the little calendar block better.
Though, this new (slightly ugly) widget is easier to navigate.
I'd like to postpone this patch until after 4.7. While nice, I'm not
convinced this is the right approach. Like, adding the term-magic to
the archive module is more of a hack than anything else. What's next,
adding a uid-check so you can filter by user ID? Or a type check so
you can filter by node type? Ultimately this should be merged into the
tracker module, or better yet, into a generic "query string to node
query"-mechanism. (I've outlined this elsewhere in the issues.)
There is also a bunch of very minor code issues that need to be looked
at.
------------------------------------------------------------------------
Tue, 13 Sep 2005 19:06:57 +0000 : Morbus Iff
Attachment: http://drupal.org/files/issues/archive_4.module (11.18 KB)
I've attached an updated version, which includes gmmktime +
user->timezones, as well as changing all date()'s to format_date()'s.
As for nodetypes/users, actually, yeah, NHPR.org is interested in those
filter functionalities too (and actually wanted them a few weeks ago).
Haven't yet addressed the PSQL stuff from Cvbge.
------------------------------------------------------------------------
Tue, 13 Sep 2005 19:09:46 +0000 : Morbus Iff
I'm not entirely convinced that this should be part of tracker.module -
the tracker module is to "track changes" or to "track updates". That's
an entire different problem than archived data, which probably hasn't
been changed for months or even years. I'd hate to see the tracker view
applied to an archive system - I could care less about the titles of a
blog post (which are usually piss-poor), especially when most *entire*
blog posts are small enough to fit into the teaser that the
archive.module shows.
------------------------------------------------------------------------
Wed, 14 Sep 2005 05:45:57 +0000 : Boris Mann
Dries, I agree with Morbus.
I can see what you're thinking with tracker (I think) -- have it handle
read/unread for all sorts of stuff, in which case you'll want all sorts
of filters.
Archive is the place where all content can be navigated primarily by
date. Let's commit this rather than live with the current completely
useless archive.module for another entire release cycle.
------------------------------------------------------------------------
Wed, 14 Sep 2005 06:45:23 +0000 : Bèr Kessels
Dries, I can see your point.
The navigation interfce feels odd. But after some time I found this UI
to be VERY good. it just feels odd because you are used to a calendar,
yet this really is a better interface for browsing dates. It requires
less clicks, offers all possible options in one glance etc.
If we don't go for this archive module. Then please pretty please,
remove at least the other one from core. It is completely useless as
Boris states Better that people must use contribs than something that
makes us look like we code crappy modules for core.
------------------------------------------------------------------------
Wed, 14 Sep 2005 08:14:06 +0000 : Kobus
It would be very sad indeed if this is not included in 4.7! I will
definately replace it on all my local sites though.
I understand the concern that the new widget is not as pretty as the
old one, but in my opinion it is the "resistance to change" that speaks
here. We're so used to think that an archive is square-ish, that this
wide rectangle is a bit awkward at first.
The gain in usefulness and easier navigation is totally making up for
that. It also gives a clearer overview of a time-frame than is
currently the case, so all in all, I am +1 for this.
Regards,
Kobus
------------------------------------------------------------------------
Wed, 14 Sep 2005 08:17:51 +0000 : stefan nagtegaal
Because I would like to to see this hit trunk as much as you all, I'll
volunteer that _after_ this is in, I'll have a look if we can
refactor/spice up the default looking of the new archive.module..
So Dries, any other concerns you have left? ;-)
------------------------------------------------------------------------
Wed, 14 Sep 2005 13:44:14 +0000 : m3avrck
I agree with said points, this module really cleans things up and works
so much better. Let's get this in now and we still have a few weeks
before 4.7 ships. We can deal with any outstanding bugs then... not
that these are really any *new* bugs since they already exist in CVS.
Not to mention this also fixes a slew of bugs with the current
archive.module, so based on that, I think this one should be included.
------------------------------------------------------------------------
Wed, 14 Sep 2005 14:18:05 +0000 : Morbus Iff
Very minor changes in this version: slightly different comments and a "
to ' conversion.
More information about the drupal-devel
mailing list