Archive module replacement for core

cant you provide a patch file?
thanks for your work

ok, thanks again :D

+1 for this change. The archive.module in core is dead.

What is the progress on this morbus ?

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

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 -

I can see your right :)

I hope it comes in HEAD before tomorrow :D

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)

Since that is also a problem with the old archive.module, I don't see why it should stop this from getting into core.

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.

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

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

Stefan: The bug is already in core, since node.module is in core. Archive.module (old and new!) just shows that bug.

I want to have it in core for 4.7, too!!!

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! ;-)

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.

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

Reviewed patch and further test, running great over here! Definetly +1 for this one!

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

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!

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

Bump to the top, freeze coming soon!!!

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

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.

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.

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.

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

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? ;-)

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.

What do I do wrong? I installed the 'archive_5.module' from the post before. But when I call 'http://www.examplesite.com/archive' all I get is a blank page.

Here's a patch (new archive module and CSS rolled into one). Updated against current HEAD. Still working great, let's get this in and get rid of the useless one there :)

What do you mean by HEAD? I'm running the 4.6.3 version of drupal. There is no error. I simply get a page with the following source code

html body /body /html

Hmmmpfff. Ok, sorry for disturbing. I'm out of here ... ;-)

OK, let me explain what's the deal with postgresql.

Creation time of a node is kept as an integer - number of seconds since 1970.
Given a year, for example 2004, archive module needs to get a list of months in which there were any posts. So if there were posts in January and February, it needs to get 1 and 2. So it needs to extract month from the integer representing node creation date.
Currently it does it by doing SELECT MONTH(FROM_UNIXTIME(created)). Both MONTH() and FROM_UNIXTIME() are mysql-specific functions, not in SQL standard and not implemented in PostgreSQL.
The Postres (and I hope the SQL standard way) is to do select extract(month from (select timestamp with time zone 'epoch' + created * interval '1 second')).

MONTH() seems to be identical to EXTRACT(MONTH FROM ...), can someone check it? If that's the case the only problem is FROM_UNIXTIME...

So what to do?
One solutions, the best IMO, would be to rewrite code in the way that it works both on mysql and postgres. Currently I have no idea how to do it.
The other solution is to implement FROM_UNIXTIME() [and MONTH()?] in postgres... Issues with this approach:

• so far FROM_UNIXTIME() is not used at all in core code, nor in contrib modules!, and I'd prefer it to stay like that. Less mysql-specific functions is better. MONTH() is not used in core, but used in several contrib modules.
• from mysql docs http://dev.mysql.com/doc/mysql/en/date-and-time-functions.html :
  

  Beginning with MySQL 4.1.3, the CURRENT_TIMESTAMP(), CURRENT_TIME(), CURRENT_DATE(), and FROM_UNIXTIME() functions return values in the connection's current time zone, which is available as the value of the time_zone system variable.

  IMO it means that this functions can return different values depending if mysql server is newer or older then 4.1.3.

• Returns a representation of the unix_timestamp argument as a value in 'YYYY-MM-DD HH:MM:SS' or YYYYMMDDHHMMSS format, depending on whether the function is used in a string or numeric context.

  AFAIK this can not be implemented in postgresql.

• The optional format argument makes it even messier, be we could not implement it so it's ok more or less.

whats the currents status of this?
do we get it in for 4.7 or not?

greets tobias

We need code that works both in PostgreSQL and MySQL.

Also, 'archive' should not be abbreviated to 'arch'.

Thanks for the module -- I couldn't get the core archive.module to do anything but display the calendar block. Every link was dead. Yours is different in format but works like a charm. Thanks again!

Perhaps if there is resistance to replacing the core module, this could be made as a contrib module and enabled as an alternate to the core archive.module?

OK, I'll quit banging on this thread in a second, but under the 10/19/2005 version of HEAD, I'm getting a one-off error for the date widget. E.g., all my posts are in October, but the archives widget lists has only a link for "Sep". In addition, when I click on, for example, "Sep" then "19", the actual URL is foo.org/archive/2005/10/19

Sweet bejeebus! Let's not ship 4.7 with b0rked core archive. Replace! Replace! I'm going to get this in...

+1 does anyone want to volunteer to test postgres patches?

Hi, this looks really good!

I'm running 4.6.3, has anyone adapted this to work on 4.6.3 or done a similar archive by month for 4.6.3??
i've tried afew things to achieve this but none of them seem to work.

thanks

Tom

I vote +1 to add this to 4.7 in place of the current calendar (or at least as an alternative).

Will this code work in 4.6.x?

I have a better question for you:
does it work with the current head (upcoming 4.7)?
I give you the answer: no, because it lacks some conversions which happened since september (forms api mainly)
so if you want to have this in core (this does not mean 4.7) then you have to convert it that it runs on drupal 4.7 AND that it runs on postgresql

I have just updated the archive module code which is in CVS for Drupal HEAD/CVS. Then came here to see that there was more functionality suggested but then voted down. Looking at #56, the CVS version of the contrib archive module provides nearly all functionality, in some aspects is more Drupal style friendly (not yet completely). The only thing it does not provide is the taxonomy restriction which was not liked among those who commented.

