False positives when matching theme based on handle

[BrunoRoemers]

Describe the bug
DSpace 7 has the ability to switch themes based on the handle of the current item/collection/community (or the handle of one of their parents). Whether a theme is a match, is ultimately decided here, however this logic contains a bug.

Example: given dso.handle = http://hdl.handle.net/123456789/123123 and theme.config.handle = 123456789/123, the theme should not be matched, but it will be matched because: http://hdl.handle.net/123456789/123123.

To Reproduce
Steps to reproduce the behavior:

1. configure a theme that matches on a handle, e.g. 123456789/123
2. create an item and edit the handle such that it equals the theme handle + something extra, e.g. 123456789/123123
3. visit the item and observe that the theme is active
   • you can inspect the Redux state with this extension to see which theme is active (theme.currentTheme, action of type dspace/theme/set has been dispatched here)

Expected behavior
The theme should NOT be active, because the theme handle does not match the item handle.

Suggestion
Add a utility function that given any string, returns a handle (in some canonical format) or undefined (if the input string could not be parsed). This function would become the single source of truth of what constitutes a valid handle.

[mwoodiupui]

Yes, the class HandleTheme looks unfinished: it takes any rather than DSpaceObject like the other matchers, and includes is clearly the wrong test for "will match the community, collection or item with the given handle" which calls for equality. (https://wiki.lyrasis.org/display/DSDOC7x/User+Interface+Configuration#UserInterfaceConfiguration-ThemeSettings)

[ybnd]

Note that

• HandleTheme.matches() uses any because handle is not a DSpaceObject property in Angular right now, it is a getter on some DSO subclasses
  I overlooked the way Item handles handles, do we have a reason not to move this down to DSpaceObject?
• Community & Collection use getters that return the dc.identifier.uri MDV instead, which may be changed & is therefore not even guaranteed to reflect the actual handle
• This is why .includes() was used. If we want leave the "handles" as is we'd want to use .endsWith() to match it fully.

I'm not exactly sure how we got to this weird situation, but I think this issue is a good opportunity to actually include handle in the Angular DSpaceObject, then we can solve all of this way more cleanly

[artlowel]

The reason handle isn't part of DSpaceObject is because there are a number of DSpaceObjects that don't have handles (e.g. Bundles and Bitstreams). The reason the matcher here uses any is because it is a shorthand way to cast the DSpaceObject from the superclass' method to any, which we need to do because otherwise the compiler would complain if we checked for the presence of the handle property. I guess we could create a HandleObject interface and make the type for dso in HandleMatcher something like T extends DSpaceObject & HandleObject

There is already a HandleService with a method to return a canonical form of a handle as suggested in the original description. Using it in the HandleTheme class will be a bit tricky as it isn't a service and therefore won't automatically have access to injection, but it's not impossible.

Atmire woud like to claim this ticket. It will take an estimated 4 hours. I realize it has medium priority but we have a client that would like to see it fixed.

[tdonohue]

@artlowel : I'm OK with bumping this up to high & letting it into 7.4 since it's a small task. Assigning to you & adding the estimate.