Yes it's an edge case, but with real world application on an education site i maintained.
My proposal was a single query per wrapper. I don't know how that looks in the code after 3 years, but I feel that the performance implications haven't been fleshed out. Also since it's been a problem since drupals in inception doesn't make it correct... But possibly an acceptable bug/problem.
At the very least we document and/report on the problem. Thoughts?
It has been done!
generalredneck → created an issue.
generalredneck → created an issue.
To test add
"commerce_timeslots": {
"type": "vcs",
"url": "https://git.drupalcode.org/issue/commerce_timeslots-3401757.git"
},
to your repositories
and require
composer require "drupal/commerce_timeslots:dev-3401757-date-picker-upgrade"
So when I did testing of the Drupal 11 version I was using datepicker 2.* because centarro/commerce-kickstart-project had it installed. I went to install the new 1.1.1 version and I got
Your requirements could not be resolved to an installable set of packages.
Problem 1
- Root composer.json requires drupal/commerce_timeslots ^1.1.1 -> satisfiable by drupal/commerce_timeslots[1.1.1].
- drupal/commerce_timeslots 1.1.1 requires drupal/jquery_ui_datepicker ^1.2 -> found drupal/jquery_ui_datepicker[dev-1.x, 1.2.0, 1.3.0, 1.4.0, 1.x-dev (alias of dev-1.x)] but it conflicts with your root composer.json require (^2.1).
The diff in the code is like so:
https://git.drupalcode.org/project/jquery_ui_datepicker/-/compare/8.x-1....
Even though there are a lot of changes... It seems to be working well for me at least on the checkout after the changes made in the latest release. I'm not sure if there are other places datepicker should be used besides in the cart pane.
I think that updating it to match kickstarter...
centarro/commerce-kickstart-project dev-main requires drupal/jquery_ui_datepicker (^2.1)
would work.
Makes sense. I didn't know there was a suggested changes feature in Gitlab... Neat. I applied them.
I want you to make a release too so I can continue working on McGreen Acres ecommerce solution! 😂
I put in a bunch of work just to try this thing out and I think it's gonna pay off!
Alright... I can confirm that the data-drupal-date-format attribute is no longer being added to the date field for what ever reason. My quick fix for this is to FORCE the attribute to be set to a fixed value. this specific piece was reported in ✨ Timeslot Widget does not update after a new date is set Active . That said, there was ALSO the 2 deprecations of Jquery Once and Modernizr. I've fixed the code so that it implements the recommendations based on the documentation at https://www.drupal.org/node/3158256 → . The comments above had the arguments swapped... therefore, my first commits to this MR did as well.
I've got the date picker working again as you can see in the screenshot and the dropdowns change value.
To be clear if you navigate to /admin/commerce/config/timeslots without the changes above, you get a WSOD that says
The website encountered an unexpected error. Try again later.
ArgumentCountError: Too few arguments to function Drupal\Core\Form\ConfigFormBase::__construct(), 1 passed in /app/web/modules/custom/commerce_timeslots/src/Form/TimeSlotSettingsForm.php on line 34 and exactly 2 expected in Drupal\Core\Form\ConfigFormBase->__construct() (line 44 of core/lib/Drupal/Core/Form/ConfigFormBase.php).
Drupal\commerce_timeslots\Form\TimeSlotSettingsForm->__construct(Object, Object) (Line: 42)
Drupal\commerce_timeslots\Form\TimeSlotSettingsForm::create(Object) (Line: 36)
Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition('\Drupal\commerce_timeslots\Form\TimeSlotSettingsForm') (Line: 48)
Drupal\Core\Controller\HtmlFormController->getFormObject(Object, '\Drupal\commerce_timeslots\Form\TimeSlotSettingsForm') (Line: 58)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 593)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 709)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
I believe what I may have been experiencing while working on this issue is related to ✨ Timeslot Widget does not update after a new date is set Active ...
We may have to break it down to see what we can get working... still JS work... still not my wheelhouse :/
To the best of my knowledge, and based on my testing, there is nothing in commerce 3 that is preventing this module from working. There are several other issues around the supported versions of Commerce 2 and Drupal 10 that are issues and preventing the module from working correctly, but Commerce 3 worked as cleanly as Commerce 2 for me using kickstart and this module.
The only changes we need to make is to bump the supported version number which I did in the MR.
So there are some Drupal 11 compatibility issues that need to be fixed. There is this change record https://www.drupal.org/node/3404140 → that was put into play: New parameter added to \Drupal\Core\Form\ConfigFormBase::__construct
It had to be fixed.
Additionally there are some lingering Drupal 10 deprecations that are out there. See 🐛 Uncaught ReferenceError: Modernizr is not defined at timeslots.js Active . This will need to be addressed as well before anyone can successfully use this module on drupal 11
Ugh, Javascript isn't my strong point. I'm really a backend developer.
I've managed to get rid of the errors, but I don't know how it's SUPPOSED to work.
There's no input field with data-drupal-date-format. So the datepicker isn't getting instantiated and the ajax calls aren't being hit on the date change...
I'm going to have to let someone else work on this part. I'm pushing up what I've done so far.
The other issues are going to be a part of https://www.drupal.org/node/3158256 → . Back in 9.2.0 Drupal core deprecated jquery once and the BC layer was removed from Drupal 10 completely.
I'm going to try and get a patch/mr in place. Moving this to Needs work as there is no patch or MR to review currently.
Additionally... based on https://www.drupal.org/node/3333253 → Modernizer library is deprecated as of Drupal 10.1.0.
generalredneck → created an issue.
Here's what I got after adding the bits in the MR to the code.
generalredneck → created an issue.
generalredneck → made their first commit to this issue’s fork.
I'm going to start by just upping the requirements and see what happens.
This should fix that. Give it a look see.
generalredneck → created an issue.
generalredneck → created an issue.
This is a super simple quick fix that does what we need it to do.
I see there is a related issue that's even older than this one.
With the way this is coded, any actual problems with the tags being attached would need to be handled in the Mailchimp module → upstream. I personally think this one is good to go, and is workin in my production instance at https://mcgreenacres.com/form/contact.
Adding credit for vandanasom.123 as for some reason it didn't get added automatically even though they made the MR.... Wierd
In the future please let the automation system move issues to "closed fixed" as per issue best practices
All I did was change the license that was added to composer.json given it was incorrect and used the one that is added by drupal's packagist magic.
Given that, since this issue was marked as RTBC before some support related issues from #19 on, I'm going to push this and get a new release out.
It is done
Hey in light of 💬 How to use? Active let's get strykaizer in here as he's already commented on it and some necessary fixes to make it work.
Over a week in and 2 yes. I wouldn't be able to get to either in a reasonable timeframe obviously.
Looking at the creds in your profile. I'm game. Would love to hear from one of the longer time maintainers like Joachim. Let's keep this open for 2 weeks per the typical project ownership guidelines and if we don't get a "no because" I'll grant you admittance.
generalredneck → created an issue.
I suspect this is because your minimal stability is set to stable. You may have to set that to alpha... though the composer dependencies in this case would be created by drupal's packagist magic. Another solution would be to require both commerce_pos and commerce_pos_keypad at the same version.
It appears this was resolved with the creation of 8.x-3.0
see https://www.drupal.org/project/commerce_pos/releases/3.0.0-alpha3 →
No problem. When I get a chance I'll get it merged in.
generalredneck → created an issue.
generalredneck → created an issue.
generalredneck → created an issue.
@dalin,
The first question is answered in the readme.
Only the most recent version of a node is reported on, so there is no
current focus on revisions.
No, it shows the sum of each bundle on each node. That may be a feature to ask for.
Rerolled with what was in 2.x
So container() is a private function, so I couldn't access it. That said, I had to increase the minimum requirements for this piece for Drupal 10.2. See the Change Record linked in the description.
I've tested this in both Drupal 10.3 and 11.0 and there were no problems. Gitlab_ci passed.
generalredneck → created an issue. See original summary → .
generalredneck → created an issue.
@thomwilhelm,
I think your patch contained parts of 🐛 update report data permission not working Fixed .
Implemented thomwilhelm's feedback into the MR. User 1 and role permissions are working as expected. Going to call this one good to go.
generalredneck → made their first commit to this issue’s fork.
Thanks a ton, marking as fixed.
If would check for !isset() or empty() in this case to make it more robust. we know that NULL == FALSE but not === as NULL is not . Also adding a comment so people understand why it's there.
@thomwilhelm,
Your patch doesn't apply cleanly against 2.x. I'm not sure, but you may have made the patch against 8.x-1.x?
In any case, I took my best attempt at merging what you had with what I had come up with before I found this issue. I added a configuration schema to square things away a bit too.
Care to give it a try? I used a different key name than you did when I initially did my changes. Let me know if it looks like I missed anything you had implemented.
generalredneck → made their first commit to this issue’s fork.
I pushed up the fix from ayush.pandey's patch because MR!12 did some reverts against some of the code changed in 2.x. Plus the patch works more completely.
Pushing the changes of MR!16 to 🐛 Why are entities saved in config ? Active
Hey, I appreciate both of yall taking a look at this. While identified areas where this would be a problem. I think the problem is bigger.
Specifically aisforaaron is storing the report state in the configuration entity. that does mean that any time you do a configuration export after content is updated (and you don't ignore this configuration entity) that it will be updated...
Example... after I run the report manually I get:
_core:
default_config_hash: WDVVbTde1eXP94qMDbx4NqQy4uOAEHLdyGsAcQBcF4o
content_types:
article: article
page: 0
hide_paras:
text: 0
import_rows_per_batch: '10'
watch_content: 0
report: '{"text":{"top":["1","2","3","4"]}}'
Prior to doing so, I get
_core:
default_config_hash: WDVVbTde1eXP94qMDbx4NqQy4uOAEHLdyGsAcQBcF4o
content_types:
article: article
page: 0
hide_paras:
text: 0
import_rows_per_batch: '10'
watch_content: 0
I propose that we use the State API → to store the report state instead of the Configuration Entity... and then we use abstracted retrieval of the state so that we can handle this error for all 3 places in one area. It's currently being handled inf ParagraphsReport::fetchJson(), but just not being used everywhere.
marking as fixed
I'm marking this one as outdated since it's for an old version and there hasn't been any movement here. I suspect that there wasn't an update to the configuration via an update hook given that it looks like this may be related to checking settings
In the words of HeMan
You have the power!
Sounds good. Never got anything from Kevin via contact form that I can find. Otherwise i would have done the same. Need to double check my notification settings on that issue queue cause I didn't see an email come through for the issue created
generalredneck → created an issue.
Adding a reroll for 10.3
I've had people here at work ask me "what are we patching". We are patching "symfony/framework-bundle" package with #31.
"symfony/framework-bundle": {
"#3452457: Fix Warming Up Route": "https://www.drupal.org/files/issues/2024-07-11/symfony_framework-bundle-fix_warming_up_routes-v6.4.9_backport-35d085d.patch"
},
Aknowledging mark_fullmer's message:
Reminder: the discussion in this issue does not involve code in the Drupal simplesamlphp_auth module. This is a discussion of a problem with Symfony and the SimpleSAMLphp library.
Don't let this backtrace full ya.
For those who need a text version of what callinmullaney posted in his image:
The debug information below may be of interest to the administrator / help desk:
SimpleSAML\Error\Error: UNHANDLEDEXCEPTION Backtrace: 1 src/SimpleSAML/Error/ExceptionHandler.php:32 (SimpleSAML \Error\ExceptionHandler::customExceptionHandler) [builtin] (N/A) Caused by: LogicException: The router "Symfony\Component\Routing\Router" cannot be warmed up because it does not implement "Symfony\Component\HttpKernel\Cache Backtrace: 6 /code/vendor/symfony/framework-bundle/CacheWarmer/RouterCacheWarmer.php:45 (Symfony\Bundle\FrameworkBundle\CacheWarmer \RouterCacheWarmer: :warmUp) 5 /code/vendor/symfony/http-kernel/CacheWarmer/CacheWarmer Aggregate.php:108 (Symfony\Component\HttpKernel\CacheWarmer\CacheWarmer Aggregate::warmUp) 4 /code/vendor/symfony/http-kernel/Kernel.php:552 (Symfony\Component\HttpKernel\Kernel::initializeContainer) 3 /code/vendor/symfony/http-kernel/Kernel.php:770 (Symfony\Component\HttpKernel\Kernel: :preBoot) 2 /code/vendor/symfony/http-kernel/Kernel.php:185 (Symfony\Component\HttpKernel\Kernel::handle) 1 src/SimpleSAML/Module.php:234 (SimpleSAML \Module: : process) e public/module.php:17 (N/A)
It might be good to have a task to rename the feature in other places, such as the tab on the content overview page. Going to put this in needs work for the tab name so i can at least remember we need to do this. Got this listed out in 🌱 2.0.x Road Map Active
I could get behind that. Not much longer. I was hung up on having "token" in there somewhere to describe what the list would contain.
I've marked this issue as fixed and added the following verbiage to the project page
If you are looking to install the Drupal 7 releases from this page, check out the All Releases → page to find the unsupported release, otherwise, drush make, and composer access is still available, Please note, you will receive a warning that these versions of the module is unsupported, and short of a major debilitating bug or security issue, there will be no maintenance by the module maintainers.
Hey there,
What has been done can't be undone. I'm sorry joseph.olstad.
Here's the message I get trying to restore the D7 release.
That said, I'll add some verbiage to the project page explaining that people can still find it under https://www.drupal.org/project/access_unpublished/releases?version=7.x → , but that we will not put any effort towards mantaining it unless there is a fatal bug or security issue raised about that version of the module.
generalredneck → created an issue.
I unfortunately can't go back and edit older versions, but I can however mark incompatibility going forward. The newer versions require Drupal 9.2 because of other changes as well.
I think this is the best I can do to resolve this issue. If you have better thoughts, feel free to reopen.
Currently, this module is only used to "view" existing content, not edit it or the like. Going to say negative to that as of now.
I believe this is a duplicate of ✨ Move Access Tokens UI to an entity Local Task / Tab Needs work . I'm going to mark it as such, but let me know if that's not the case.
@DamienMcKenna,
You have any thoughts? I noodled on a name for a while. Access Tokens is about as concise/meaningful as we can get without making that tab take up half the page.
Thinking of moving this into the new version and opening a new ticket for "moar better name" :D
generalredneck → created an issue.
I approve and good call.
generalredneck → made their first commit to this issue’s fork.
I don't believe the information here is relevant to this module. And the change to the module configuration settings form is no longer valid
@W01F,
Following up, Joachim was quick to respond. As far as we know there is no known limitation with last comment time. In what way are you using it and what results are you getting? What where you expecting? If we can reproduce what you are seeing we may be able to help.