Restored highlighting of title in search results #1973

corrad82-4s · 2022-11-21T15:43:11Z

References

Fixes Missing highlights in title in search results #1968

Description

Short summary of changes (1-2 sentences).
Title metadata value in search results list is filled using firstMetadataValue function, taking into account parts to be highlighted, if any.

Instructions for Reviewers

Please add a more detailed description of the changes made by your PR. At a minimum, providing a bulleted list of changes in your PR is helpful to reviewers.

List of changes in this PR:

item-search-result-list-element-component updated to use firstMetadataValue

Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

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.
My PR passes TSLint validation using yarn run lint
My PR doesn't introduce circular dependencies
My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

tdonohue

@corrad82-4s & @nikunj59: Finally tested this today. This works, but only for Items and Publication Entities. If the search result is any other type of Entity (e.g. Journal, Project, OrgUnit or Person) or is a Community or Collection, then the title highlighting still does not occur. For example, try searching for a person's last name and the Person Entity won't have its title highlighted. Or, try searching for a term in a Journal title, and again highlighting will not occur for a matching Journal Entity. Or search on a Collection title, and you'll see the Collection's title will not be highlighted in the results.

So, this appears to only be a partial fix. I think the issue may be in the dsoTitle (which seems to be used as the title of all objects in search results). By default that dsoTitle just get the name of the object, but in this scenario maybe there's a way we can dynamically update it to use the firstMetadataValue('dc.title')?? I admit though that I'm not sure how plausible that would be... I'm just pointing out that if you can fix dsoTitle directly, then it may fix this issue for all objects.

…gUnit or Person) or is a Community or Collection

nikunj59 · 2022-12-19T08:23:09Z

@tdonohue as Per your suggestion, updated changes: 247e61c

corrad82-4s · 2022-12-19T11:57:07Z

This PR has been updated with latest requested changes

tdonohue

@corrad82-4s and @nikunj59 : Re-tested this today. The behavior is better, but it's still not working for OrgUnits & Person Entities. I did notice those are the only two Entity Types where you switched away from using dsoTitle. Could that be the reason these don't work?

I have verified though that Community/Collection/Project/Journal/Journal Issue/Journal Volume/Publication/Item all work. It's just down to the last two entities.

nikunj59 · 2022-12-20T10:23:44Z

@tdonohue
can you please check if those orgUnits and person entities have hitHighlights from BE response?
I tested in local and its working fine with my person data.

In orgUnit dont have hitHighlights:

tdonohue · 2022-12-20T15:26:12Z

@nikunj59 : When searching Person Entities, I see the behavior in your first screenshot. If you look closely, the first person in your screenshot above should have hit highlighting. You searched on "test" and somehow it didn't highlight "Test, Test" (the first person in your list). If I similarly search on a Person's last name or first name, I cannot get hit highlighting to appear.

I can verify that in the REST API response, the hitHighlights are always null in these scenarios. So, it appears this PR may be all the fixes that are needed on the frontend. But we obviously still have bugs on the backend for Person and OrgUnit entities.

So, I can move this PR forward as-is. However, do you or @corrad82-4s plan to fix these backend bugs? Or should we create a new ticket to clarify that know issues with hit highlighting still occur for Person and OrgUnit entities?

tdonohue

👍 Changing my vote to a +1. This PR appears to have fixed the issues on the frontend.

However, Person and OrgUnit entities still do not show hit highlighting in the Name/Title field. This appears to be a backend issue though as the hitHighlights in the REST response is always null.

So, this fixes the frontend bug, but some known issues still remain.

corrad82-4s · 2023-01-03T12:10:02Z

@tdonohue : we are checking the backend issue. The cause seems to be a missing configuration. We are testing this updated configuration with this development. Once are tests will be run, we will update you through this PR

nikunj59 · 2023-01-06T11:10:51Z

@corrad82-4s @tdonohue
As per My investigation
For Person it comes in highlights as person.familyName, person.givenName with html tags.
For OrgUnit it comes in highlights as organization.legalName with html tags.
But we are using to highlight title this.firstMetadataValue('dc.title')

tdonohue · 2023-01-06T15:07:33Z

@nikunj59 and @corrad82-4s : As the hit highlights are coming through on a different metadata field, that implies that this PR may need to not assume that the highlighted field is always dc.title. The fields that are receiving hit highlights for Person and OrgUnit are the same fields which the DSONameService uses to display Person & OrgUnit names: https://github.com/DSpace/dspace-angular/blob/main/src/app/core/breadcrumbs/dso-name.service.ts#L28-L42

Maybe this PR should be refactored to somehow use DSONameService instead of only using the dc.title field?? Otherwise, it seems like hit highlighting will never work for objects which do not use dc.title.

I'm going to leave this PR open until you have a chance to reanalyze whether there's a way to refactor to use DSONameService. I suspect there should be, but I do realize it may take some closer analysis.

nikunj59 · 2023-01-09T06:32:21Z

@tdonohue
Thanks for clarification.
I'll cover responded field for Person and OrgUnit and refactor pull request.

nikunj59 · 2023-01-09T08:59:47Z

