🇩🇪Germany @Harlor

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.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

It seems we have to adjust the tests as well

I just created a MR with the changes from message_notify-could_not_send_message-2936095-10.patch

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

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

harlor → created an issue.

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

harlor → created an issue.

Ah OK the tests did pass at a rerun now.

Back to needs review.

I'm not sure if the test failures have anything todo with the changes in this issue. I hope someone finds time to find this out.

I created a new MR based on 11.x with the diff from MR 3106.

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

harlor → created an issue.

harlor → created an issue.

Maybe we should move the menu migration to an optional sub module. There might be potential use cases where someone doesn't need a menu migration or menu_item_extras...

But that can wait... The code looks good

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

Since we only add tests I guess this won't break anything...

harlor → created an issue.

harlor → changed the visibility of the branch 3511208-add-dependency-on to hidden.

harlor → changed the visibility of the branch 3511208-add-dependency-on to active.

harlor → changed the visibility of the branch 3511208-add-dependency-on to hidden.

harlor → created an issue.

harlor → created an issue.

Good points!

harlor → created an issue.

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

harlor → created an issue.

+1 Any updates on this?