Bug: theme matching by handle doesn't work, but uuid does

[misilot]

Describe the bug
Matching on a collection handle does not appear to work for a collection, but using a UUID does work (except for the duplicated elements #2346)

To Reproduce
Steps to reproduce the behavior:

themes:
  - name: etdr
    extends: krex
    handle: '2097/4'
    # uuid: '6d684812-6d29-4564-8b51-f5670957936b'

  # default theme, based on DSpace
  - name: krex

This works though

themes:
  - name: etdr
    extends: krex
    uuid: '6d684812-6d29-4564-8b51-f5670957936b'

  # default theme, based on DSpace
  - name: krex

Expected behavior
Theming should be applied to the collection items when using the handle, or the uuid.

Related work

[alexandrevryghem]

I tried the same with this config, and it worked (I also had to uncomment the CustomEagerThemeModule in src/themes/eager-themes.module.ts):

themes:
  - name: custom
    extends: dspace
    handle: '123456789/2'
  - name: dspace

Could you maybe go over these steps and check that your theme is correctly set up

[misilot]

@alexandrevryghem I believe it's setup correctly? The copy of dspace to krex seems to work (https://github.com/kstatelibrariesit/dspace-angular/tree/etdr-theme/src/themes/krex) but not the subtheme etdr

I made the repo public incase it would be easier to see where I might have messed up

[alexandrevryghem]

@misilot sorry for the late response I was on vacation. Can you try rebasing/merging your branch with the branch from the linked PR and check that both issues are now resolved?

[misilot]

@alexandrevryghem I was trying to apply this to the 7.6 branch I have, but it doesn't apply cleanly.

git am --show-current-patch=diff
---
 src/app/shared/theme-support/themed.component.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/app/shared/theme-support/themed.component.ts b/src/app/shared/theme-support/themed.component.ts
index 2ff0713f460..920fbba6e56 100644
--- a/src/app/shared/theme-support/themed.component.ts
+++ b/src/app/shared/theme-support/themed.component.ts
@@ -1,10 +1,10 @@
 import {
+  AfterViewInit,
   Component,
   ViewChild,
   ViewContainerRef,
   ComponentRef,
   SimpleChanges,
-  OnInit,
   OnDestroy,
   ComponentFactoryResolver,
   ChangeDetectorRef,
@@ -21,7 +21,7 @@ import { GenericConstructor } from '../../core/shared/generic-constructor';
   styleUrls: ['./themed.component.scss'],
   templateUrl: './themed.component.html',
 })
-export abstract class ThemedComponent<T> implements OnInit, OnDestroy, OnChanges {
+export abstract class ThemedComponent<T> implements AfterViewInit, OnDestroy, OnChanges {
   @ViewChild('vcr', { read: ViewContainerRef }) vcr: ViewContainerRef;
   protected compRef: ComponentRef<T>;

@@ -49,7 +49,7 @@ export abstract class ThemedComponent<T> implements OnInit, OnDestroy, OnChanges
     }
   }

