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
- Create a feed type.
- On the feed type processor settings, set a value for "Expire content items". For example, set it to "after 1 hour".
- Create a feed and import data.
- Wait until the expire time is over (for example: 1 hour).
- 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
- Add test coverage for the expire feature using the UI. See steps to reproduce.
- Investigate how much work it is to also support the expire feature on cron runs.
- If the expire feature is made for cron in this issue, add test coverage for this case.
- 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
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:
Comments
Comment #4
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedI've written a tests for expiring through the UI and cron.
I only wrote a fix for expiring through the UI so far.
Comment #6
MegaChriz CreditAttribution: MegaChriz as a volunteer commentedA 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.
Comment #9
jwilson3Follow. Not only is cron affected, but also
drush feed-im
is affected.