🇩🇪Germany @Harlor

harlor → created an issue.

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

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

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?

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

harlor → created an issue.

I guess we should focus on getting this into 2.x. Since there are some substantial changes this needs a reroll.

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

The Merge request https://git.drupalcode.org/project/disable_link_rel/-/merge_requests/2 works for me!

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

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

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

@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

harlor → created an issue.

LGTM

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

harlor → created an issue.

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.

Yep

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.

harlor → created an issue.

Indeed - good catch!

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

harlor → created an issue.

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

harlor → created an issue.

See https://github.com/goalgorilla/open_social/pull/4326

The changes look reasonable. Though I haven't tested it.

We found an issue with site path aliases - which I tried to fix

harlor → created an issue.

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.

harlor → created an issue.

harlor → created an issue.

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

harlor → created an issue.

Thank you that works!

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

Oh see: https://git.erdfisch.de/contrib/sites/-/merge_requests/87/diffs?commit_i...

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.

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

Fixed merge conflicts and got review from hydra.

If this still works we can merge this.

We have to do this tempstore keyspecific

'.sites_group:' . $group->id()

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

harlor → created an issue.