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

Changes for Elasticsearch 8 #271

Closed
wants to merge 39 commits into from
Closed

Conversation

wkdewey
Copy link
Contributor

@wkdewey wkdewey commented Nov 21, 2022

Replaces #263, based on new release branch.

Related to CDRH/api#123. This will introduce further breaking changes if included in the new release.

jduss4 and others added 30 commits November 21, 2022 09:50
other work in this commit:
  split facet translation functionality into own method
  basic code cleanup to match styleguide recommendations

v1.0 version sends facets with just key: value
v1.1 (future) will send key: { source: , num: }

backwards compatible:
- value_label deprecated but forwards input to new method
  so apps that overrode the search result items or item
  show metadata pages are OK largely

not so much backwards compatible:
- if entire files were overridden like facets and browse_facets
  then instead of a num the app will display a hash
remaining code should be API agnostic as far as display and queries
apps which have overwritten helpers may encounter errors
I dunno what I was thinking when I wrote this code, because
looking at it today I couldn't figure out why I thought that was
a good idea. Must have just not been paying attention.
Also fixes comment which used the wrong value

Robocop agrees that it is bad.
https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Lint/CircularArgumentReference
it's an odd bug...considering we've used the paginator a lot
but I suspect it was due to various parameters I was plugging in
trying to get some deprecation warnings to show up in query.rb
matches up with each other for one thing, less confusing for another
keyword facets now returning with markup of this kind
but without sanitize that is just being hidden
@page_facets exist when helper called by something
connected to typical items#index action, but if other
templates are using `value_label` / `metadata` etc,
they may not have defined any @page_facets and in that case,
the value should simply be returned as is
I'm guessing this was introduced here:
4079e00
but I am impressed we didn't notice until now
the issue was that if you had a 2 page search result,
the second page was not displaying because the "last" page was
not in the list of next pages, and the last page is also no longer
available by default

fixed by adding the total page number to the pages_next
and make sure to display dots if there are more results after
the current range max page
- ALL_LANGUAGES was not used in the application, but languages is
  so setting that config option instead
- the setup generator was creating files like "en|es.yml" due to
  not splitting on the pipe separator, this behavior is fixed
other work in this commit:
  split facet translation functionality into own method
  basic code cleanup to match styleguide recommendations

v1.0 version sends facets with just key: value
v1.1 (future) will send key: { source: , num: }

backwards compatible:
- value_label deprecated but forwards input to new method
  so apps that overrode the search result items or item
  show metadata pages are OK largely

not so much backwards compatible:
- if entire files were overridden like facets and browse_facets
  then instead of a num the app will display a hash
remaining code should be API agnostic as far as display and queries
apps which have overwritten helpers may encounter errors
matches up with each other for one thing, less confusing for another
keyword facets now returning with markup of this kind
but without sanitize that is just being hidden
@wkdewey
Copy link
Contributor Author

wkdewey commented Nov 30, 2022

cherry-picked the relevant commits into new_release_staging

@wkdewey wkdewey closed this Nov 30, 2022
@wkdewey wkdewey deleted the elasticsearch_upgrade branch November 30, 2022 16:49
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