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

gábor hojtsy’s picture

Project: Drupal.org site moderators » Drupal core
Version: » 7.x-dev
Component: Other » aggregator.module

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.

karschsp’s picture

Issue tags: +Novice

This would be a good one for the novice queue.

alex_b’s picture

Title: Aggregator module update interval » Aggregator update interval default

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: Convert Aggregator feeds into entities because #293318 moves the settings form to the content type form.

JamesAn’s picture

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.

JamesAn’s picture

Status: Active » Postponed

Just doing some spring (summer?) cleaning.

This is postponed until #293318: Convert Aggregator feeds into entities is finished.

JamesAn’s picture

Version: 7.x-dev » 8.x-dev

As #293318: Convert Aggregator feeds into entities is now slated for D8, this is deferred as well.

damien tournoud’s picture

Version: 8.x-dev » 7.x-dev
Status: Postponed » Active

Only feature requests and tasks can be assigned to D8 for now, because the Drupal 8 tree is not opened yet.

franz’s picture

Version: 7.x-dev » 8.x-dev

I think this belongs to 8.x-dev now.

Does it need to be backported to 7.x ?

oriol_e9g’s picture

Version: 8.x-dev » 7.x-dev
Status: Active » Fixed

Actually this is solved in D8/D7:

'#default_value' => isset($feed->refresh) ? $feed->refresh : 3600,

Status: Fixed » Closed (fixed)
Issue tags: -drupal.org upgrade, -Novice

Automatically closed -- issue fixed for 2 weeks with no activity.