🇺🇸United States @maskedjellybean

maskedjellybean → created an issue.

Here's a version of the patch for Group 1.0.x (tested on 1.6.0) that combines the patch from #12 with the patch from #3 of this issue: https://www.drupal.org/node/3364703 →

The two are unrelated, but since they touch the same code, you can't apply both at once. This is likely something that only a few people would run into and probably frowned upon by maintainers, but uploading anyways. All this does differently from #12 is reload the entity in order to check that it still exists before saving. Strangely the code comments around line 216 of GroupContent.php imply that this is already happening, but it's clearly not.

Wow thank you for this! In my case this error was being thrown during a Behat afterScenario while attempting to delete nodes. Here is a version of the patch for Group 1.0.x (tested against 1.6.0) for those of us still stuck on a super outdated version.

That link is very informative, and thank you for writing it, but what I was trying to say is that we should clean up the documentation on drupal.org. The DDEV stuff is currently sprinkled all over the page instead of under a specific header.

But if there is a place to link to @selwynpolits site we should do it. Maybe a More Resources header?

Hey @JoseFran, you can generate a patch at: https://git.drupalcode.org/project/hook_event_dispatcher/-/merge_request... (just add .patch to the merge request URL).

@murz, can you explain how to use this feature once the patch is applied? I'd like to implement hook_ENTITY_TYPE_view.

I added a related issue that was seemingly incorrectly closed. Just to reiterate what was pointed out in that issue, there is almost certainly a performance penalty for using hook_entity_view when really you only need hook_ENTITY_TYPE_view. The first is called when viewing any entity and the second is only called when viewing a specific entity type. If events are going to be as performant as hooks then this needs to be figured out.

I have this problem but I'm not using the private file system (only public), so the patch doesn't help me. I tried the event subscriber in #3, and while it did work, it seems a little overbearing to run that if statement on every page request across the site on the off chance that the request contains "csv". Adding to .htaccess as suggested in #2 didn't work for me.

In the end I did what was suggested by the reporter: Set the file name in the views path settings.

This is a bug for sure, but at least it's easily worked around.

jwilson3 → credited maskedjellybean → .

I was experiencing this with Composer version constraint ^2.0@RC. Upgrading to ^3.0@RC appears to have fixed it. No patch was needed.

Wonder if someone who uses DDEV would move all the mentions of DDEV underneath the DDEV header. I think it would help folks navigate this easier, even if it means repeating some info.

Thank you for the patch. It fixed my issue importing into Outlook for Mac.

If you want to replicate the problem, use the module without this patch and try to import into Outlook for Mac. You'll see an error. Apply this patch, make a change to the node, save and repeat. No error.

If you don't have Outlook for Mac, you run a file generated by this module through the ICS validator:

https://icalendar.org/validator.html

You'll see errors like "Invalid TZOFFSETFROM".

Apply the patch, make a change to the node, save, download the file again and run it through the validator. You won't see that error anymore.

Realized my change to \Drupal\ics_field\ICalTimezoneGenerator::getMinMaxTimestamps was causing the file to be generated without BEGIN:DAYLIGHT, TZOFFSETFROM etc. Fixed with this patch.

This made me realize why my changes appeared to fix Outlook support. It's because prior to this patch I had accidentally removed the TZOFFSETFROM property. This module formats TZOFFSETFROM incorrectly, which Outlook (at least Outlook for Mac) then refuses to import. By accidentally removing that property entirely I had "fixed" Outlook. Now I realize there's a separate issue for this: https://www.drupal.org/project/ics_field/issues/3004699 →

The patch from the MR works for me in 10.1.8 to resolve this error thrown by a view. The view then functions as it should. Not sure what to make of that.

I think @MFH is right about this being potentially confusing. Shouldn't the title be "Create a custom plugin block"?

Made the requested changed to the MR. Unfortunately I can't do anything more to make the tests pass because the error appears to have nothing to do with the test being added here.

Removes unnecessary forward slash.

Capitalizes PHPUnit. Remove unnecessary language. Moves link to Lando under top Lando heading.

