Just came across this during setup and same problem. Reviewed the MR it looks good, updated status to get this updated documentation committed. Thanks all!
ben.hamelin → made their first commit to this issue’s fork.
Thank you @rushikesh-raval
I am planning some feature upgrades for the Feeds iCal module that I will share once they are committed. 'm hopeing these enhancements will provide sufficient PHP code to meet these requirements.
Good morning! Just chiming in here, this fixed an issue we were dealing with where the breadcrumb was being driven by the assignment to a Group. Alias was derived from the group path, but breadcrumb was being cached. Thanks everyone!
Any technical solution exclusively in Drupal code is always going to run up against the hosting environment limitations. One way I could see this working would be with a SMTP server provided by the Drupal Association. You would then need an account on d.o. plus DNS configuration to enable sending. Then the service could be supported $$ wise by association funds, donations, etc.
@codekarate or other maintainers. What's the status of this module?
Good morning @avpaderno. I'd like to re-open this request as there have been a number of recent improvements to the Feeds iCal module, in addition to other contributions I've made in the past few years. Here are some of the recent fixes I've directly written code for:
https://www.drupal.org/project/project_browser/issues/3293909
🐛
Security and maintained icons need to communicate correct information
Fixed
https://www.drupal.org/project/feeds_ical/issues/3445551
🐛
PHP 8.2: Creation of dynamic property is deprecated
Fixed
https://www.drupal.org/project/feeds_ical/issues/3379345
🐛
Deprecated function in ICal
Fixed
Additionally, I am co-maintainer of the following modules:
https://www.drupal.org/project/sfds →
https://www.drupal.org/project/layout_section_fields →
Hoping this additional work is sufficient, but if not I have some features I'm planning for Feeds iCal which I think would provide enough additional PHP code to cover the requirements.
Thanks!
- Ben
Release 2.3.0 contains the new fields
@vasyok - Thank you for that clarification and additional info. I understand now that you wish to have access to the raw values for DTSTART and DTEND. I have opened this issue in response to address: https://www.drupal.org/project/feeds_ical/issues/3477371 ✨ Provide the DTSTART and DTEND values as raw strings Active
ben.hamelin → created an issue.
@vasyok Not sure, but it looks like you are mapping to a TEXT field that has a character limit. I would map those to Date / Time fields.
As this appears to be an unrelated issue and the original request has been addressed, I'm going to close this issue.
Thanks for using the module, if you have any suggestions for documentation or examples you want to add please feel free to open a merge request!
That shouldn't be happening. LOCATION is provided as one of the default fields since it is expected in an ical feed.
https://icalendar.org/
Can you provide an example of your ICS / ICAL source file?
@vasyok - The only way to move any of the Predefined source fields outside that group would be to override the src/Feeds/Parser/IcalParser.php class, specifically the getMappingSources() method.
To my knowledge the group they are in has no affect on the final mapping. What is your goal in trying to regroup them?
Hi @VasyOK,
The "Predefined" source values come from `src/Feeds/Parser/IcalParser.php` parser plugin. See https://git.drupalcode.org/project/feeds_ical/-/blob/2.x-dev/src/Feeds/P...
The "Feed Entity" group is provided by Feeds module.
The "Ical" group is added once you start adding custom source values. I did not see the Ical group until I added a test source in my local development environment.
@johnpicozzi Ok updated styles and addressed the linting and FunctionalJavascript tests. This is ready for another look!
I've rebased from 2.0.x and updated the markup to include the button tag with title attribute as suggested. This is ready for review.
ben.hamelin → changed the visibility of the branch 3293909-security-and-maintained to hidden.
@rockmachine Thanks for the follow up. Glad things are working for you now!
To future proof your code changes you should introduce a custom module: https://www.drupal.org/docs/develop/creating-modules →
The goal here would be to create a sub class of the IcalItem class that contains the properties you need to override. This will keep your code intact in your new custom module, and allow the Feeds iCal module to continue to receive updates as they are released. Otherwise you will need to refrain from updating the module which is not best practice.
@artinruins Thanks, does that imply we would need the legend as proposed in:
https://www.drupal.org/project/project_browser/issues/3282163
✨
Improve iconography usability by adding a legend
Needs work
Is there an approach where we use a button without a labeldby attribute, and perhaps place the title attribute on the button tag?
<button class="pb-project__status-icon" title="This module is covered by the Drupal Security Team."><img src="/modules/contrib/project_browser/images/blue-security-shield-icon.svg" class="pb-icon pb-icon--false pb-icon--status false" alt="Security Advisory Coverage" ></button>
@bnjmnm do you have other expectations here, or can anyone point to an existing core UI that does something similar? I thought the "Status report" page might but I don't see title attributes on those icons.
Working on these updates, including adding "title" as noted, as well as "tabindex" to enable tabbing. Is it appropriate to add "tabindex" here, or should we rather add an anchor tag which would support tabbing by default without the need for the tabindex. The links could go to relevant d.o docs.
ben.hamelin → made their first commit to this issue’s fork.
Hi @rockmachine - Just wanted to check in and see how things are working for you. Were any of these suggestions helpful?
@rockmachine Are you able to provide some details on the following?
- How many events are in the ICS feed? How large is the file?
- What is the data structure for one of your events? Can you share an example, as I'm interested to know how many properties are in each event.
- What version of PHP are you running?
- What are the system resources (RAM, CPU)?
- Can you share code snippets of what you changed? How did you make these changes, manually or did you use a custom sub class?
This certainly seems like it could be a result from adding the "additionalFields" properties in https://www.drupal.org/project/feeds_ical/issues/3358713 ✨ Add additional fields Fixed
We did address a similar report in https://www.drupal.org/project/feeds_ical/issues/3445551 🐛 PHP 8.2: Creation of dynamic property is deprecated Fixed
However, I'm also wondering how this might be working if each event has a large number of properties, and if there are many events, if we are simply seeing the performance issues in writing watchdog logs during the import. Assuming a high number of deprecation errors per event across a high volume of events would potentially result in a large volume of writes.
As a quick test, could you disable watchdog (Database Logging module) and PHP error logging? Then try the import again to see if there are improvements?
As a potential solution for this I am thinking we make the "additionalFields" handling optional for each feed via config. That would allow users to opt in to additionalFields only if needed.
Additionally the approach you have taken to extend the IcalItem class should allow you to define all expected fields, and hence remove the deprecation warnings.
Please take some time to provide feedback here and let me know if disabling the logging improves the performance at all.
@arunkumark make sure you have followed manual steps outlined in #12. There is manual data removal likely needed before you can uninstall.
Can you take a look at the MR? If this fix works for you I'll merge it in and tag 2.2.1 as the latest release.
Thanks!
Thanks @ryanrobinson_wlu! This was at least partially introduced in 2.2.0 with the "Add additionalProperties to IcalParser" fix.
I've added the missing properties you identified, as well as an exception to the additionalProperties logic to hopefully avoid these moving forward.
I also looked at the docs for " #[AllowDynamicProperties]" in https://php.watch/versions/8.2/dynamic-properties-deprecated but that just feels like hiding a potential issue.
@pfrilling for MR review
ben.hamelin → created an issue.
@tBKoT @rszrama I reviewed the patch in #51. It looks good to me, including the removal of the card_code from privateTempStore once it is no longer needed. I believe the need for this approach has been outlined throughout the comments, albeit these are dated and I'm not up to date on the latest Authorize.net API and documentation.
Committed and released with 2.2.0.
Thanks @KarlShea!!
@KarlShea - looks like we got that release in 3.4.0
https://github.com/u01jmg3/ics-parser/releases/tag/v3.4.0
We could plan to up the requirement here and release this as 2.2.0
ben.hamelin → created an issue.
@othermachines agreed, it boils down to what Gatsby needs to know in order to keep the site updated. That will vary from site to site, for example we don't build the menu with Gatsby but we do need paragraph and media data.
It's also worth reviewing this issue: https://www.drupal.org/project/gatsby/issues/3419600 💬 Fastbuilds logic and documentation update Needs review
I've proposed documentation updates to better align with actual module behavior. The "Build published" is another example of misleading or inaccurate docs. It's not checking for published status, but rather restricting to "NodeInterface" when that is checked. It doesn't make sense, and in general the docs seem more targeted at users who are leveraging Gatsby Cloud. For more custom solutions (we build and host our Gatsby site in a custom cloud environment) the docs and logic don't align.
I suggest unchecking the "Build Published" setting and then saving your menu item, to see if it shows up in the fatsbuild logs. If it does, it should get picked up by your next build.
Hi @lindsay.wils!
Please try the patch in #37 or #39. We're using 37 on our affected project.
Longer term we need to open a MR, get review and approval from the maintainers on this fix. It's been a while since I originally dug into this, but at that time I outlined the problem as best I could in comments #14 and #20. But I haven't revisited it since so don't know the current status of the Auth API.
Thanks for this work! We were seeing some PHP errors on occasion:
TypeError: Drupal\gatsby\GatsbyEntityLogger::getJson(): Argument #3 ($recursive) must be of type bool, null given, called in /var/www/html/web/modules/contrib/gatsby/src/GatsbyEntityLogger.php on line 82 in Drupal\gatsby\GatsbyEntityLogger->getJson() (line 247 of /var/www/html/web/modules/contrib/gatsby/src/GatsbyEntityLogger.php).
Added a test to resolve that here.
@dudrop We were seeing similar behavior for any non-node bundle with the "Build Published" setting enabled. If you look at the logic in the gatsby_entity_update() hook: https://git.drupalcode.org/project/gatsby/-/blob/2.0.x/gatsby.module?ref...
it's saying "If build published is set and this is not a node, don't do anything" which doesn't make sense to me logically but does explain why your entity changes aren't showing.
I would suggest trying to make your changes with the "build published" setting disabled. You should then see your entities in the logs at /admin/config/services/gatsby/fastbuilds/logs
Description in #13, #16 were our problem (no wrapper around {{ children }})
This fixed for us in custom theme template container.html.twig:
<div{{ attributes.addClass(classes) }}>{{ children }}</div>
ben.hamelin → created an issue.
ben.hamelin → made their first commit to this issue’s fork.
Just surfacing the change record here as it took a bit to dig out and hoping this helps someone!
https://www.drupal.org/node/3255994 →
I did not dig in to this much, but just noting that during an recent upgrade to 10.1 (from 9.5) this patch was breaking suggestions for views - existing template overrides (e.g. views-view-list--[machine_name]) were not being used. Removing the patch resolved for me.
I ended up following steps outlined in #2 to require this issue branch project directly. We have not tried the recent patches in #53
Thanks for these recent troubleshooting notes. In my case, the upgrade to simplesamlphp/composer-module-installer would not apply for some reason, so the deletion of /vendor/ after having set the new requirement on it to ^1.3 finally worked.
@DieterHolvoet Code review there looks good to me!
This patch / MR is against the development branch 1.0.x - there is no 1.1.x branch to match that tagged release. The tag for 1.1.0 is an older commit, and I couldn't get the patch to apply there.
We ideally would get a D10 release tagged with all updated code.
ben.hamelin → created an issue.
This issue fork is stale, I'm going to open new issue, forked from 1.1.0, and get this fix in.
ben.hamelin → made their first commit to this issue’s fork.
I am guessing this was an issue with the site we first encountered the issue on. It was a rather large site (60,000+ nodes) that had been migrated from D7 and had some legacy code and data. I'm not sure what led to the initial error but this definitely fixed the issue for us.
Happy to let this close out if you feel it's not worthwhile.
I am guessing this was an issue with the site we first encountered the issue on. It was a rather large site (60,000+ nodes) that had been migrated from D7 and had some legacy code and data. I'm not sure what led to the initial error but this definitely fixed the issue for us.
Happy to let this close out if you feel it's not worthwhile.
@DieterHolvoet what is the status of a new tagged release for D10?
ben.hamelin → created an issue.
Minor change in this patch so it applies to 3.x (file_url_transform_relative() method deprecation).
Just a quick question here as we are seeing this right now during some testing on a lower environment. In my case, the feed itself is the same (Feed ID 1) but the source file name was changed. During testing I reduced the size of the original file, and renamed it. Then after the next import, an item I expected to be unpublished was not.
My direct question is whether the name of the source file being changed would cause the node to be disassociated with the feed?
You bet, glad it's working for you!
@ryanrobinson_wlu Thanks for opening this up. I've just tagged a new release 2.1.0 that updates the requirement for johngrogg/ics-parser.
Can you confirm this updated version resolves your issue?
Looks like we need to up the version requirement for johngrogg/ics-parser to ^3
https://github.com/u01jmg3/ics-parser/issues/325
We have a need for this as well, our JSONAPI endpoints are behind HTTP Authentication
This is working as expected in latest version
@morganlyndel Thanks for the patch here, tested and committed to 2.x - cutting new release
ben.hamelin → made their first commit to this issue’s fork.
I believe this should be all set with latest release, closing
ben.hamelin → created an issue.
Tagged new release 2.0.5 - thanks all!
ben.hamelin → created an issue.
Just noting #9 works for us too, and a quick review of the patch looks good to me!
Thanks Peter and Duro - I'd not heard of Ludwig before.
This patch should work indefinitely, so if it's ok with you both I'll close this out and not introduce to the project.
Let me know if for some reason this won't work for you.
Please use the latest tagged release, 2.0.4 - that contains the fix and should resolve this issue. I'll postpone for now, please confirm if the update works. Otherwise we can debug further.
Sorry, actual filename should be
migrate_plus.migration.d7_views_migration.yml
ben.hamelin → created an issue.
ben.hamelin → created an issue.