-  ngOnInit(): void {
+  ngAfterViewInit(): void {
     this.destroyComponentInstance();
     this.themeSub = this.themeService.getThemeName$().subscribe(() => {
       this.renderComponentInstance();

is what git am isn't happy about

[alexandrevryghem]

I just tried merging in themed-component-fixes-main into etdr-theme and it succeeded without any merge conflicts

 git merge themed-component-fixes-main                                                                                                                                                                  
 Merge made by the 'ort' strategy.
 src/app/shared/theme-support/themed.component.ts | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

[misilot]

Thanks @alexandrevryghem so

  - name: etdr
    extends: krex
    uuid: '6d684812-6d29-4564-8b51-f5670957936b'

appears to work, but not

  - name: etdr
    extends: krex
    handle: '2097/4'

However, the duplicating theme element issue did go away!

[alexandrevryghem]

@misilot I couldn't reproduce this error. I tried with both dynamically created components like UntypedComponent and standard themable components like HeaderNavbarWrapperComponent, and I still wasn't able to reproduced this error I also tried running it in both prod and dev mode without any luck. Can you provide more info about how you noticed that it didn't work like for which component doesn't it work on your branch and what change would you expect where.

[misilot]

@alexandrevryghem

So I'm trying to add these additional metadata specific components to specific communities (the etdr theme)
https://github.com/kstatelibrariesit/dspace-angular/blob/etdr-theme/src/themes/etdr/app/item-page/simple/item-types/untyped-item/untyped-item.component.html#L65-L75

If I use in my config.yml file

themes:
  # Specific Item
  # - name: etdr
  #   extends: krex
  #   handle: '2097/43295'

  - name: etdr
    extends: krex
    handle: '2097/4'

  # default theme, based on DSpace
  - name: krex
    headTags:
      # Insert <link rel="icon" href="assets/krex/images/favicons/favicon.ico" sizes="any"/> into the <head> of the page.
      - tagName: link
        attributes:
          rel: icon
          href: assets/krex/images/favicons/favicon.ico
          sizes: any

I get the following results (not working)

However if I use

themes:
  - name: etdr
    extends: krex
    uuid: '6d684812-6d29-4564-8b51-f5670957936b'

  # default theme, based on DSpace
  - name: krex
    headTags:
      # Insert <link rel="icon" href="assets/krex/images/favicons/favicon.ico" sizes="any"/> into the <head> of the page.
      - tagName: link
        attributes:
          rel: icon
          href: assets/krex/images/favicons/favicon.ico
          sizes: any

It does work as expected. (The duplicative collections was me, since I was trying to prove something was working when I started down customizing it)

[alexandrevryghem]

@misilot I used your branch again to test it and I'm still not able to reproduce the error, I asked a colleague (@nona-luypaert) to also test it on your branch to be sure I didn't miss anything and she also wasn't able to reproduce your error. Have you already tried running yarn clean & removing the .angular folder?
https://github.com/DSpace/dspace-angular/assets/43750381/ccdffd5a-ccb0-497c-95c0-6c77a87b22e0

[misilot]

@alexandrevryghem I ran docker system prune to purge all of the build components, and I am still seeing it when I target a collection by the handle. Not sure if there is anything else I can test?

[alexandrevryghem]

Have you also tried running it locally using yarn start:dev/yarn start:prod, maybe it's docker related?

[misilot]

I don't have yarn on the local system. Let me try applying the changes on our test system and see what it does with built assets and pm2

…

On Tue, Aug 1, 2023 at 9:51 AM Alexandre Vryghem ***@***.***> wrote: Have you also tried running it locally using yarn start:dev/yarn start:prod, maybe it's docker related? — Reply to this email directly, view it on GitHub <#2348 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALBHSGYG6MCVIUZWEQLDPTXTEJX3ANCNFSM6AAAAAAZ2AQQYU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[misilot]

@alexandrevryghem So I did the following

• yarn clean
• rm -Rf .angular/
• yarn install
• yarn build:prod
• rsync -avzh /klib/build/dspace-angular/dist/ /klib/app/dspace-ui-deploy/dist/ --delete
• cd /klib/app/dspace-ui-deploy/
• pm2 restart dspace-ui.json (with handle filter for the etdr theme configured)
• Tested (did not work)
• modifed config/config.yml to enable the uuid filter
• Tested (did work)

[misilot]

@tdonohue I see that PR #2395 is linked to this. That PR addresses #2346

[alanorth]

I confirm that matching by handle in DSpace 7.6 doesn't work for me either.

[alexandrevryghem]

Hey @alanorth, are you able to reproduce it on the main branch or were you only able to reproduce it in your project? And if you were able to reproduce it on main, how do you run your dspace with yarn start:dev/yarn start:prod or with pm2? Because I wasn't able to reproduce it 😅

[misilot]

I'm using pm2 (I don't remember if I said that before)

…

On Thu, Sep 28, 2023 at 3:13 PM Alexandre Vryghem ***@***.***> wrote: Hey @alanorth <https://github.com/alanorth>, are you able to reproduce it on the main branch or were you only able to reproduce it in your project? And if you were able to reproduce it on main, how do you run your dspace with yarn start:dev/yarn start:prod or with pm2? Because I wasn't able to reproduce it 😅 — Reply to this email directly, view it on GitHub <#2348 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALBHSHXCAA4DH5RDZ7S7OLX4XK6VANCNFSM6AAAAAAZ2AQQYU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[alanorth]

Hi, I was testing via yarn start:dev, but I just tried via yarn start (production mode) and it definitely doesn't work for me on 7.6.

[alexandrevryghem]

@alanorth could you maybe give me your config.yml that you used on the main branch

[alanorth]

@alexandrevryghem I was working on DSpace 7.6, not main. But sure, this is my config.dev.yaml:

https://gist.github.com/alanorth/4d6e9a1db7a09567fd588d47ec34d8f9

[alexandrevryghem]

Good news, I was able to reproduce the error locally. The error only happens for clients who change their handle.canonical.prefix, because this regex expects all handle urls to have handle in front of the handle, but this means that urls like http://hdl.handle.net/xxx/yyy currently aren't supported. I will provide a PR for this

[misilot]

@alexandrevryghem glad that might fix the issue, but not sure it is the case since we access the url's like https://fqdn/handle/2097/4

Our handle.canonical.prefix=https://hdl.handle.net/ is though 🤷

[alexandrevryghem]

The handle that is used to find the correct theme isn't the one from the url, but the one in the metadata of the Community/Collection/Item. So in your case that regex will try to retrieve the handle from the url https://hdl.handle.net/2097/4. If you already want to test it on your local machine you can temporarily update the handle of your collection and replace https://hdl.handle.net/ with https://fqdn/handle/ and then you should be able to see the correct theme again.

[alexandrevryghem]

Hey @misilot, I apologize for the delay in finishing the PR, but my last few weeks have been quite busy 😅, but I was able to finish it today could you maybe take a look and check if it fixes your issue?

[misilot]

Thank you for fixing this @alexandrevryghem!