Adds example of Lando custom tooling command to run PHPUnit. Moves all mentions of Lando under the "PHPUnit in Lando" heading. I haven't done the same of DDEV because I don't use it. I'm not convinced that Lando and DDEV should be mentioned before the "Setting up to run PHPUnit tests manually" heading or that that heading is the best descriptor.

Thank you for the patch in #99. It still works in Drupal core 10.2.3.

I opened a MR (in case that helps move this along) which contains the changes from #99, plus hopefully fixes the tests, although I can already see they've failed...

maskedjellybean → made their first commit to this issue’s fork.

Opened a MR containing the changes from #4. I didn't include the later patches because they contain changes from https://www.drupal.org/project/ics_field/issues/2852239 ✨ Include end date / time in ICS file. Needs work .

Thank you for this. Patch #4 still applies cleanly to version 3.x and works perfectly. I used the token value of an Address field as the Location field value and my calendar apps interpret it exactly right.

New patch that fixes passing incorrect parameter type to \Eluceo\iCal\Property\Event\RecurrenceRule::setUntil

Realized that this was not working in Outlook. This new patch fixes that. It does a more generic check for HTML in the description. If there is HTML, it runs html2text and calls $iCalendarEvent->setDescription($this->getCalendarProperty('description')) with escaping enabled. If there is no HTML it calls $iCalendarEvent->setDescription($this->getCalendarProperty('description'), FALSE) with escaping disabled and calls $iCalendarEvent->setDescriptionHTML('') in order to set it to an empty string. This last piece is what fixes the Outlook compatibility.

Also, in order to make this work in Outlook you will need the patch from https://www.drupal.org/project/ics_field/issues/2946641#comment-15523825 ✨ Support date_recur field type. Active , because the module as it stands currently (with no patches applied) is not compatible with Outlook. Somehow I stumbled on a fix for Outlook in that otherwise unrelated patch.

Attaching patches. Be sure to apply ical-make-description-escaping-optional.patch to eluceo/ical, not drupal/ics_field.

After following the Steps to reproduce, and then applying the patches, you can verify they work by doing this:

1. Resave node to trigger regenerating of ICS file.
2. Click link to download ICS file.
3. Import the ICS file into any calendar app. You should see new lines instead of "\\n".

maskedjellybean → created an issue.

I found that Composer will not apply the patch generated from the merge request, so here is a manually created patch.

I've open a merge request:

https://git.drupalcode.org/project/ics_field/-/merge_requests/18#6bc0130...

Patch:

https://git.drupalcode.org/project/ics_field/-/merge_requests/18.patch

It expands on the patch from #4 to fully support date_recur field type by adding an RRULE property to the generated ICS file. With the addition of the RRULE property, every occurrence of an event will appear on the calendar, instead of only the first. This is what someone using a date_recur field would expect.

Other changes

Refactored dateslist/dates_list array structure

The dateslist/dates_list array structure is refactored from something like this:

[
  "1970-01-01 01:00:00 Europe/Zurich",
  "1971-02-02 02:00:00 Europe/Zurich",
]

To something like this:

[
  [
    'startdate' => "1970-01-01 01:00:00 Europe/Zurich",
    'enddate' => "1970-01-01 02:00:00 Europe/Zurich",
    'rrule' => 'RRULE:FREQ=DAILY;COUNT=10'
  ],
  [
    'startdate' => "1971-02-02 02:00:00 Europe/Zurich",
    'enddate' => "1971-02-02 03:00:00 Europe/Zurich",
    'rrule' => 'RRULE:FREQ=WEEKLY;COUNT=5'
  ],
]

This is because of the need to associate an RRULE with each start date of a multi value field. Additionally, an RRULE needs an end date, so that has been added as well. This may interfere/overlap with this issue. ✨ Include end date / time in ICS file. Needs work

I have not actually tested a multi value field, so this needs reviewing. With a date_recur field there is no need for multiple values since it allows you configure a pattern of occurrences based on the start date rather than several explicit start dates.

I tried to fix the tests, but I don't have a way to run phpunit at the moment so I probably messed up.

