feed-me icon indicating copy to clipboard operation
feed-me copied to clipboard

CP UI doesn't warn that checking "Disable missing elements" AND "Delete missing elements" will break EVENT_AFTER_PROCESS_FEED

Open simonkuran opened this issue 4 years ago • 4 comments

Description

If both "Disable missing elements" and "Delete missing elements" are checked in the CP, line 479 of services/process.php will stop EVENT_AFTER_PROCESS_FEED from firing. This is super confusing, the feed works just fine and no errors are shown in the CP. The only indication is a "You can't have Delete and Disabled enabled at the same time as an Import Strategy." warning in the logs.

I feel like one of these things should happen.

  1. "Delete missing elements" should be used because it's the last checkbox.
  2. "Disable missing elements" and "Delete missing elements" should switch from checkboxes to radio buttons since only one of them should be used.
  3. EVENT_AFTER_PROCESS_FEED should still be triggered even if missing items from the feed aren't handled.
Screen Shot 2020-10-08 at 3 08 53 PM

P.S. It looks like when the "You can’t choose “Disable missing elements in the target site” for feeds without a target site." log warning is triggered on line 484 of services/process.php as similar situation occurs.

Steps to reproduce

  1. Check "Disable missing elements" and "Delete missing elements" on a feed.
  2. Listen for EVENT_AFTER_PROCESS_FEED.

Additional info

  • Feed Me: 4.2.4

simonkuran avatar Oct 08 '20 11:10 simonkuran

Wouldn't a more obvious solution be that if you choose one, the other option is disabled? It's a version of your option 2, I guess. But it might make more sense, as the radio button route would need a third option for neither deleting nor disabling. And if you get stumped by it, it's not unreasonable to expect people, after a moment of thought, to realise that an element can't be both deleted and disabled at the same time.

As for any saved feeds up until now, I would favour defaulting to disabling, as it's more non-destructive. But I agree doing nothing is clearly not helping too much.

kristiansp avatar Oct 17 '20 18:10 kristiansp

Disabling the un-selected option does make sense. If I check both I would expect the elements to be deleted, or disabled and then deleted. And I certainly wouldn't expect EVENT_AFTER_PROCESS_FEED to not be triggered, that's the biggest issue in my book.

simonkuran avatar Oct 19 '20 06:10 simonkuran

This is still an ongoing issue.

Just encountered recently, on Craft 4.

Both of these options being checked is causing Blitz to not clear its cache, as EVENT_AFTER_PROCESS_FEED is not triggered.

Soben avatar Nov 13 '23 20:11 Soben

I was just about to report the same problem that has cost me an hour.

In storage/logs/feedme.log there is an info message logged, which I think should at least be a warning, and be displayed in the "Logs" tab. Especially because neither is applied.

{"date":"2024-03-08 12:00:00","type":"info","message":"Import: You can't have Delete and Disabled enabled at the same time as an Import Strategy.","key":"xxx"}

tricki avatar Mar 08 '24 15:03 tricki