OK basically this seems to work, but:
- We need to make sure the new dependency dynamic_entity_reference is installed correctly (I added an update script that should fix that).
- I don't understand the changes in slots_inline_block - Do we want to support other content types here too?
-- Either we should drop the support for other inline content entities (As the module name suggests)
-- Or this needs some work
This essentially reverts https://git.drupalcode.org/project/sites_group_overrides/-/commit/07b8a7... - maybe we should discuss this with Marc to get a solution that works for both cases.
harlor → changed the visibility of the branch 3549338-compatibility-with-sites to hidden.
Todos:
- Implement tests
- Try to use getWeight methods by initializing plugins
- Move trait methods to slots weight service
Ah and the MR contains the changes from ✨ Add admin views for slots Needs work so we should merge this one first.
If the implementation is approved - We should add some tests.
The last changes are not generic - and I'm not sure if they come from a problem in this module
I'm not a maintainer but my understanding is that Open Social is kept secure even though it is using an outdated drupal core version by applying patches for known security vulnerabilities.
See 📌 Drupal Core Security Update for OS 12 Active
I tried to fix 2 Issues:
1st: We shouldn't check all conditions in the slot Id views filter but only the slot id condition.
2nd: Some redirect made no sense or were broken (especially in LB)
1. According to EntityFormInterface the save() method has a return value of type int. (I brought that one back)
2. The return type of validateForm() and submitForm() is not specified on the FormInterface - so even though it doesn't seem to be used anywhere I'm wondering if it really hurts if we pass through the return value.
Did your IDE or some tool notify you about potential issues?
I guess we should focus on getting this into 2.x. Since there are some substantial changes this needs a reroll.
The Merge request https://git.drupalcode.org/project/disable_link_rel/-/merge_requests/2 works for me!
@bhogue, Which version of sites do you use?
Our customer tested this successfully.
Some changes are committed in 📌 Add alternate site URLs Active
Code looks good. Let's wait if our customers are happy.
This should be fixed by https://git.erdfisch.de/contrib/sites/-/merge_requests/101
We have a region_key setting now i sites core.
The settings API landed
LGTM
harlor → made their first commit to this issue’s fork.
harlor → created an issue.
I'd like to have a link to the klaro app form the SPA config form. On the klaro App form it would be nice to disable the fields that are written when the spa is saved.
I'd wait for the discussion in 🐛 Block Plugins don't store their context mappings on submit Needs work before we merge this - maybe the return type will be removed again...
@bhogue, I think your issue is caused by 🐛 Block Plugins don't store their context mappings on submit Needs work .
It's a bit weird that BlockBase does now specify a return type when PluginFormInterface does not. But as a workaround it should be totally fine to add this return type in SpaBlock.
I opened a separate issue for that: ✨ Allow to set an aria-label on the iframe element Active
harlor → created an issue.
Example in contrib: https://www.drupal.org/project/spa/issues/3526733#comment-16203161 ✨ Allow to set an aria-label on the iframe element Active (comment #8)
I'm wondering if we should specify the return type when there is no return type on the interface in this case PluginFormInterface yet.
Indeed - good catch!
Then 🐛 LdapUserLoginEvent isn't dispatched correctly Active might be the issue.
I think this is fine now.
Since its just adding a drush generate command and there are no changes of the existing code base I just merge this.
Which version of LDAP did you use? I ran into the following issue: https://www.drupal.org/project/ldap/issues/3534935 🐛 LdapUserLoginEvent isn't dispatched correctly Active
The changes look reasonable. Though I haven't tested it.
We found an issue with site path aliases - which I tried to fix
I think the interviewer could offer 3 options:
1: Decorate a specific form ID
2: Decorate a form based on a hook_form_FORM_ID_alter hook_form_alter...
3: Use a custom applies method
Sorry I meant:
#[FormDecorator('form_tfa_base_overview_alter')]
Looks good to me - I Just saw one little text that doesn't fit anymore
Ah yeah - currently you would have to use:
#[FormDecorator('tfa_base_overview_alter')]
The background was that I thought it might be convenient when one can read the attribute just like the hook_form_alter:
#[FormDecorator(hooK: 'tfa_base_overview_alter')]
But I'm actually not really happy with that...
I'm not sure why the generated form decorator did not get applied. Can you give details what went wrong?
Currently the applies method is added if the user does not choose a specific form_id or hook_form_alter equivalent. I mean we could just always generate the applied method. That would be fine for me.
Yeah the array key weight is a bit confusing. I think we can just change the to test_string.
Thank you that works!
I ran into a fatal in sites_path_alias when I tried deleting the relationship while being masqueraded - but it seems to be unrelated to these changes. I'll create an issue there.