Fixed Outlook compatibility

The current version of the module generates ICS files that are incompatible with Outlook. Outlook will not import them. I have no idea how, but somehow I stumbled on a fix for this with my changes. Originally I was looking into fixing this the way that the addtocal_augment module fixed it, by not setting timezones, and instead setting dates using UTC 🐛 ics import problems due to malformed VTIMEZONE definition Fixed . However, that turned out to not be necessary.

I realized my previous comment is unrelated to this specific issue so I created a new one: https://www.drupal.org/project/ics_field/issues/3436571 🐛 Field storage does not exist error Active

maskedjellybean → created an issue.

maskedjellybean → created an issue.

On ics_field 3.0.0, this patch does apply cleanly, however it creates an error that's thrown on most pages and results in my having to drop the database because even config import can't undo it.

Steps to reproduce:

1. Apply patch.
2. Add a Calendar download field to content type that has a date_recur field
3. In field settings, select the date_recur field. The error will thrown after the AJAX request. In watch dog you'll see: Error Drupal\Core\Field\FieldException: Attempted to create an instance of field with name field_ics_download on entity type node when the field storage does not exist. in Drupal\field\Entity\FieldConfig->getFieldStorageDefinition() (line 316 of /app/docroot/core/modules/field/src/Entity/FieldConfig.php) This error will be shown on most subsequent page loads.

I'd also say that in order to actually support date_recur, we would want to add recurring events to the ICS file using RRULE. The date_recur module actually stores the RRULE string in the database in the field table, so we should be able to get it from there.

Thanks @danflanagan8. That makes sense. I appreciate your effort! Hope you can get full control over the module somehow.

Thank you. #12 worked for me in Drupal 10.2.3.

It allowed me to aggregate using a date_recur field in a view of occurrences. I set the start date to aggregate using Min and the end date to aggregate using Max. This gave me one row for each event, and each row displayed the date of the first occurrence and the last occurrence. Exactly what I needed. Before applying the patch I saw this error:

TypeError: round(): Argument #1 ($num) must be of type int|float, string given in round() (line 173 of /app/docroot/core/modules/views/src/Plugin/views/field/NumericField.php)

Thank you for this. #17 worked for me in Drupal 10.2.3.

Could we get a solid answer on when/if support for addevent.com will be added? I see the update in this issue description says "Update: 8.x-4.x-dev supports "Add to calendar field type via Addevent.com" now", but that doesn't appear to be true. Somehow 8.x-4.x is more out of date than 8.x-3.x, which I'm already using, and where no mention of "addevent" is found in the codebase.

Thanks all and thanks @HitchShock. #33 works for me in Drupal 10.2.3 and Group 1.6.0.

Adding some pieces of the error that is thrown by this issue so hopefully folks find this easier:

access denied Warning Path: /views/ajax?_wrapper_format=drupal_ajax
Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException: in Drupal\views\Controller\ViewAjaxController->ajaxView() (line 220 of /docroot/core/modules/views/src/Controller/ViewAjaxController.php)

I agree that this would be better UX.

This issue is related but different:
https://www.drupal.org/project/drupal/issues/2883755 ✨ Introduce a submit confirmation step modal to Form API for usability Active

I also found this module which I'm surprised hasn't been posted here yet:
https://www.drupal.org/project/admin_dialogs →

I haven't tried it yet, but as the time of posting it is maintained.

Regardless I've realize this will not help you add a confirmation to an existing class. For an explanation why see:  https://www.drupal.org/project/drupal/issues/2883755#comment-15463064  ✨ Introduce a submit confirmation step modal to Form API for usability Active If you'd like to see this issue resolved in core, comment on that issue.

I've love to see a solution for this.

