Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create a search page using Drupal views #4060

Merged
merged 38 commits into from
Feb 8, 2024
Merged

Conversation

tiffneybare
Copy link
Collaborator

@tiffneybare tiffneybare commented Nov 15, 2023

  • 15261
  • 12919

QA Steps

  •  Checkout the metastore_search_view_2 branch in your local DKAN
  • drush en sample_content metastore_facets
  • drush dkan:sample-content:create
  • drush cron
  • drush cim --partial --source=modules/contrib/dkan/modules/metastore/modules/metastore_search/config/install
  • Change the default theme to olivero
  • Go to /admin/structure/block
  • Place Categories, Tags, and Publishers blocks into the sidebar region, limit display to the /dataset-search page and save.
  • Go to /dataset-search
  • Confirm you see the 10 sample datasets
  • Confirm the results change as expected when facet checkboxes are checked
  • Confirm the dataset titles link to the dataset page

Notes

This introduces a breaking change to the search api config so existing sites will need to import the config manually IF they have not altered the search api config. And if this feature is desired on a site that has modifications to the dkan search api, they will need to be reconciled first with the changes this branch is adding.

@janette
Copy link
Member

janette commented Nov 16, 2023

This looks good, I don't think we should merge until we add a twig template for the results list.

@janette janette marked this pull request as draft December 12, 2023 22:21
@janette janette changed the title Creating a search api page using Drupal views Create a search page using Drupal views Dec 16, 2023
@janette janette force-pushed the metastore_search_view_2 branch 3 times, most recently from e19b60d to 56b5977 Compare January 5, 2024 00:09
@janette janette marked this pull request as ready for review January 5, 2024 01:54
Copy link
Member

@dafeder dafeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall. See two changes.

@@ -0,0 +1,34 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need this to be added to the DKAN composer.json, this won't get read be composer for a project

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an optional module that is available but not enabled by default, I think the follow up ticket is to create a second ddev site install command for a standing up a non-decoupled FE site. Or am I missing your point here?

@@ -26,6 +26,11 @@ field_settings:
datasource_id: dkan_dataset
property_path: modified
type: string
nid:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need an update hook to do the config import task automatically as well, you shouldn't have to run drush cim when you enable a module.

Copy link
Member

@dafeder dafeder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing -- maybe to discuss. How would one adapt this if they changed the schema or wanted different facets then those provided here - would you need to disable this module and write your own? may need more documentation

@dafeder dafeder merged commit c7eb1ac into 2.x Feb 8, 2024
12 checks passed
@dafeder dafeder deleted the metastore_search_view_2 branch February 8, 2024 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants