🇨🇦Canada @joelpittet

Thanks for cleaning this up, passing to Alex as he might understand it and it appears I made it for him. I am sure if it’s important it will come back. Closing to help out the process a bit further

Seems to do the trick, thanks @ramil g

joelpittet → created an issue.

@fernly, feel free to jump on slack if you have any questions. I am around on the #og channel though sometimes more available than others (like right now on a ferry)

Thanks for moving that over! Would you mind creating a merge request (MR) for it? That way the tests can run, as patches no longer trigger DrupalCI now that things are moving to GitLabCI.

Unless there’s a specific reason, I’d prefer to see this against the 2.x branch — though I understand if you haven’t upgraded yet.

Of course this will likely need a test to prove that we are solving a problem.

Thanks for moving that over, it might also be a duplicate? 📌 OG Access port Active

I should make this more prominent on the homepage or something:
https://www.drupal.org/project/og_access →

joelpittet → created an issue.

joelpittet → changed the visibility of the branch 2.1.x to hidden.

It probably should be an either/or scenario. I’d hazard a guess that Monolog takes over a bit earlier in the logging chain (though I could be wrong). In my case, I had both enabled, and Monolog was beating EmailLog to the punch. Disabling Monolog got EmailLog working as expected.

Just chiming in as a user (not the maintainer) — I think this might be “working as designed,” even if the design isn’t ideal. Since the actual functionality and configuration live in the submodules, enabling only the main module gives the impression something should happen, but it doesn’t. That could lead to some confusion or a false positive, but technically it’s behaving as intended.

@poker10 I think I got the unrelated changes out of the MR. Also I agree removing the hook would be a BC break for people who may have fixed their noisy logs with it, so leaving it would be my preference.

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

Setting to Needs review to check out the merge request

@gngn great! Consider changing the status to "Reviewed & Tested by Community" to get this committed!

Need a second pair of eyes to confirm it does what it needs to before committing. So setting to Needs Review.

@herved I have committed it to 1.x too. Sorry I rushed and should have created a separate MR and just cherry-picked the 2 commits, a bit messy.

Thanks @loze, closing as outdated

crediting

Added a kernel test that exercises the new logic using node entities. I picked nodes because they’re the most common OG group type and we already have entity_test coverage elsewhere, so this gives us a bit of variety.
Happy to swap it to entity_test if you’d prefer uniformity—just let me know.
Test bot is green 🤖✅!

Moving to 2.x because that is where we are doing most of the dev. I feel like I would have run into and fixed this already, are the steps to recreate this accurate? I just created a webform without seeing the error in the issue summary.

The bigger question is because webform's aren't bundleable nor content entities, should they even see the options? I am not sure here, maybe @claudiu.cristea or @amitaibu can chim in?

Just a heads-up: assigning tasks to yourself can be a bit of a double-edged sword. It’s easy to lose track of them, and they can end up stalled. When in doubt, it’s usually better to leave them unassigned.

I am committing this to unblock the other tests, but please have a look at the tests or the views relationships if I made a mistake, though it seems to be doing the trick.

Thanks @alorenc I agree it's missing, I have merged that in to the dev branch.

Thanks @loze, I will merge that in a moment. It's tricky to know which things are serialized.

I did a related test over here, have a look maybe it can be piggybacked on ✨ Add views relationship to group entities from og_membership entities Needs work

joelpittet → created an issue.

@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 /