I mistakenly thought I could solve this using ConfirmFormBase ( https://www.drupal.org/docs/drupal-apis/form-api/confirmformbase-to-conf... → ), but I've come to the disappointing conclusion that it's simply not feasible to add confirmation to an existing form that does not already extend ConfirmFormBase. In order to add this to an existing form you would first need to override the existing form class with a new class that you control. This is possible. However, in order to avoid duplicating all of the code from the existing class, you would want to extend that class and only modify the necessary methods while calling the parent methods inside your modified methods when possible. But then of course your new class does not extend ConfirmFormBase, so you are right back where you started.

I guess it would be more helpful if the example demonstrated something more than a confirmation. What is the actual action that we are confirming? It’s an incomplete example.

What is the practical purpose of this? I see no way to attach this to an existing form or even a custom form. Isn't the example essentially just a form? It doesn't actually "confirm" the action of another form at all.

I realized I can get around the issue by editing my local config split then choosing Advanced > Do not patch dependents. I understand this undoes some of the benefits of 2.x, but it is a way to avoid this issue for now. Since my local dev scripts will yell at me (and other devs) if drush config:status returns something other than "No differences between DB and sync directory.", I need to do this.

maskedjellybean → created an issue.

Oops, fixed the accidental foreach loop deletion.

The patch from #7 does not apply cleanly against branch 2.0.x because it's based on 8.x.1.x. For those needing a fix for branch 2.0.x (I tested against module version 2.0.0-alpha2), see the MR I created which replicates/rerolls the changes against 2.0.x, or here's a link to the patch:

https://git.drupalcode.org/project/linkchecker/-/merge_requests/61.patch

Thank you for the fix!

For those applying these patches: After applying, you need to save the form at /admin/config/content/linkchecker before the fixes will take affect. I came to this thread because cron was failing with:

ValueError: array_chunk(): Argument #2 ($length) must be greater than 0 in array_chunk() (line 132 of /app/docroot/modules/contrib/linkchecker/src/LinkCheckerService.php).

So this wasn't obvious to me.

maskedjellybean → made their first commit to this issue’s fork.

If there wasn’t so much Drupal 7 documentation on this site that is so easy to stumble upon, either via the navigation or via Google, I could agree. With the current state of the site I think we should do anything we can do to help new users. The breadcrumbs don’t even have any indication of Drupal versions.

Following up to say that the issue that spurred this question has an answer here: https://www.drupal.org/project/feeds/issues/3283876#comment-15249862 🐛 Image link is mapped incorrectly Active
And the issue seems unrelated to having multiple feed types importing into one content type.

It would still be nice to have a definitive answer to whether we are supposed to import multiple feed types into one content type, but it does work.

Thank you for carrying this forward! Sadly I no longer have an Algolia project to work with so I can't test the new patch out.

maskedjellybean → created an issue.

Thank you. The problem was the field_event_dispatcher submodule not being enabled.

I found that as mentioned in #3, my filters were not the same on the export display as on the other display. Reverting filters to default on the export display fixed the issue for me.

Mention Upgrade Status along with Drupal Rector so folks know they shouldn't uninstall Upgrade Status yet if they have custom modules.

I wonder if we really need to call out the fact that Drupal Console is abandoned. I think it's been abandoned for a few years now and it's not really Drupal 10 related.

Thanks ressa! That does make sense. Good call.

quietone, I agree with ressa that we are documenting upgrading to the next major version, not updating to the next minor version. Upgrading is a much more time consuming process because of all the deprecations you need to replace, dealing with modules that aren't compatible etc.

For what it's worth, these were the rest of my notes taken during the upgrade process so I don't have anything else to add that would make the page longer. :-)

Hey @ressa! I do understand the concern about the length of the page, but I think the length reflects the potential difficulty of the upgrade process. It's taken my organization over a month and hundreds of commits to upgrade one site. Granted it's a very large site with a lot of custom modules and tests, and the organization tends to move slowly and carefully.

With regards to Drupal Rector, I feel strongly it should be described here. Upgrading all our custom modules would have taken so much longer without it. I think any large site with custom modules or themes would benefit immensely from using it, so the more we can steer people towards it the easier their experience will be.

The "Automatically replace deprecated code using Drupal Rector" heading should be under the "3. Update custom modules and/or themes" heading because it's specifically useful for replacing deprecated code in custom modules. Running it on contributed modules would not be useful because the changes will be replaced when composer update or install is run. It would be useful for contributed module maintainers, but I think that's outside the scope of this documentation.