It is a standalone module now (no need to patch anything) and can be tried on Drupal 4.7 or HEAD. It should work.

Two issues still stand: PgSQL compatibility with the month query (apart from this single query, the module should be PgSQL compatible IMHO); proper timezone support (this might have been done better in #56.

More to the point title. The development version of the module is here: http://cvs.drupal.org/viewcvs/drupal/contributions/modules/archive/

I Tested this with 4.7.2 and it doesn't quite work. When click on the "archives" menu, the number of post counts for each day seem to be correctly. (for example, if 7/13/2006 has 20 posts and 7/14/2006 has 15 posts, then the count shown is correct). However, when I click on the link to show these posts, ALL posts are shown (7/13 plus 7/14), which is wrong.

Can someone please fix this for 4.7.2+ ? :)

Thanks a bunch.

@vph: Indeed there is a small bug with parameter passing in the module's HEAD code. It will get fixed soon. I have already tackled the postgres compatibility part, and I am on the timezone handling fixup. So a new better module should be in CVS soon.

Thanks.

About the timezone mess up, I realized that too. I found a solution that works for me. The code

start_of_day = mktime(0,0,0,$month,$day,$year) - $user->time_zone;
end_of_day = mktime(0,0,0,$month,$day+1,$year) - $user->time_zone;

is essentially incorrect. The following, however, seems to be correct for me:

start_of_day = mktime(-4,0,0,$month,$day,$year) - $user->time_zone;
end_of_day = mktime(-4,0,0,$month,$day+1,$year) - $user->time_zone;

For example, start of 7/19 would be at 7:19 0h,0m,0s and end of 7/19 would be at 7:20 0h,0h,0s.

vph, does this patch (for CVS version of contributions/modules/archive/archive.module rev 1.2) work for you?

Timezone problem could not be solved if we use mktime() and date() function which depend on system (PHP) timezone, IMHO. We should use gmmktime() and gmdate() with proper adjustment for user timezone.

Now, I wrote new patch for archive.module rev 1.2 in CVS (contributions/modules/archive/archive.module).

This patch contains:

• Correct handling of timezone.
• _archive_months() is rewritten with PostgreSQL compatible, timezone aware method.
• Fix range of 'created' in _archive_query(), don't contain 00:00 on the next day.
• Fix parameter order of function call _archive_validate_day().

New version of _archive_months() may not be efficient as older version,
because this execute 12 query per page. But this would not make
things slow significantly, and this would work with PostgreSQL.

@oishi: as I have said, I have a version, which should work with pgsql, and is not using 12 SQL queries instead of one. I am also confident that BETWEEN is supported by pgsql. Please prove me wrong, if this is not the case. I am looking into the date stuff posted by Morbus and you, and will test soon. I have not committed the updated module to CVS yet because of the date problems I am still tackling with (testing with posts with different interesting timestamps).

Morbus, I'm not sure what you write in #21. Function time() returns number of seconds from '1970-01-01 00:00:00 +00:00' (UTC) as PHP manual says, does not it?

I have some acounts on different Linux systems, time() function returns same value (seconds from epoch) on every system.

• Red Hat Linux 7.3, kernel 2.4.18, PHP 4.3.1, JST(+0900)
• Red Hat Enterprise Linux ES 4, kernel 2.6.9, PHP 4.3.9, JST(+0900)
• Debian GNU/Linux sid, kernel 2.6.17.1, PHP 4.4.2, JST(+0900)
• Debian GNU/Linux sid, kernel 2.6.17.3, PHP 5.1.4, JST(+0900)

(Unfortunately, timezones of all systems I can access are Japan...)

When I run this code (in +0900 system),

$t = time();
echo $t, "\n";
echo "Localtime: ", date("Y-m-d H:i:s", $t), "\n";
echo "UTC time: ", gmdate("Y-m-d H:i:s", $t), "\n";

output is

1153702745
Localtime: 2006-07-24 09:59:05
UTC time: 2006-07-24 00:59:05

time() value 1153702745 is seconds from '1970-01-01 00:00:00 +00:00', not from '1970-01-01 00:00:00 +09:00'. [1153702745 % (60 * 60* 24) = 3545. 3545 sec is 0h 59m 5s. This is current UTC time, not localtime. ]

Any problem?

Goba:

BETWEEN is supported by PostgreSQL of course, problem is not a compatibility issue. When I write SQL "created BETWEEN $start AND $end", this mean "created >= $start AND created <= $end", both of $start and $end are inclusive.

Original version of _archive_query():

function _archive_query(&$date) {
  global $user;

  if ($date->day > 0 && $date->month > 0) {
    // Confine the display interval to only one day
    $start = mktime(0, 0, 0, $date->month, $date->day, $date->year) - $user->timezone;
    $end   = mktime(0, 0, 0, $date->month, $date->day + 1, $date->year) - $user->timezone;
  }
  elseif ($date->month > 0) {
    // Confine the display interval to one month
    $start = mktime(0, 0, 0, $date->month, 1, $date->year) - $user->timezone;
    $end   = mktime(0, 0, 0, $date->month+1, 0, $date->year) - $user->timezone;
  }
  else {
    // Confine the display interval to one year
    $start = mktime(0, 0, 0, 1, 1, $date->year) - $user->timezone;
    $end   = mktime(0, 0, 0, 1, 0, $date->year+1) - $user->timezone;
  }

  return array('SELECT nid, type FROM {node} WHERE status = 1 AND created BETWEE
N %d AND %d ORDER BY created DESC', $start, $end);
}

When I specify '2006/07/24', $start time is '2006-07-24 00:00:00' and $end is '2006-07-25 00:00:00'. So node which created in '2006-07-24 00:00:00' is displayed both day '2006/07/23' and '2006/07/24'. My intention of avoid BETWEEN here is there.

In addition, (this is not BETWEEN problem) when '2006/07' is specified, nodes created in last day of July is not selected. When '2006' is specified, last day of 2006 (Dec 31) is not selected.

My change in _archive_months() is mainly timezone fix. I don't have other idea to handle timezone correctly using PostgreSQL compatible way. (I can't see your working version of _archive_months()...)

After deeper investigation on how Drupal handles node dates, and consequently how should archive module behave, I fixed the contrib archive module (also consulting oishi's patch, and used most of it). I have a different solution for postgresql, in which we teach from_unixtime() to postgresql.

I have tested the new module with esoteric timezone settings (the server, Drupal and user timezones were way different), nodes sent on corner dates and such, and it seems to work fine. Please test.

My commit: http://drupal.org/cvs?commit=37193

Ps. I am unsure whether this should go into core, but it will not get decided be me.

Thank you for working.

I found some problem still remaining in _archive_query() and _archive_months().

_archive_query():

• If only year specified, this function does not return nodes on last day of the year. If only year and month specified, this does not return nodes on last day of the month. These are because of miss-specified 'day' parameter of gmmktime for end time.
• In WHERE clause, end condition of 'created' must be exclusive.
  (Use 'created >= %d AND created < %d' instead of 'created >= %d AND created >= %d'.)
• Is '$user->timezone' needed?

So I think this function should be:

function _archive_query($date) {
  if ($date->day > 0 && $date->month > 0) {
    // Confine the display interval to only one day
    $start = gmmktime(0, 0, 0, $date->month, $date->day, $date->year);
    $end   = gmmktime(0, 0, 0, $date->month, $date->day + 1, $date->year);
  }
  elseif ($date->month > 0) {
    // Confine the display interval to one month
    $start = gmmktime(0, 0, 0, $date->month, 1, $date->year);
    $end   = gmmktime(0, 0, 0, $date->month+1, 1, $date->year);
  }
  else {
    // Confine the display interval to one year
    $start = gmmktime(0, 0, 0, 1, 1, $date->year);
    $end   = gmmktime(0, 0, 0, 1, 1, $date->year+1);
  }

  return array('SELECT nid, type FROM {node} WHERE status = 1 AND created >= %d AND created < %d ORDER BY created DESC', $start - $date->timezone, $end - $date->timezone);
}

_archive_months():

This fuction returns months using SQL 'EXTRACT(MONTH FROM FROM_UNIXTIME(created))', but returned month does not match for user timezone because FROM_UNIXTIME returns time in local (DB server's) timezone.

I suggest to use this version. This would not be so efficient (sorry...), but work. We can tune SQL anytime, later.

function _archive_months($date) {
  $months_with_posts = array();
  for ($month = 1; $month <= 12; $month++) {
    $start  = gmmktime(0, 0, 0, $month, 1, $date->year) - $date->timezone;
    $end    = gmmktime(0, 0, 0, $month+1, 1, $date->year) - $date->timezone;
    $result = db_query('SELECT COUNT(*) count FROM {node} WHERE status = 1 AND created >= %d AND created < %d', $start, $end);
    if (($row = db_fetch_object($result)) && $row->count > 0) {
      $months_with_posts[] = $month;
    }
  }
  return $months_with_posts;
}

@oishi: thanks for the detailed writeup. The $user->timezone stuff was not intended there. But the others were intended and indeed improperly written. Now patched the code mostly with your suggested fixes. Tested with differing server/site/user timezone settings, posts on the edge of months, and it seems to work nicely. New version in CVS.

+Also fixed node access problems, using db_rewrite_sql

Thank you. This version works well for me.

Since there was no actual interest in this replacement for the core archive module, and neither the existing one in core, archive module was removed from core. I have created a project, which now has a distinct issue queue, and has all the cvs messages and such to overview.

That module is now the place to add possible improvements to functioanilty. As I indicated, I might be interested in adding a calendar view for a block, which was previously available in core. The problem with it is it needs to be cached by user, since otherwise user/node level permissions are not possible to take care of.

Reassigned this issue to the new archive module, so we will be able to easily find this discussion later. Marking this fixed. Open new issues for new problems please.