When building up the search_admin form, hook_search_admin is invoked on selected modules allowing them to add module-specific configuration form elements. Because system_settings_form does no longer store configuration, implementors of hook_search_admin need to implement their submit-handler themselves. In order to make adding #submit-handlers from within hook_search_admin work, the main form must be merged with the module-specific forms using array_merge_recursive instead of array_merge.

In the long run it would probably be feasible to remove hook_search_admin entirely and instead tell other modules to form_alter if they need to. I suggest to attack that one in a follow-up issue if needed.

Files: 
CommentFileSizeAuthor
#16 1831802-16-allow-submit-on-hook-search-admin-tests-only.patch5.65 KBbnjmnm
PASSED: [[SimpleTest]]: [MySQL] 40,731 pass(es).
[ View ]
#12 allow-submit-on-hook-search-admin-d7-1831802-11.patch545 bytesbnjmnm
PASSED: [[SimpleTest]]: [MySQL] 40,387 pass(es).
[ View ]
#4 1831802-3-allow-submit-on-hook-search-admin-tests-only.patch4.52 KBznerol
FAILED: [[SimpleTest]]: [MySQL] 48,156 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#4 1831802-3-allow-submit-on-hook-search-admin.patch6.65 KBznerol
PASSED: [[SimpleTest]]: [MySQL] 49,862 pass(es).
[ View ]
#3 1831802-2-search-admin-merge-recursive.patch724 bytesznerol
PASSED: [[SimpleTest]]: [MySQL] 47,610 pass(es).
[ View ]
#1 1831802-search-admin-merge-recursive.patch728 bytesznerol
PASSED: [[SimpleTest]]: [MySQL] 47,601 pass(es).
[ View ]

Comments

Status:Active» Needs review
Issue tags:+Configuration system
StatusFileSize
new728 bytes
PASSED: [[SimpleTest]]: [MySQL] 47,601 pass(es).
[ View ]

Attach patch and set tag.

As discussed, I'm not sure if we should use NestedArray::mergeDeep() (or the drupal_array_merge_deep wrapper function) here.

I think we usually that one for forms as it could otherwise result in a messed up structure. Can you do a quick re-roll then I can RTBC this and we can unblock the issue that depends on this.

StatusFileSize
new724 bytes
PASSED: [[SimpleTest]]: [MySQL] 47,610 pass(es).
[ View ]

Done.

StatusFileSize
new6.65 KB
PASSED: [[SimpleTest]]: [MySQL] 49,862 pass(es).
[ View ]
new4.52 KB
FAILED: [[SimpleTest]]: [MySQL] 48,156 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

Attached is a test-only patch (which hopefully will turn red) demonstrating the problem and an updated patch including the tests and also updated hook-documentation in search.api.php.

We need the fix over here #1831632: Convert node_rank_ $rank variables to use configuration system.

Because form-submission now must be handled by search clients, this issue probably ought to have a change notice.

Status:Needs review» Needs work
Issue tags:-Configuration system

The last submitted patch, 1831802-3-allow-submit-on-hook-search-admin.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Reviewed & tested by the community

This looks good to me, assuming the patch still applies (re-tested already).

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)
Issue tags:-Configuration system+needs backport to D7

OK I don't understand why we have this hook at all, but this looks like it could use backporting to 7.x (although 'minor' for that), so I've committed this patch and opened #1891024: Remove hook_search_admin() in favour of hook_form_alter() to remove it in 8.x.

Assigned:Unassigned» bnjmnm
Issue summary:View changes

Working on 7.x backport.

Status:Patch (to be ported)» Needs review
StatusFileSize
new545 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,387 pass(es).
[ View ]

Backported to 7.x

Assigned:bnjmnm» Unassigned

Assigned:Unassigned» bnjmnm

Status:Needs review» Needs work

The tests would also need to be backported.

Status:Needs work» Needs review
StatusFileSize
new5.65 KB
PASSED: [[SimpleTest]]: [MySQL] 40,731 pass(es).
[ View ]

The test passes without making any changes to the search module, and manually performing the steps works fine as well. However, when I dsm($form) after adding module specific form elements and a new submit handler, only the new submit handler is present in the form array. Despite this being the only handler present, all changes appear to register after submitting the form. I'm not sure why that's the case.

I'm not sure what's going on... Someone reported this issue presumably because there was a bug somewhere. I don't know how to reproduce this bug... the test-only patch passing suggests that maybe there is not actually a bug to fix on this issue?

This was necessary for #1831632: Convert node_rank_ $rank variables to use configuration system. Therefore I consider this Drupal 8 only. I see no reason to backport it.

Version:7.x-dev» 8.x-dev
Status:Needs review» Fixed
Issue tags:-needs backport to D7

OK, until such time as someone protests that we need it in Drupal 7.x, I'm setting this back to 8.x/fixed and removing the backport tag.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.