Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.In function aggregator_parse_feed() the publication date / timestamp is derived out one of several fields. Sometimes a feed does contain none of these fields. The consequence is the item is created several times, each with the current date. The real problem is, these items are always shown on top.
This can be easily solved by replacing the timestamp with the timestamp from an earlier record of the item. I don't know how to create a diff, so I paste my current code:
// Save this item. Try to avoid duplicate entries as much as possible. If
// we find a duplicate entry, we resolve it and pass along its ID is such
// that we can update it if needed.
if (!empty($guid)) {
$entry = db_fetch_object(db_query("SELECT iid, timestamp FROM {aggregator_item} WHERE fid = %d AND guid = '%s'", $feed['fid'], $guid));
}
else if ($link && $link != $feed['link'] && $link != $feed['url']) {
$entry = db_fetch_object(db_query("SELECT iid, timestamp FROM {aggregator_item} WHERE fid = %d AND link = '%s'", $feed['fid'], $link));
}
else {
$entry = db_fetch_object(db_query("SELECT iid, timestamp FROM {aggregator_item} WHERE fid = %d AND link = '%s'", $feed['fid'], $link));
}
// if timestamp not known from input, but from a known record, use the records-timestamp
if ($date == 'now' && $entry->timestamp) {
$timestamp = $entry->timestamp;
}
My changes:
1. select from the aggregator_item record also the timestamp.
2. if the timestamp still is the default 'now' and retrieved from a record from the database, use the latter as the timestamp.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | aggregator-timestamp-fix-1304758-4.patch | 1.99 KB | PROMES |
| #9 | aggregator-timestamp-fix-1304758-3.patch | 2.01 KB | PROMES |
| #4 | aggregator-timestamp-fix-1304758-2.patch | 2.01 KB | PROMES |
| #1 | aggregator-timestamp-fix-1304758-1.patch | 1.81 KB | NROTC_Webmaster |
Comments
Comment #1
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedWhile this seems fairly straightforward to me I'm not sure that this will be implemented.
This is also very similar to the way they implemented it in D8 although it was moved to aggregator.processor.inc
Comment #2
PROMESThanks for your patch.
I presume that either D8 or wrong or your patch. See the equal where clauses in the second and thir query, both: where ... link = "%s" ... . In the original lines it states once link = "%s" and once title = "%s".
But the title and link in the D8 implementation are reversed between the second and third query. So I don't know when to use title or link.
A further extension in my code is after the first query a test whether the entry allready exists (coming from real life data):
I hope to improve this fine module.
Comment #3
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedThat was in your original code that you posted. Yes, it should be title and I didn't catch that when creating the patch. If you could copy your entire section of code from line 799 to the end of your changes I can create a patch for this.
You will still need to get some other people to review this and then approval from one of the core maintainers to get this in.
Comment #4
PROMESYou are right. The difference is in my original code.
I now can create diffs. Attached is my current diff-file, based on version 6.26.
I don't know any other user of this patch. If someone likes this, please drop a line.
Comment #5
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedComment #8
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedI'm not sure why this failed testing but you should make patches against the dev branch.
Comment #9
PROMESNew patch against dev branch.
I presume the test fails because I used as module name: aggregator-new.module.
Comment #10
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedComment #12
PROMESThe problems are:
1. failed testing: Detect invalid patch format. Ensure the patch only contains unix-style line endings.
2. I am working on a Windows pc, so the requirement seems not so easy to meet.
Do you know a Windows program to change the line endings into Unix style? I found a lot of same as #1 warnings on this site, but no solution.
Comment #13
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedI like notepad++ as it has lots of options to set it up based on the coding standards. Eclipse can integrate the drupal coding standard with DrupalCS so that may be useful but in my opinion it is rather difficult to get started with.
Comment #14
NROTC_Webmaster CreditAttribution: NROTC_Webmaster commentedAdditionally, you can go to Development toolsand pick from the list on the page.
Comment #15
PROMESThanks NROTC_Webmaster. After some testing with the settings of Notepad++, which already was one of my editors, I presume the file is now in Unix line endings.
Comment #16
ParisLiakos CreditAttribution: ParisLiakos commentedShould go in 8.x first
Comment #27
SpokjeThe
aggregatormodule has been removed from Core in10.0.x-devand now lives on as a contrib module.Issues in the Core queue about the
aggregatormodule, like this one, have been moved to the contrib module queue.Comment #28
larowlan