dspace-angular
dspace-angular copied to clipboard
Fixed authority ids being displayed on search page instead of their display value
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 anObservableof all the routeParamsexcept 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 aPaginatedList<FacetValue>[]but actually aFacetValues). It will now also emit theAppliedFilters to theSearchComponent.SearchComponent: With the changes inSearchFacetFilterComponent#applyFilterValueit is now possible to pass along all theAppliedFilters 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 usesequalsas the operator for range filters, because this new PR uses theAppliedFilters returned from the backend instead of the parameters who start with anf.in the url, the label would have looked like this:Date: [2020 TO 2024]. By setting the operator of range filters torangeI was able to make the loader render theSearchLabelRangeComponentwhich can correctly split the[2020 TO 2024]value into 2 separate labels. The quick fix for this was to override the operator withrangein Angular in theSearchRangeFilterComponent. 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.authorby:- Uncommenting these lines in
authority.cfgplugin.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
authorityto the list of consumers (event.dispatcher.default.consumers) indspace.cfg
- Uncommenting these lines in
- Create an item and fill in the
dc.contributor.authorwith 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.
Hi @alexandrevryghem, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!
@alexandrevryghem : Did you still want this considered for 8.0? If so, could you update this PR to remove the merge conflicts?
Hi @alexandrevryghem, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!