The "Common issues" heading is now meant to be specifically common issues when following the directions under "4. Upgrade to Drupal 10" but totally open to making that more clear somehow.

Remove info about config export from "Upgrade to Drupal 10" section because it is now in "Post upgrade tasks" section.

Add "Post upgrade tasks" section.

Replace nested list with H3 headings. In hindsight it was hard to read and impossible to link to subsections.

Flesh out "Update custom modules and/or themes" section with notes I made while updating my custom modules and themes.

Remove extra blank lines.

Clean up "Update contributed modules/projects" section to make it flow chronologically. Change bulleted list to ordered list to indicate it is not a step by step process the reader can follow. Change the heading to an H2 because this will likely be a time consuming process and deserves to be featured more prominently. 

Thank you @MegaChriz for merge request 147 in comment #11. Applying the patch fixed the error and allowed cron to finish! I work with @jnicola so this addresses the problem the issue was created for.

However after applying the patch, on the first cron run I saw this warning:

[warning] unlink(/mnt/tmp/projectname/feeds_http_fetcherw5KhhB): No such file or directory FileSystem.php:124

I have not seen this warning on subsequent cron runs.

With patch #56 applied to Drupal 10.1.6 I get this error in watchdog when submitting (pressing submit/apply button) with an embedded view with AJAX enabled and with exposed filters:

access denied	Warning	Path: /views/ajax?_wrapper_format=drupal_ajax&title=&field_event_date_value=2023-12-04&field_event_date_end_value=&field_event_tags_target_id=&moderation_state=All&view_name=group_dashboar

And it seems that the exposed filters have no affect on the results.

Additionally when resetting the view using the reset button, despite having AJAX enabled for the view, the entire page is reloaded and I am taken to the top of the page. This is less than ideal.

@pookmish You are a life saver! I would've never figured this out. Confirmed that the patch for in #2 for hook_event_dispatcher 4.x resolves the error.

My dev process involves installing the site, so as to have an "empty" database for development and testing. Without the patch this fails due to a memory limit error connected to hook_event_dispatcher. Here's part of the stack trace:

11:32:28 Fatal error: Allowed memory size of 1073741824 bytes exhausted (tried to allocate 6433664 bytes) in /var/www/docroot/core/lib/Drupal/Core/Database/StatementWrapperIterator.php on line 110
11:32:28 
11:32:28 Fatal error: Uncaught PDOException: SQLSTATE[HY000]: General error: 2014 Cannot execute queries while other unbuffered queries are active.  Consider using PDOStatement::fetchAll().  Alternatively, if your code is only ever going to run against mysql, you may enable query buffering by setting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute. in /var/www/docroot/core/lib/Drupal/Core/Database/StatementWrapperIterator.php:110
11:32:28 Stack trace:
11:32:28 #0 /var/www/docroot/core/lib/Drupal/Core/Database/StatementWrapperIterator.php(110): PDOStatement->execute(Array)
11:32:28 #1 /var/www/docroot/core/lib/Drupal/Core/Database/Query/Upsert.php(115): Drupal\Core\Database\StatementWrapperIterator->execute(Array, Array)
11:32:28 #2 /var/www/docroot/core/lib/Drupal/Core/Cache/DatabaseBackend.php(283): Drupal\Core\Database\Query\Upsert->execute()
11:32:28 #3 /var/www/docroot/core/lib/Drupal/Core/Cache/DatabaseBackend.php(210): Drupal\Core\Cache\DatabaseBackend->doSetMultiple(Array)
11:32:28 #4 /var/www/docroot/core/lib/Drupal/Core/Cache/DatabaseBackend.php(186): Drupal\Core\Cache\DatabaseBackend->setMultiple(Array)
11:32:28 #5 /var/www/docroot/core/lib/Drupal/Core/Extension/ModuleHandler.php(329): Drupal\Core\Cache\DatabaseBackend->set('module_implemen...', Array)
11:32:28 #6 /var/www/docroot/modules/contrib/hook_event_dispatcher/src/HookEventDispatcherModuleHandlerProxyTrait.php(121): Drupal\Core\Extension\ModuleHandler->writeCache()
11:32:28 #7 /var/www/docroot/core/lib/Drupal/Core/EventSubscriber/RequestCloseSubscriber.php(42): Drupal\hook_event_dispatcher\HookEventDispatcherModuleHandler->writeCache()
11:32:28 #8 [internal function]: Drupal\Core\EventSubscriber\RequestCloseSubscriber->onTerminate(Object(Symfony\Component\HttpKernel\Event\TerminateEvent), 'kernel.terminat...', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
11:32:28 #9 /var/www/docroot/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(111): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\TerminateEvent), 'kernel.terminat...', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
11:32:28 #10 /var/www/vendor/symfony/http-kernel/HttpKernel.php(115): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object(Symfony\Component\HttpKernel\Event\TerminateEvent), 'kernel.terminat...')
11:32:28 #11 /var/www/docroot/core/lib/Drupal/Core/StackMiddleware/StackedHttpKernel.php(63): Symfony\Component\HttpKernel\HttpKernel->terminate(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Render\HtmlResponse))
11:32:28 #12 /var/www/docroot/core/lib/Drupal/Core/DrupalKernel.php(688): Drupal\Core\StackMiddleware\StackedHttpKernel->terminate(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Render\HtmlResponse))
11:32:28 #13 /var/www/vendor/drush/drush/src/Boot/DrupalBoot8.php(394): Drupal\Core\DrupalKernel->terminate(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Render\HtmlResponse))
11:32:28 #14 [internal function]: Drush\Boot\DrupalBoot8->terminate()