@tdonohue
in DSONameService factory you mentioned will return plain title because on the dso object which its processing is indexableObject.
https://github.com/DSpace/dspace-angular/blob/main/src/app/shared/object-list/search-result-list-element/search-result-list-element.component.ts#L35-L36

corrad82-4s · 2023-01-10T14:33:57Z

Hi @tdonohue ,
we have pushed latest required changes. A further REST PR (DSpace/DSpace#8626) has been created to enable hightlighting for Person and OrgUnits.

nikunj59 · 2023-01-10T14:35:10Z

@tdonohue
I Refactored PR.
Added function for gethithighlight https://github.com/DSpace/dspace-angular/pull/1973/files#diff-f47bc0e13fa389e84a9676ccae07ef8655f3e935cfbeabdf321e97de70d6a4a9R71-R111

tdonohue

👍 Thanks @nikunj59 and @corrad82-4s ! This works well now, and I appreciate that the logic has all moved into dso-name.service.ts alongside the getName() method there.

I was able to verify that all object types & all entity types now show hit highlighting in the displayed title/name.

nikunj59 added 2 commits November 18, 2022 16:03

[CST-7604] fixed text highlight on search page

7d58a1e

[CST-7604] fixed text highlight on search page

1ee3114

tdonohue assigned corrad82-4s Nov 28, 2022

tdonohue added improvement component: Discovery related to discovery search or browse system 1 APPROVAL pull request only requires a single approval to merge bug and removed improvement labels Nov 28, 2022

tdonohue added this to the 7.5 milestone Nov 28, 2022

tdonohue self-requested a review December 1, 2022 15:41

tdonohue reviewed Dec 14, 2022

View reviewed changes

[CST-7604] fixed as per tom comment to highlight Journal, Project, Or…

247e61c

…gUnit or Person) or is a Community or Collection

tdonohue reviewed Dec 19, 2022

View reviewed changes

tdonohue approved these changes Dec 20, 2022

View reviewed changes

Merge remote-tracking branch 'origin/main' into CST-7604

4645d4b

nikunj59 and others added 4 commits January 10, 2023 16:24

Merge remote-tracking branch 'origin/main' into CST-7604

fc2c7c5

[CST-7604] added hithighlight for person and orgunit

f9f3ad3

[CST-7604] added hithighlight for person and orgunit

b7b2803

Merge remote-tracking branch 'origin/CST-7604' into CST-7604

99f8a31

corrad82-4s mentioned this pull request Jan 10, 2023

Configuration updated to highlight Orgunit and Person DSpace/DSpace#8626

Merged

8 tasks

nikunj59 and others added 2 commits January 11, 2023 02:46

[CST-7604] test cases update

5660e7d

Merge remote-tracking branch 'origin/CST-7604' into CST-7604

fec3f09

tdonohue self-requested a review January 11, 2023 22:06

tdonohue approved these changes Jan 11, 2023

View reviewed changes

tdonohue merged commit 793bed3 into DSpace:main Jan 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restored highlighting of title in search results #1973

Restored highlighting of title in search results #1973

corrad82-4s commented Nov 21, 2022 •

edited by tdonohue

Loading

tdonohue left a comment •

edited

Loading

nikunj59 commented Dec 19, 2022 •

edited

Loading

corrad82-4s commented Dec 19, 2022

tdonohue left a comment

nikunj59 commented Dec 20, 2022

tdonohue commented Dec 20, 2022 •

edited

Loading

tdonohue left a comment

corrad82-4s commented Jan 3, 2023

nikunj59 commented Jan 6, 2023

tdonohue commented Jan 6, 2023

nikunj59 commented Jan 9, 2023 •

edited

Loading

nikunj59 commented Jan 9, 2023

corrad82-4s commented Jan 10, 2023

nikunj59 commented Jan 10, 2023 •

edited

Loading

tdonohue left a comment

Restored highlighting of title in search results #1973

Restored highlighting of title in search results #1973

Conversation

corrad82-4s commented Nov 21, 2022 • edited by tdonohue Loading

References

Description

Instructions for Reviewers

Checklist

tdonohue left a comment • edited Loading

Choose a reason for hiding this comment

nikunj59 commented Dec 19, 2022 • edited Loading

corrad82-4s commented Dec 19, 2022

tdonohue left a comment

Choose a reason for hiding this comment

nikunj59 commented Dec 20, 2022

tdonohue commented Dec 20, 2022 • edited Loading

tdonohue left a comment

Choose a reason for hiding this comment

corrad82-4s commented Jan 3, 2023

nikunj59 commented Jan 6, 2023

tdonohue commented Jan 6, 2023

nikunj59 commented Jan 9, 2023 • edited Loading

nikunj59 commented Jan 9, 2023

corrad82-4s commented Jan 10, 2023

nikunj59 commented Jan 10, 2023 • edited Loading

tdonohue left a comment

Choose a reason for hiding this comment

corrad82-4s commented Nov 21, 2022 •

edited by tdonohue

Loading

tdonohue left a comment •

edited

Loading

nikunj59 commented Dec 19, 2022 •

edited

Loading

tdonohue commented Dec 20, 2022 •

edited

Loading

nikunj59 commented Jan 9, 2023 •

edited

Loading

nikunj59 commented Jan 10, 2023 •

edited

Loading