- 🇮🇳India dev16.addweb
silvi.addweb → made their first commit to this issue’s fork.
- Status changed to Needs review
4 months ago 12:51pm 4 July 2024 - 🇮🇳India dev16.addweb
Hello,
I created an MR and fixed phpcs problems. Review, please.
- 🇮🇳India riddhi.addweb
The mentioned page issue is resolved, & I have also checked and it is working as expected. I am attaching the Screenshots & doing RTBC for the same.
- Status changed to RTBC
4 months ago 12:47pm 5 July 2024 - Status changed to Needs work
3 months ago 7:11pm 30 August 2024 - 🇺🇸United States bburg Washington D.C.
I'm getting an error with the Merge Request branch, on php 8.3.
$ curl https://views-ical.ddev.site/events.ics The website encountered an unexpected error. Try again later.<br><br><em class="placeholder">ArgumentCountError</em>: Too few arguments to function Drupal\views_ical\ViewsIcalHelper::__construct(), 0 passed in /var/www/html/web/core/lib/Drupal/Component/DependencyInjection/Container.php on line 261 and exactly 2 expected in <em class="placeholder">Drupal\views_ical\ViewsIcalHelper->__construct()</em> (line <em class="placeholder">47</em> of <em class="placeholder">modules/contrib/views_ical/src/ViewsIcalHelper.php</em>). <pre class="backtrace">Drupal\Component\DependencyInjection\Container->createService() (Line: 179) Drupal\Component\DependencyInjection\Container->get() (Line: 92) Drupal\views_ical\Plugin\views\style\IcalWizard::create() (Line: 21) Drupal\Core\Plugin\Factory\ContainerFactory->createInstance() (Line: 83) Drupal\Component\Plugin\PluginManagerBase->createInstance() (Line: 824) Drupal\views\Plugin\views\display\DisplayPluginBase->getPlugin() (Line: 941) Drupal\views\ViewExecutable->initStyle() (Line: 921) Drupal\views\ViewExecutable->getStyle() (Line: 482) Drupal\views\Plugin\views\field\FieldPluginBase->defineOptions() (Line: 407) Drupal\views\Plugin\views\field\EntityField->defineOptions() (Line: 143) Drupal\views\Plugin\views\PluginBase->init() (Line: 110) Drupal\views\Plugin\views\HandlerBase->init() (Line: 142) Drupal\views\Plugin\views\field\FieldPluginBase->init() (Line: 218) Drupal\views\Plugin\views\field\EntityField->init() (Line: 902) Drupal\views\Plugin\views\display\DisplayPluginBase->getHandlers() (Line: 1099) Drupal\views\ViewExecutable->_initHandler() (Line: 957) Drupal\views\ViewExecutable->initHandlers() (Line: 2326) Drupal\views\Plugin\views\display\DisplayPluginBase->preExecute() (Line: 1751) Drupal\views\ViewExecutable->preExecute() (Line: 1686) Drupal\views\ViewExecutable->executeDisplay() (Line: 81) Drupal\views\Element\View::preRenderViewElement() call_user_func_array() (Line: 113) Drupal\Core\Render\Renderer->doTrustedCallback() (Line: 870) Drupal\Core\Render\Renderer->doCallback() (Line: 432) Drupal\Core\Render\Renderer->doRender() (Line: 248) Drupal\Core\Render\Renderer->render() (Line: 153) Drupal\Core\Render\Renderer->Drupal\Core\Render\{closure}() (Line: 638) Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 152) Drupal\Core\Render\Renderer->renderRoot() (Line: 110) Drupal\views\Plugin\views\display\Feed::buildResponse() (Line: 140) Drupal\views_ical\Plugin\views\display\IcalDisplay::buildResponse() (Line: 56) Drupal\views\Routing\ViewPageController->handle() call_user_func_array() (Line: 123) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638) Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181) Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76) Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53) Drupal\Core\StackMiddleware\Session->handle() (Line: 48) Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28) Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 191) Drupal\page_cache\StackMiddleware\PageCache->fetch() (Line: 128) Drupal\page_cache\StackMiddleware\PageCache->lookup() (Line: 82) Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48) Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51) Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 36) Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51) Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 741) Drupal\Core\DrupalKernel->handle() (Line: 19) - 🇺🇸United States bburg Washington D.C.
We weren't really using ViewsIcalHelper as a proper service before. This patch does that, but we weren't defining any arguments for ViewsIcalHelper in views_ical.services.yml. Adding:
arguments: ['@date.formatter', '@token.entity_mapper']
Sort of broke on me, because we have an implicit assumption that the token module is present and installed, but the views_ical module itself doesn't declare token as a dependency. I'm not really sure how to manage a soft dependency like that. While I'm hesitant to add new dependencies in general, token has over half a million installs, so this one might be alright. I had a question of what to do about the lock file update, since I can't really run composer commands to drupal module dependencies within the module, and it looks like the general practice is not to ship contrib modules with lock files. So I'll just remove that.
- 🇺🇸United States bburg Washington D.C.
Looking more at the patch, It's also adding another dependency to Smart Date via this "SmartDateTrait". Which we should only be optionally supporting if the site already has Smart Date installed, has configured an entity to use Smart Date fields, and then configured the ical view to use those fields. It shouldn't be declared as a trait to one of our classes like this, and I'm not really sure why it's even needed at all here.
As an aside, I get the impression that this patch was put together in an effort to farm contribution credit, and that the author(s) didn't have a local environment running, or make any attempt to test the patch in such an environment. If I see such posts in my module queue again, I will mark them as spam. If you use this module in your own projects, and are genuinely interested in contributing to it, then I am grateful for any effort there, but blindly posting untested patches just adds more work for maintainers to validate.