Thank you @chetan 11, this seems to resolve the issue for me.

I did end up needing the wrapper service. Here's a complete example:

In addition to the code in #13, you will need a getter method:

  /**
   * Gets Menu Tree Storage service.
   *
   * @return \Drupal\Core\Menu\MenuTreeStorageInterface
   */
  public function getMenuTreeStorage(): MenuTreeStorageInterface {
    return $this->menuTreeStorage;
  }

Then inject your new service where it's needed instead of the MenuLinkStorage service.

Use it like this:
$this->treeStorageWrapper->getMenuTreeStorage()->getRootPathIds($plugin_id)

Thanks @ressa. I’m sorry this is actually the first time I’m reading the discussion here. I felt that the documentation was lacking and since I’m upgrading myself now I’d add to it while it’s fresh in my head. Actually this is the first time I saw the page you linked, and I agree they cover a lot of the same  topics. I agree with linking to that whenever possible. I think you also make a good point that it’s easy for users to skip the overview page. Maybe it should be one page. I get the impression we can do whatever we think is best.

I'm seeing this as well.

Thank you for the MR/patch. After applying I no longer see the error.

Thank you for the example! I don't understand how it's an improvement over accessing the service directly, but that's ok.

Hmm actually I think #6 works for this situation too.

Has anyone who needs to use methods from the now unusable service figured out a workaround? Can you post an example here of wrapping the now private service in a public service? I need to use \Drupal\Core\Menu\MenuTreeStorage::getRootPathIds.

The cause of this is an incorrectly formatted library name, likely in YOURTHEME.libraries.yml. Look for a library that does not follow this format: "MODULE_NAME/LIBRARY_NAME" (the forward slash is crucial). If you are able to step debug you could also add a breakpoint within \Drupal\Core\Asset\LibraryDependencyResolver::doGetDependencies (currently line 67). Refresh the page that triggers the error and continue debugging until you find that $library does not follow the correct format. Search your codebase for the incorrect format and replace it if it is in a custom module or theme. If it is in a contrib module, open an issue there.

Perhaps the goal of this ticket could be to have core throw a useful error message when this happens?

Removed unnecessary bold heading.

Move errors under generic "Common errors" heading. Add steps for running composer install and exporting config.

Clarified the process of dealing with "Your requirements could not be resolved to an installable set of packages" error. Rearranged information into a more logical order.

