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

I3412 built in advanced search form #3700

Merged
merged 8 commits into from
Aug 25, 2023

Conversation

maxkadel
Copy link
Contributor

Connected to #3412

Current status:

  • Styling parallels current advanced search
  • Builds initial advanced searches correctly, including selecting field for search and passing along booleans
  • Tests in spec/features/advanced_searching_spec.rb pass when run individually; however, not in the context of the whole suite, because the controller is not re-loaded, so the json dsl is not used when it should be. This should resolve if Move Flipflop out of controller #3695 is resolved.
  • From a search, clicking "Edit search" raises Error: Pivot Facet needs at least one field name:
  • From advanced search, does not create a "constraint-value" on the search results page (also true for Numismatics search)
  • When trying to remove a search constraint generated by regular search, raises RSolr::Error::Http - 400 Bad Request Error: Error when parsing json query, expect a json object here, but found : null

maxkadel and others added 6 commits August 21, 2023 10:36
- Still need fields to come from dropdown, rather than displaying all the fields at once.

Co-authored-by: Jane Sandberg <sandbergja@users.noreply.github.com>
Next steps:
Write a test in the component spec, make a parallel to the guided search fields partial that aligns with the new advanced search (e.g. "clause_1_field", etc.)

Co-authored-by: Jane Sandberg <sandbergja@users.noreply.github.com>
- Search form is searching correct fields
- Needs more testing
- Booleans are not actually hooked up correctly - not sure if this is possible within json syntax
- Locally there are some failing tests when I turn the feature on for everything, mostly in browse specs - don't know if this is related to local solr config change
Co-authored-by: Jane Sandberg <sandbergja@users.noreply.github.com>
Co-authored-by: Jane Sandberg <sandbergja@users.noreply.github.com>
@maxkadel maxkadel marked this pull request as ready for review August 24, 2023 15:07
@pulbot pulbot temporarily deployed to staging August 24, 2023 15:09 Inactive
@pulbot pulbot temporarily deployed to staging August 24, 2023 15:12 Inactive
@coveralls
Copy link

coveralls commented Aug 24, 2023

Coverage Status

coverage: 95.383% (-0.05%) from 95.433% when pulling 089b43e on i3412_built_in_advanced_search_form into f222853 on main.

Copy link
Member

@sandbergja sandbergja left a comment

Choose a reason for hiding this comment

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

Thanks, @maxkadel ! Just a few small cleanup suggestions.

spec/features/advanced_searching_spec.rb Show resolved Hide resolved
spec/features/advanced_searching_spec.rb Outdated Show resolved Hide resolved
maxkadel and others added 2 commits August 25, 2023 09:59
Co-authored-by: Jane Sandberg <sandbergja@users.noreply.github.com>
Copy link
Member

@sandbergja sandbergja left a comment

Choose a reason for hiding this comment

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

Thanks, @maxkadel ! 🦄🎉

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.

None yet

4 participants