@mortona2k RE #9 I don't think I was thinking about that in this problem. But if it doesn't work now, it likely won't be fixed here...
Feel free to add tests if you care to see if it has affects on it but try to avoid scope creeping this, please
Thanks @garphy for taking a crack at it. I dropped it because my brain needed a break... I felt close but for some reason no dice! Thanks for the test alterations as well!
I haven't made a release, try the dev branch, I will make a new beta release when I finish the date_recur integration eval.
Thank you both, I have committed and will make a new release soon.
Thanks, @robbiehobby — the D10 patch worked great and completely bypassed the issue (good in our case)!
We encountered this because
Link Checker →
bot modified some links on nodes, resulting in multiple revisions sharing the same changed timestamp (as expected from an automated bot). At the same time, we had multilingual support enabled (due to search_api_solr dependencies), and we were also running d7_node_complete migrations.
The root of the problem was the comparison $this->value == $original_value, which assumes the values are different. In our case, up to six revisions shared the same changed timestamp!
During migration, this caused confusion: the second revision would get the current timestamp, the third would revert to the original migrated timestamp, the fourth would get the current one again, and so on—because the $original revision matched a previous one with the same changed value.
I recommend keeping encoding="UTF-8" in there — it ensures correct character decoding, avoids cross-platform inconsistencies, and even if XML parsers are required to assume UTF-8 by default, it’s still best practice to include it explicitly.
Oh that would help, yes please!
I am/was using it, it seemed to be doing what it needed too. A bit tricky with composer to get it installed with this patch, unless you know a workaround? Or should I manually install it to test?
I am quite sure I got this working in 10.x, but I put some time into making it work (whole time above was testing against 10.x ^^). Can I use 📌 Consider adding Drupal 10 support back to version 2 Active as my other issue?
I scope creep'd myself a bit here, but mostly to the letter of what the task set out to do, so I will commit this and keep moving forward. I hope this resolves more problems that it causes.
Would love to add more but we did all the things here so closing this as complete. Will create specific tests to add to our kickstart
While doing 📌 Audit and remove obsolete @todo comments and dead code Active I really questioned the value of this check. Didn't change it there but it might need thought.
@gilmord & @zero2one could one of your (or any others) give me some some minimum steps to reproduce the bug/feature this MR or patches are meant to do?
@gilmord is views_daterange_filters_daterange some contrib module you are trying to resolve along with this? Your comment is not clear but that is not a plugin from core so maybe it's meant to support this
https://www.drupal.org/project/views_daterange_filters →
joelpittet → created an issue.
Hiding patches to avoid confusion.
joelpittet → created an issue.
I’m thrilled to see patches that get this working, or at least partially working! The idea is really interesting, and I totally agree that’s how most calendars work.
@steven jones I added a test as I suggested in #62 plus what it should look like with this change as well. It's super bare bones but maybe give you the confidence to commit?
Couple of notes:
- the new options weren't defaulted so I added some defaults unioned on to prevent
Exception: Warning: Undefined array key "metadata"
issues from showing up after commit.
- The previous version of this didn't have
encoding="utf-8"so my test would likely fail in a test-only run. Though likely a good thing to have on XML so I didn't remove it from this MR.
@steven jones Thanks for the feedback. Reading between the lines, it sounds like the minimum needed here might be a test confirming that the XML response remains unchanged with the new feature settings unchanged. That would help ensure existing sites aren’t affected. Does that sound like the right approach? (Noting there are currently no XML output tests.)
joelpittet → changed the visibility of the branch 8.x-1.x to hidden.
joelpittet → changed the visibility of the branch 2886357-changing-default-response to active.
joelpittet → changed the visibility of the branch 2886357-changing-default-response to hidden.
I fixed the link and was targetting any you're "following" 😉 for any lurkers here to do the same!
@solideogloria Thanks so much for your help getting this through! I always thought (and mentioned in Drupal Slack #drupal-thanks channel yesterday) I should just follow your issues and help you get them over the finish line, as they are always great and well thought through. 🙏
https://www.drupal.org/project/issues/search?&pr[…]solideogloria&status%5B%5D=Open →
I guess it's ecosystem support for the custom gateway, not general support, but I am happy to chat in Slack if that is where you prefer.
Thanks @jsacksick — oh yes, that makes sense. We’re using 🐛 "Warning: session_id(): Cannot change session id" when cron runs to delete abandoned carts Needs review on 2.x, which explains the fatal error.
We collect some payment info locally (like home address) to retain important order details before they’re handed off to the offsite gateway (TouchNet upay proxy for UBC). I suppose we could move that to a custom pane — but would that avoid the error, or would we still hit There are no payment gateways available for this order. if the order is free in 3.x?
For context, these are event registrations for kids learning to code.
Also, regarding SupportsZeroBalanceOrderInterface — should the manual payment gateway implement that? Should we?
joelpittet → changed the visibility of the branch 3520313-deprecated-function-strendswith to hidden.
Sorry this has been a while, is this still a problem? The patch doesn't currently apply but I am wondering if others are running into this bug?
joelpittet → changed the visibility of the branch 8.x-1.x to hidden.
Anybody else watching this issue, feel free to give it a whirl. Thanks very much to @solideogloria for doing that manual test and finding the culprit.
@solideogloria Mind giving this a quick try to see if it does what you need? A proper test would be ideal, but honestly a quick manual check is good enough for me to feel okay committing it.
@solideogloria you are totally right, it's a balance because I am trying not to break existing fixes, also dates in PHP are hard, and the long functions don't help.
Here's a straight reroll as I could get it without fixing any of the coding standards. Attached is a diff of the diffs (as interdiff but it's not) for anybody who might blame the reroll for it not working as it might of before.
@loopy1492 please try the dev release, as there are lots of improvements in there.
Things have changed, I will close this as outdated, but if something from this is still important please let me know.
Thank you @johnv for helping pave this forward.
joelpittet → changed the visibility of the branch 8.x-1.x to hidden.
joelpittet → changed the visibility of the branch 3354820-do-not-add to hidden.
This is much apperciated, the patch didn't apply so I rewrote it a bit but otherwise it's as you suggested. Default is 'U'.
$storage_format = match ($datetime_type) {
DateTimeItem::DATETIME_TYPE_DATE => DateTimeItemInterface::DATE_STORAGE_FORMAT,
DateTimeItem::DATETIME_TYPE_DATETIME => DateTimeItemInterface::DATETIME_STORAGE_FORMAT,
default => 'U',
};
Thanks @rauch I have committed this change to the 8.x-1.x dev branch
@sorlov Thanks I committed your patch. I have committed it to the 8.x-1.x dev branch
@niko- if the other changes are needed, please create a separate issue.
@niko- those other changes seem out of scope.
joelpittet → made their first commit to this issue’s fork.
I'm not opposed to this change, though I would like to ensure I don't break it once it's committed and this is already a complicated module. Could you add a quick smoke test to ensure it continues to work as I untangle the @todos in the code base?
joelpittet → made their first commit to this issue’s fork.
joelpittet → made their first commit to this issue’s fork.
Argh, ok thank for chiming in so fast @solideogloria, I will spend some time testing and rebasing (because I committed the conflicting changes). Hopefully add a test in the process.
Sorry the code was a bit complex to review and now is way too far to merge, there was quite a few changes. I wonder if the changes can be broken down a bit more discretely so that they are easier to commit?
The merge conflicts are mostly in
src/Plugin/views/style/Calendar.php
The .ddev in .gitignore is no longer needed, and so hope this change wouldn't touch the .gitignore file.
Happy to write a test to ensure this bug doesn't continue to happen, but the changes are quite big from the looks of them, so not sure what to do here...
joelpittet → made their first commit to this issue’s fork.
Happy to entertain patches to improve this, as I don't use this. Please create an MR against 8.x-1.x branch.
joelpittet → created an issue.
The test failures on phpcs are unrelated to the patch. I am committing this to dev, thanks @ramil g for catching this.
joelpittet → made their first commit to this issue’s fork.
Aw man I was using this on Drupal 10 :(
Thanks for the fix, seems reasonable to leave early before this check for a /
Since @catch reverted the other issue, I caught up the branch to see if it now passes as I assume is expected now.
joelpittet → made their first commit to this issue’s fork.
Sorry if I sound defensive, I am in a rush to get out the door... I will look a bit later to see if I can understand a bit deeper what is going on between the two regressions. It feels like it needs to be some sort of fallback in my guess, but it is likely way more complicated than that.
@acbramley see the test we have here, there was a regression fixed here as well. The screenshot shows the field handler was copied to all the handlers where in D7 it was only on 2 (that's why it appears to be a copy/paste mistake, it wasn't copied over like that on any of the other related commits at the time, in my deep dive/hunt for the root of the problem). Removing them let the default shine through (EntityField in our case).
https://git.drupalcode.org/project/drupal/-/commit/d15e2ea581dac9e112f15...
The here is the field we are testing aggregation on (id).
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/views...
@catch, It looks like there may be a regression—thanks for catching that.
Ideally, we'd add more tests here to show the new regression, no? The test in here shows the regression we'd been living with for a while.
See the screenshot of the various problems this solves in #58 #2735997-58: Decimal separator and decimals settings ignored when aggregating decimal fields → on top of it looking to be a copy/paste mistake (I keep saying that but looking at the diff it might be hard to see what I am saying), I wish dawehner could confirm this... sigh
joelpittet → made their first commit to this issue’s fork.
joelpittet → made their first commit to this issue’s fork.
The formatting is a bit jarring but the code looks solid.
Related 📌 RenderElement is deprecated Active
That is reasonable and good to bump the version of Drupal up to 10.3 to coincide with this change.
joelpittet → created an issue.
That seems like a way forward here, thanks @joachim
joelpittet → created an issue. See original summary → .
joelpittet → created an issue.
Thanks for your help I finished off the last little bits here and committed this all.
Thanks, glad to be on the board of dependencies!
Thank you @anish.ir, I have merged this in.
📌 Add DDEV Drupal Contrib for ease-of-maintenance Active Hope you don't mind, I cloned this great issue summary for calendar.
Looks like this has been resolved, I did offer to co-maintain that module to help easier in the future. 📌 Offer to co-maintain Active
I know there are test failures. I will fix them in the parent issues. I made PHPCS more strict, copying from what we did in og → .
joelpittet → created an issue.
joelpittet → created an issue.
This is a quickfix but it needs a pager that reads the value too, though it is better than nothing as someone could write a Views argument or argument default plugin that would read from the query string.
Side comment in the issue summary I wrote ?month=2025-04 even though I know in D8+ the month arg is in Ym format. I would like to see this easily configurable because ?month=202504 is practical but not a great hackable/human parseable URL
joelpittet → created an issue.
Thanks @claudiu.cristea!