Changed instances of "Drupal 8" to "Drupal 8+" to indicate this documentation is still applicable to Drupal 9 and 10.

I'm not sure whether this is a bug or task. I only know it doesn't seem right. Having the additional nested block classes will cause unexpected styling issues.

This has been a problem on every Drupal 8+ project I've worked on regardless of theme.

Thanks @jwilson3. I've created a new issue: https://www.drupal.org/project/label_help/issues/3390701 🐛 getThirdPartySetting() returns null so form is not altered Active

maskedjellybean → created an issue.

Merge request opened:

https://git.drupalcode.org/project/feeds/-/merge_requests/137

If anyone needs a patch:

https://git.drupalcode.org/project/feeds/-/merge_requests/137.patch

Explanation and disclaimer

Please know this only a workaround and I don't truly expect this to be merged. It does not get at the true source of the issue. But let me explain what this workaround does.

I can only speak from our experience. This can happen when you have multiple feed types with multiple feeds and they all update one content type. It's certainly possible this bug occurs under other circumstances, but this is ours.

Removing this if statement from \Drupal\feeds\Laminas\Extension\Mediarss\Entry::getMediaElement will work around the issue:

    if (array_key_exists($media_key, $this->data)) {
      return $this->data[$media_key];
    }

This is because if you are importing multiple feeds at once either by running cron or by running drush feeds:import-all (although I have seen this happen more consistently when triggered by cron), and a feed has already imported that has <media:content /> data, when the next feed is imported that also has <media:content /> data, $this->data can contain stale/incorrect data from the previous feed. Removing the if statement forces the media:content url to parsed/retrieved from the feed.

Why $this->data contains stale data is a larger issue. I've tried to understand it but I can't. I think it has to do with \Drupal\feeds\Feeds\Parser\SyndicationParser::parse and the way $entry is instantiated within the foreach loop (currently starting at line 94). It is not clear to me what class $entry is or how the current code even works.

Within the foreach loop, $entry may be one of these classes:

• \Drupal\feeds\Laminas\Extension\Mediarss\Entry
• \Laminas\Feed\Reader\Feed\Rss
• \Laminas\Feed\Reader\Entry\Rss

It seems to me that \Drupal\feeds\Laminas\Extension\Mediarss\Entry should extend \Laminas\Feed\Reader\Feed\Rss instead of \Laminas\Feed\Reader\Feed\AbstractFeed. I say this because there are no issues with stale/incorrect data for the methods on \Laminas\Feed\Reader\Feed\Rss such as \Laminas\Feed\Reader\Feed\Rss::getTitle.

maskedjellybean → made their first commit to this issue’s fork.

maskedjellybean → created an issue.

I'm not sure if this is related, but occasionally and seemingly randomly $content = $field->getThirdPartySetting('label_help', 'label_help_description'); in label_help_form_alter() will return null, causing the entire form alter to do nothing.

Merge request opened. https://git.drupalcode.org/project/label_help/-/merge_requests/5

This also fixes a couple typos, adds variable names to a docblock and fixes this warning which has been popping up:

Warning: Undefined array key "#type" in label_help_form_alter() (line 219 of modules/contrib/label_help/label_help.module).
label_help_form_alter(Array, Object, 'node_event_edit_form') (Line: 562)

Thank you for this module. Digging into the code made me appreciate how difficult this was to implement!

For anyone coming to this issue with the same problem, remember you can get a patch by appending .patch to the end of the merge request URL:
https://git.drupalcode.org/project/label_help/-/merge_requests/5.patch

maskedjellybean → created an issue.

If you are overriding a template from another module in your module then the answer varies slightly. In this example we override a template from the webform module.

In modules/custom/my_module/my_module.module

/**
 * Implements hook_theme_suggestions_alter().
 */
function my_module_theme_suggestions_alter(array &$suggestions, array $variables, $hook) {
  // Use switch so we can easily add another hook in the future.
  switch ($hook) {
    case 'webform_email_html':
      $suggestions[] = $hook . '__' . 'my_module';
      break;
  }
}

