Posted by webchick on March 2, 2009 at 6:29am
Jump to:
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | aggregator.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | drupal.org upgrade, Novice |
Issue Summary
I noticed this defaults to 15 mins now. It seemed like this used to default to 1 hour. Is this change by design, or is it a bug?
Comments
#1
Looking at the code, this is due to the signature of http://api.drupal.org/api/function/aggregator_form_feed/6 which was added pre-file-split (so the history of aggregator.admin.inc is not enough to look at). The patch which added the 900 sec default in was committed by Steven and created by chx: #130971: The kitchen sink (remove notices, fix bugs, enhance code styling) Looks like this was an inadvertent modification to how it works, since the default inside the function is 3600.
This bug / inconsistency is still present in Drupal 7, so should be fixed there first.
#2
This would be a good one for the novice queue.
#3
Right now it defaults to 1 hour actually, because there is a double check for whether $feed->refresh is present - see aggregator_form_feed()
if (empty($feed->refresh)) {$feed->refresh = 3600;
}
// ...
$form['refresh'] = array('#type' => 'select',
'#title' => t('Update interval'),
'#default_value' => isset($feed->refresh) ? $feed->refresh : 900,
'#options' => $period,
'#description' => t('The length of time between feed updates. Requires a correctly configured <a href="@cron">cron maintenance task</a>.', array('@cron' => url('admin/reports/status'))),
);
However, I think we should not just remove this double check and set the default to 30 minutes, but
* we should remove the 15 minutes options (< 30 min update interval is overly aggressive. < 30 minutes behavior for corner cases can be changed with form_alter).
* we should add a configurable default on the settings form ('aggregator_default_refresh').
This patch will depend on #293318: Make Aggregator feeds into entities because #293318 moves the settings form to the content type form.
#4
This patch might as well depend on #19646: Aggregator: Feed Suspend Option as well, as #19646 adds a 'Never' option to the update interval so that feed updates can be suspended indefinitely.
#5
Just doing some spring (summer?) cleaning.
This is postponed until #293318: Make Aggregator feeds into entities is finished.
#6
As #293318: Make Aggregator feeds into entities is now slated for D8, this is deferred as well.
#7
Only feature requests and tasks can be assigned to D8 for now, because the Drupal 8 tree is not opened yet.
#9
I think this belongs to 8.x-dev now.
Does it need to be backported to 7.x ?
#10
Actually this is solved in D8/D7:
'#default_value' => isset($feed->refresh) ? $feed->refresh : 3600,#11
Automatically closed -- issue fixed for 2 weeks with no activity.