Problem/Motivation

Since Feeds 8.x-3.0-alpha7 the expire feature was accidentally broken. This happened when doing some refactoring in #2811429: Switch to feed owner during manual import..

Specifically, FeedsImportHandler used to have the following code:

/**
   * Finishes importing, or starts unfinished stages.
   *
   * @param \Drupal\feeds\FeedInterface $feed
   *   The feed.
   */
  public function batchPostProcess(FeedInterface $feed) {
    if ($feed->progressParsing() !== StateInterface::BATCH_COMPLETE) {
      $this->startBatchParse($feed);
    }
    elseif ($feed->progressFetching() !== StateInterface::BATCH_COMPLETE) {
      $this->startBatchFetch($feed);
    }
    elseif ($feed->progressCleaning() !== StateInterface::BATCH_COMPLETE) {
      $this->startBatchClean($feed);
    }
    else {
      $feed->finishImport();
      $feed->startBatchExpire();
    }
  }

See one of the last line, where it says $feed->startBatchExpire();. This line got vanished. Something like it should have been added to FeedsExecutable::finish() (or FeedsBatchExecutable::finish(), since the expire feature didn't even work on cron yet).

Steps to reproduce

  1. Create a feed type.
  2. On the feed type processor settings, set a value for "Expire content items". For example, set it to "after 1 hour".
  3. Create a feed and import data.
  4. Wait until the expire time is over (for example: 1 hour).
  5. Import an updated feed in which some items are missing.

Expected result: the items that were imported an hour ago and that were not updated, should have been deleted.
Actual result: the expire batch does not happen.

Proposed resolution

It would be cool if the expire feature works both on cron and in the UI. But if that's a lot of work, we can for now probably also restore the functionality by triggering the expiration in FeedsBatchExecutable::finish().

We do need a functional test to ensure that the expire batch gets triggered.

Remaining tasks

  1. Add test coverage for the expire feature using the UI. See steps to reproduce.
  2. Investigate how much work it is to also support the expire feature on cron runs.
  3. If the expire feature is made for cron in this issue, add test coverage for this case.
  4. Restore the expire feature (for UI and perhaps also for cron).

User interface changes

None.

API changes

Probably none.

Data model changes

None.

Issue fork feeds-3193092

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

MegaChriz created an issue. See original summary.

MegaChriz’s picture

Status: Active » Needs work

I've written a tests for expiring through the UI and cron.

I only wrote a fix for expiring through the UI so far.

  • MegaChriz committed 4752a29 on 8.x-3.x
    Issue #3193092 by MegaChriz: Expire functionality broken
    
MegaChriz’s picture

A partly fix for the expire feature has been merged, but expiring items does not happen on cron runs yet. So leaving the issue open for that.

jwilson3’s picture

Follow. Not only is cron affected, but also drush feed-im is affected.

j_ten_man made their first commit to this issue’s fork.