/**
 * Implements hook_theme().
 */
function my_module_theme($existing, $type, $theme, $path) {
  return [
    'webform_email_html__my_module' => [
      'base hook' => 'webform_email_html',
      'path' => $path . '/templates/webform',
    ],
  ];
}

We can now create a new template file at modules/custom/my_module/templates/webform/webform-email-html--my-module.html.twig by copying and modifying webform-email-html.html.twig from the webform module. Clear caches, refresh the page and it should load.

Thanks @Simon-P. #14 is the correct way to reproduce. That is exactly what caused the error for me. Changing the Filter Identifier back to the default value avoids the error. We need to figure out why changing the Filter Identifier causes this error.

maskedjellybean → created an issue.

Ran into this again on an unrelated piece of code and once again reverting back to hook_views_query_alter fixes it. I definitely think this is a real issue.

I don't have my local environment setup for phpunit yet but I'll try to validate this.

Good find @rodrigoaguilera. I can't think of a reason for this either. I wonder if simply removing that check would solve the problem and if so would the core maintainers accept it.

This seems like it should be part of core. The only way to do achieve the same filtered result via core right now is to expose the operation filter. Then the user has to choose "Is empty (NULL)" from a separate filter and "- Any -" from the main filter. This is not good UX.

In the meanwhile I was able to do this for a one off situation using a couple hooks and I wanted to share.

The idea is to expose the operation filter and then hide it in a form alter. Also in the form alter, add a "None" option to the main filter. Then in a query alter, if None was selected, apply a JOIN and WHERE as though "Is empty (NULL)" were chosen from the operator filter.

Things to know about the code example:
- My field is a taxonomy reference field with the machine name field_prof_languages.
- My view has the machine name profile_language_report.
- field_prof_languages_target_id_op refers to the operation filter.
- For what it's worth, you can't do this via an event subscriber because of this: https://www.drupal.org/project/hook_event_dispatcher/issues/3365175 🐛 VIEWS_QUERY_ALTER doesn't allow unset() but equivalent hook does Active

/**
 * Implements hook_form_FORM_ID_alter().
 *
 * PURPOSE: Add custom "None" option to Profile Language Report view filter.
 */
function mymodule_form_views_exposed_form_alter(&$form, FormStateInterface $form_state, $form_id) {
  if ($form['#id'] === 'views-exposed-form-profile-language-report-page-1'
    || $form['#id'] === 'views-exposed-form-profile-language-report-data-export-1'
  ) {
    // Hide operations field.
    // We instead add "None" as option to main field
    // and alter the query in views query alter.
    $form['language_wrapper']['field_prof_languages_target_id_op']['#access'] = FALSE;
    $options = $form['language_wrapper']['language']['#options'];
    $form['language_wrapper']['language']['#options'] = array_slice(
        $options, 0, 1, TRUE) +
      [
        'None' => new TranslatableMarkup(
          '- None (no language set) -'),
      ] +
      array_slice($options, 1, count($options) - 1, TRUE);
  }
}

/**
 * Implements hook_views_query_alter().
 *
 * PURPOSE: Alter Profile Language Report view query based on
 * "None" option added in mymodule_form_views_exposed_form_alter.
 */
function mymodule_views_query_alter(ViewExecutable $view, QueryPluginBase $query) {
  if (isset($view->getExposedInput()['language']) && $view->getExposedInput()['language'] === 'None') {
    $definition = [
      'table' => 'node__field_prof_languages',
      'field' => 'entity_id',
      'left_table' => 'node_field_data',
      'left_field' => 'nid',
    ];
    $join = \Drupal::service('plugin.manager.views.join')
      ->createInstance('standard', $definition);
    $query->addRelationship('node__field_prof_languages', $join, 'node_field_data');
    $query->addWhere(
      1,
      'node__field_prof_languages.field_prof_languages_target_id',
      NULL,
      'IS NULL'
    );
    // Remove unwanted where condition created by
    // our custom "None" option added in form alter.
    unset($query->where[0]);
  }
}

maskedjellybean → created an issue.