Problem/Motivation

Search module is being deprecated so move all the search related code from the Node module to the Search module.

    core/modules/node/config/install/core.entity_view_mode.node.search_index.yml
    core/modules/node/config/install/core.entity_view_mode.node.search_result.yml
    core/modules/node/config/optional/search.page.node_search.yml
    core/modules/node/config/schema/node.schema.yml

Steps to reproduce

Proposed resolution

Create a sub module search_node for the moved code.

Remaining tasks

Deprecate the search plugin
Other changes as needed
Move tests for node searching to the new module

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3587564

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

quietone created an issue. See original summary.

quietone opened merge request !15628

quietone’s picture

quietone commented

Moved user_search plugin and config from the user module to a sub module in Search, search_user. That is working and a manual test of updating worked as well.

Did the same for node module, moving that to search_node. A manual test of update fails, with The "node_search" plugin does not exist. There is also more search related code in the node module that should move.

quietone’s picture

quietone commented

There was a cspell error that was not detected by commit-code-check locally, It was in core/modules/search/modules/search_node/config/schema/search_node.schema.yml.

quietone’s picture

quietone commented
Title: Move search config to Search module » Move search functionality from node to Search module
Status: Active » Needs review
quietone’s picture

quietone commented
Issue summary: View changes

Setting to needs review to see how this is going. Reviews should pay attention to all the configuration changes. I am not sure they are correct.

There are tests in the Search module that can probably be moved to search_node, I just haven't looked at that yet.

smustgrave made their first commit to this issue’s fork.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Applied the change suggested by @berdir, did it directly in the UI as it was pretty straight forward. Tests are still clean and manually testing and update hook ran fine.

  • catch committed a1a05d18 on main 
    task: #3587564 Move search functionality from node to Search module

By...

catch closed merge request !15628

catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to main, thanks!

Will need a backport MR for 11.x

quietone opened merge request !15870

smustgrave’s picture

smustgrave commented
Status: Patch (to be ported) » Reviewed & tested by the community

Seems like a good backport to 11.x to me, all green.

catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 11.4.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and 11.4.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !15870

  • catch committed fa9f7873 on 11.4.x 
    task: #3587564 Move search functionality from node to Search module

By...

  • catch committed bcb66945 on 11.x 
    task: #3587564 Move search functionality from node to Search module

By...
smustgrave’s picture

smustgrave commented

Not sure if this should be re-opened but discovered an issue on 11.4.0-rc1

When I attempt to uninstall search. search_help and search_node get uninstalled and then I get an error that node_search plugin doesn't exist. Believe it's because the search_node did not cleanup the search page tied to it.

godotislate’s picture

godotislate
he/him
commented
Status: Fixed » Needs work

Wonder if this similar to #3581109: Move HelpSearch to Search sub-module in that the update hook that installs the new submodule needs to go in system.install, so that it runs before others.

Or maybe we just revert for now.
Can't revert since it's in RC1.

catch’s picture

catch
he/him
Primary language English
commented

Don't think we can revert this now it's already in the RC - people might already be running the RC.

Are we missing an update to set the new dependencies for the node search page?

Also think @godotislate is right that we need to move the update to system.install. will need to leave an empty update where it currently is.

smustgrave’s picture

smustgrave commented

I only found it after updating to rc1 on a lower environment.

We are migrating to Drupal 11 and taking the time to review some modules and was uninstalling search.

I would of thought uninstalling search_node would of deleted the content search page

godotislate’s picture

godotislate
he/him
commented
Issue tags: +11.4.0 release priority, +11.4.0 blocker

I think I misread #19 wrong, and I thought it was about the update process. I think the move to system.install might still be needed to prevent some other update hook rebuilding routing before search_node is installed.

But having issues uninstalled search after the update is a separate thing. After a quick look at the commit, I think the issue might be that the module dependency in search.page.node_search is not changed from node to search_node in active configuration.

godotislate opened merge request !16070

godotislate’s picture

godotislate
he/him
commented
Status: Needs work » Needs review

MR for changes per #23: https://git.drupalcode.org/project/drupal/-/merge_requests/16070

We probably need a manual test of an update from 11.4.0-rc1 to the MR branch as well.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

That was 100% the reason

smustgrave’s picture

smustgrave commented

Also realized the same may need to open for search_help right?

catch’s picture

catch
he/him
Primary language English
commented

@smustgrave did you also test this?

We probably need a manual test of an update from 11.4.0-rc1 to the MR branch as well.

I think we should do that to ensure that the new update runs fine on a site that has already run the previous update. It looks good to me, but so did the original update that we committed tbh.

smustgrave’s picture

smustgrave commented

@catch yup!

So I had already written a custom hook to delete the search pages so what I did was reinstall the 3 modules (search, search_node, search_help).
Exported the config
Altered the content page to depend on search
Imported the config
Tried to uninstall search and I get the error.
Applied the patch
Ran the update
Search uninstalls fine now.

godotislate’s picture

godotislate
he/him
commented

Also realized the same may need to open for search_help right?

I think search_help was fine because the search page for help already had a dependency on search.

  • catch committed 5ad58ce3 on main 
    task: #3587564 Move search functionality from node to Search module

By...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to main, thanks!

We need a backport MR for 11.x/11.4.x

catch closed merge request !16070

godotislate opened merge request !16076

godotislate’s picture

godotislate
he/him
commented
Status: Patch (to be ported) » Needs review

MR for 11.x https://git.drupalcode.org/project/drupal/-/merge_requests/16076
Applies to 11.4.x as well.

Not sure if the warning about the Validatable config job failure has any significance.

  • catch committed 35c628fb on 11.4.x 
    task: #3587564 Move search functionality from node to Search module

By...

catch closed merge request !16076

  • catch committed 92c94c6e on 11.x 
    task: #3587564 Move search functionality from node to Search module

By...
catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Fixed

Not sure if the warning about the Validatable config job failure has any significance.

Very much doubt it.

Committed/pushed the backport MR to 11.x and 11.4.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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