dspace-angular icon indicating copy to clipboard operation
dspace-angular copied to clipboard

Fixed authority ids being displayed on search page instead of their display value

Open alexandrevryghem opened this issue 1 year ago • 1 comments

References

  • Fixes #2804
  • Fixes #2813

Description

This PR fixes the problems with authorityKeys being displayed in the facets and the labels. Previously the selected FacetValues in the search sidebar were defined by going over all the FacetValues that were received from the backend and looping over all of them and checking if they were present in the url. This has now been simplified by simply using the appliedFilters array instead. The labels were also refactored, since they displayed the filter values from the url, which meant that they always showed authority ids instead of their display value/label.

Instructions for Reviewers

List of changes in this PR:

  • RouteService & SearchConfigurationService : These services have been updated to return an Observable of all the route Params except for a certain filter/value/operator combination. This logic was previously implemented in slightly different ways on multiple places: Search facet values, search facets applied filters, search facet suggestions and in the search labels. Moving this from the components into a common service will now ensure that they all behave in the same way and it results in less duplicate code.
  • SearchFacetFilterComponent#applyFilterValue: This method has been simplified and the wrong return type has been fixed (it was never a PaginatedList<FacetValue>[] but actually a FacetValues). It will now also emit the AppliedFilters to the SearchComponent.
  • SearchComponent: With the changes in SearchFacetFilterComponent#applyFilterValue it is now possible to pass along all the AppliedFilters to the labels (by doing it this way it also simplifies the way the search labels work since now the only need to display the given @Input()).
  • SearchLabelLoaderComponent & SearchLabelRangeComponent: Currently the backend still uses equals as the operator for range filters, because this new PR uses the AppliedFilters returned from the backend instead of the parameters who start with an f. in the url, the label would have looked like this: Date: [2020 TO 2024]. By setting the operator of range filters to range I was able to make the loader render the SearchLabelRangeComponent which can correctly split the [2020 TO 2024] value into 2 separate labels. The quick fix for this was to override the operator with range in Angular in the SearchRangeFilterComponent. This should however ideally be fixed in the backend but I first wanted to get feedback on this https://github.com/DSpace/dspace-angular/blob/10dfedd0a2ac5ced99086335f06d8895fe5c4c09/src/app/shared/search/search-filters/search-filter/search-range-filter/search-range-filter.component.ts#L123-L126

Guidance for how to test or review this PR:

  • Enable authority for dc.contributor.author by:
    • Uncommenting these lines in authority.cfg
      plugin.named.org.dspace.content.authority.ChoiceAuthority = \
      org.dspace.content.authority.SolrAuthority = SolrAuthorAuthority
      
      choices.plugin.dc.contributor.author = SolrAuthorAuthority
      choices.presentation.dc.contributor.author = authorLookup
      authority.controlled.dc.contributor.author = true
      authority.author.indexer.field.1=dc.contributor.author
      
    • Add authority to the list of consumers (event.dispatcher.default.consumers) in dspace.cfg
  • Create an item and fill in the dc.contributor.author with a text value
  • Create a second item and select the value you filled in in the previous item (you need to create a second item in order for that author to be displayed in the search facets, this is a backend bug I think)

Now test the following things:

  • Check that the label is now used in both the search facets & the search labels
  • Check that the pagination still works correctly in the facets
  • Check that selecting multiple authority values is possible and that you can correctly unselect them too
  • Check that the date facet & it's labels still work
  • This PR also fixes a performance bug, when you select the last date range in the date facet, currently the facets & results are retrieved twice, this is now also fixed

Checklist

  • [x] My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • [x] My PR passes ESLint validation using yarn lint
  • [x] My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • [x] If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • [x] If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • [x] If my PR fixes an issue ticket, I've linked them together.

alexandrevryghem avatar Feb 16 '24 15:02 alexandrevryghem

Hi @alexandrevryghem, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!

github-actions[bot] avatar Feb 21 '24 18:02 github-actions[bot]

@alexandrevryghem : Did you still want this considered for 8.0? If so, could you update this PR to remove the merge conflicts?

tdonohue avatar Apr 29 '24 22:04 tdonohue

Hi @alexandrevryghem, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!

github-actions[bot] avatar Apr 30 '24 17:04 github-actions[bot]