That sounds right. I'll get fixes and tests committed to dev branch this weekend.
Good catch @isalmanhaider. I'll turn the patch into an MR and add a PHPUnit test for this condition within the next week.
The new MR for 11.3 worked for me, thank you @herved
I noticed this yesterday while testing an 11.2 to 11.3 upgrade. Thank you @alexpott and @berdir for your work on this.
We needed this after upgrading to Drupal 11.3.2, otherwise would get an exception from the menu_breadcrumb module for any page with a disabled menu item.
I'll post over there as well.
Taking another run at this, I'm trying to replicate the error but need more info. Can you clarify the data model in the case with a node reference. I'm assuming Event content type has unlimited reference to some other content type, call it SubEvent, and the SubEvent content type has a reference to a Registration field. SubEvent form display is configured to use Inline Entity Form - Settings. What is the form display for the main Event for its SubEvent field? What display is used for the Registration field? What exact sequence of steps gives the corrupt data. Thank you.
Commited to dev branch and will be in the next release. Granted credits. Thanks for catching this and for the help fixing @joelpittet!
john.oltman → created an issue. See original summary → .
@joelpittet I opened the MR on this issue. It's currently a one-liner. It will get bigger once I add tests for the affected class. With this fix in place, the conflict module works without any need for changes. As I mentioned in my post over there, a "wider impact" is not desirable because it could cause bugs in existing sites.
So the problem does indeed lie with the registration module. I posted the resolution over there and should have a fix committed within the next few days.
My concern with the proposed change to the conflict module as seen in the MR is there is an installed base of thousands of sites, hundreds of which probably have at least one computed field in their schema somewhere. Hard coding a change like this out of the blue, especially when the problem lies elsewhere to begin with, could create regressions for all those sites which rely on the existing behavior.
Regarding the issue in ✨ Allow some fields to be skipped Active , that is a Feature Request, not a bug, and after 5+ years of things working a certain way, I recommend providing an opt-out mechanism for fields through config or an alter hook, as mentioned on that issue, instead of hard coding the type of change in this MR. That way people have a mechanism for opting *any* field out, not just computed fields, which I think is the intention of that issue. This would have the side benefit that it would not change existing behavior for current users of the conflict module - they would have to take steps to activate the opt-out. If you want to do that, you would post an MR to that issue and not to this one.
Given the above, I propose you close this issue as "works as designed". Since you opened the issue @joelpittet, I'll leave it up to you as to how you want to handle it, but that would be my advice. Thanks much for catching and posting this!
This is a bug in the getValue method of the HostEntityItem class. I have a fix but need to add some tests. Will post an MR as soon as I can.
The getValue method is not used internally, so the flaw is only exposed if a different module calls it, as the conflict module does.
I'm going to re-title this issue so it is more clear where the problem lies.
Please hold on this until I determine if the registration module is at fault or not. Should have an answer within the next day or two.
Thanks Joel! I looked at your MR for the conflict module and it made me wonder why $field_item_list->getValue() returned an empty array for a settings entity that should have had a host entity returned instead. So it's not clear to me yet that conflict is at fault. Let me experiment with this and see what I can find.
Thanks for digging into it - I'll try and reproduce on my test D10 system as well
Thanks @joel - can you clarify - does the Event node still exist, or was that deleted along the way.
Thanks @richgerdes, your latest post makes sense to me.
With that many registrants, reminder emails are sent using a queue via Cron instead of immediately. And the Registration Notifier queue has a limit of 30 seconds per Cron run. So doing the math, if your system can send 3 emails per second, which seems plausible, then you'd be sending 90 per Cron run. So it would take 10 hours to send them all.
What I recommend is increase Cron to run once every 15 minutes, or even once every 5 minutes. That may cause some overhead stress for things other than email, in that case you can install the Ultimate Cron module and schedule most Cron tasks for hourly, but bump the Registration Notifier queue to run every 2 minutes. Running every 2 minutes would get all emails sent within 20 minutes of the reminder time. Since most of the time there are no emails to send, running every 2 minutes won't cause too much stress on your system.
Try one of those options and see if that helps.
I was referring to Drupal logs - watchdog logs. More questions:
* How many emails and users are we talking about?
* How often is Cron running?
* What is the schedule used by registration_scheduled_action?
Setting to support request until we know where the problem lies.
Thanks for the post, interesting report. There are a thousand users of the module and this is the first I've heard of this one. But let's dig into it. Questions:
* Do the logs indicate the emails are sent on time? Or the logs also indicate 10 hours later? It is likely email infrastructure if the former.
* Is the registration_scheduled_action module enabled?
* Do emails go out at the right time, and then again 10 hours later? Or just after 10 hours.
Let's get those answers and go from there.
john.oltman → created an issue. See original summary → .
This was not part of the recent 1.8.2 release, which is somewhat confusing since it was merged into the dev branch and the issue was closed prior to release 1.8.2. Is QA still testing it?
Thank you I joined the channel and should be able to make the Zoom
Hi Rich, thanks for taking this on, and I'm happy to assist in any way that I can. I'd like to learn more about your desired end state so I'll try and connect with you on Drupal Slack within the next few days. I agree that due to the scope of the changes, this should likely be part of a 4.x dev branch. Talk soon.
@drunken_monkey do you have a timetable for a new release of the module. Look forward to making use of this!
Turns out the first MR only got halfway home. The use statements for LegacyHook and new Hook were missing (which is why phpstan was complaining without the ignores). So the new OO hooks were not running. Once the use statements were added, the fact that the legacy implement alters must remain procedural, and the OO hook for it must use the Order attribute, were noticeable. The new MR gets all the way there.
The registration_module_implements_alter method is why tests fail without the handler logic. Working on a solution.
Thanks for your work on this @ritarshi_chakraborty! I saw that there were still a couple of coding standards violations, so I took care of those. I also got autowire working - you were close and backed it out because it was causing test failures, but it just needed to be tweaked, so I put that back in with the minor changes required.
I also experimented with not having the hook handler container logic, since I have not seen that pattern required in the documentation or in other modules. However it still failed, so I put your logic back in.
Currently the MR is passing green in all stages. I am researching whether there is a trick to getting the legacy hooks to work without the special container logic so tests pass. We'll see what I find. Keeping this at 'ready for review' for now.
https://www.drupal.org/project/drupal/issues/3442009 📌 OOP hooks using event dispatcher Needs review
Your answers all make sense and I think this is good to go. Also, I see why the MR is not showing up here, your branch is on the main repo instead of the issue fork. If it's easy to move the branch and update the existing MR to point to it without losing the MR comments and history, then I advise that - otherwise skip it. Thanks again.
This worked great for me in my test, the autosuggest core was copied to the appropriate server by the update hook and I manually removed the global setting after it completed. I then tested my autocomplete and it was still working. So a completely seamless experience. Fantastic.
I did have a couple of questions I left on the MR, one relating to the new legacy form alter hook and one relating to why the global setting was not removed by the update hook. Overall this is really great, very high quality work. Thank you!
john.oltman → changed the visibility of the branch 3551333-move-auto-suggest to active.
john.oltman → changed the visibility of the branch 3551333-move-auto-suggest to hidden.
Also I am curious why the MR does not show up here on the issue page, now that you took it out of Draft. But I can reach it from the link you gave me.
Thank you - there are test failures related to the number of parameters being passed to a constructor - can you fix those and then I'll give this a try.
Thanks for the additional info. The relationship is solid, but as you infer, the settings data still there causes it to resolve even if the host entity registration field is set to Disable Registrations. Thus your listing could show that registration is enabled (because the enabled box is checked in the old settings) for a host, when it actually isn't. I'm still wary of automatically deleting the settings in this case, since there may be registrations, and I don't think you are proposing those should be deleted too, and there should be symmetry between them.
What I am leaning towards is having options on the purger submodule to purge settings and purge registrations when the host it set to Disabled. So you would enable that submodule and check 2 boxes on that submodule's new settings page. The existing functionality would also become optional in that module as well (delete registrations and delete settings when a host is deleted). So there would be 4 checkboxes. Should be pretty clean so existing users get current behavior, but it is easy to get the alternate behavior you need.
Documentation is now available here:
https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... →
Help for the API and help for customizing views is still needed, but can be done at any time.