Pass enabled value to external links controller so it can be properly disabled

https://community.openproject.org/projects/openproject/work_packages/71946/activity

app/helpers/application_helper.rb

@@ -305,6 +305,7 @@ module ApplicationHelper
       controller: "application auto-theme-switcher hover-card-trigger beforeunload external-links highlight-target-element",
       relative_url_root: root_path,
       overflowing_identifier: ".__overflowing_body",
+      external_links_enabled_value: Setting.capture_external_links?,
       rendered_at: Time.zone.now.iso8601,
       turbo: local_assigns[:turbo_opt_out] ? "false" : nil
     }.merge(user_theme_data_attributes)

frontend/src/stimulus/controllers/external-links.controller.ts

@@ -70,6 +70,12 @@ const shouldProcessLink = (link:HTMLAnchorElement) => {
  * dynamically loaded content.
  */
 export default class ExternalLinksController extends ApplicationController {
+  static values = {
+    enabled: Boolean
+  };
+
+  declare readonly enabledValue:boolean;
+
   connect() {
     useMutation(this, { attributes: true, childList: true, subtree: true, attributeFilter: ['target', 'href'] });
 
@@ -77,9 +83,9 @@ export default class ExternalLinksController extends ApplicationController {
     document.querySelectorAll<HTMLAnchorElement>(LINK_QUERY).forEach((link)=>{
       if (!shouldProcessLink(link)) return;
 
-      if (isLinkBlank(link)) updateBlankLink(link);
+      if (isLinkBlank(link)) this.updateBlankLink(link);
 
-      if (isLinkExternal(link)) updateExternalLink(link);
+      if (isLinkExternal(link)) this.updateExternalLink(link);
     });
   }
 
@@ -89,16 +95,16 @@ export default class ExternalLinksController extends ApplicationController {
         if (isElement(node)) {
           // Added element itself is an external link
           if (isLink(node) && shouldProcessLink(node)) {
-            if (isLinkBlank(node)) updateBlankLink(node);
-            if (isLinkExternal(node)) updateExternalLink(node);
+            if (isLinkBlank(node)) this.updateBlankLink(node);
+            if (isLinkExternal(node)) this.updateExternalLink(node);
           }
 
           node.querySelectorAll<HTMLAnchorElement>(LINK_QUERY).forEach((link)=>{
             if (!shouldProcessLink(link)) return;
 
-            if (isLinkBlank(link)) updateBlankLink(link);
+            if (isLinkBlank(link)) this.updateBlankLink(link);
 
-            if (isLinkExternal(link)) updateExternalLink(link);
+            if (isLinkExternal(link)) this.updateExternalLink(link);
           });
         }
       });
@@ -110,28 +116,29 @@ export default class ExternalLinksController extends ApplicationController {
         isLink(mutation.target) &&
         shouldProcessLink(mutation.target)
       ) {
-        if (mutation.attributeName === 'target' && isLinkBlank(mutation.target)) updateBlankLink(mutation.target);
-        if (mutation.attributeName === 'href' && isLinkExternal(mutation.target)) updateExternalLink(mutation.target);
+        if (mutation.attributeName === 'target' && isLinkBlank(mutation.target)) this.updateBlankLink(mutation.target);
+        if (mutation.attributeName === 'href' && isLinkExternal(mutation.target)) this.updateExternalLink(mutation.target);
       }
     });
   }
-}
 
-function updateBlankLink(link:HTMLAnchorElement) {
-  // Ensure accessibility description
-  attributeTokenList(link, 'aria-describedby').add(BLANK_LINK_DESCRIPTION_ID);
-}
-
-function updateExternalLink(link:HTMLAnchorElement) {
-  // Ensure external link behavior
-  link.target = '_blank';
-  attributeTokenList(link, 'rel').add('noopener', 'noreferrer');
-
-  // Capture external links through redirect page
-  // The backend controller will redirect directly if the feature is disabled
-  if (!link.dataset.allowExternalLink) {
-    const originalHref = link.href;
-    const basePath = window.appBasePath ?? '';
-    link.href = `${basePath}/external_redirect?url=${encodeURIComponent(originalHref)}`;
+  private updateBlankLink(link:HTMLAnchorElement) {
+    // Ensure accessibility description
+    attributeTokenList(link, 'aria-describedby').add(BLANK_LINK_DESCRIPTION_ID);
   }
+
+  private updateExternalLink(link:HTMLAnchorElement) {
+    // Ensure external link behavior
+    link.target = '_blank';
+    attributeTokenList(link, 'rel').add('noopener', 'noreferrer');
+
+    // Capture external links through redirect page
+    // The backend controller will redirect directly if the feature is disabled
+    if (this.enabledValue && !link.dataset.allowExternalLink) {
+      const originalHref = link.href;
+      const basePath = window.appBasePath ?? '';
+      link.href = `${basePath}/external_redirect?url=${encodeURIComponent(originalHref)}`;
+    }
+  }
+
 }

spec/views/layouts/base.html.erb_spec.rb

@@ -38,6 +38,7 @@ RSpec.describe "layouts/base" do
   include Capybara::RSpecMatchers
 
   include Redmine::MenuManager::MenuHelper
+
   helper Redmine::MenuManager::MenuHelper
   let(:user) { build_stubbed(:user) }
   let(:anonymous) { build_stubbed(:anonymous) }
@@ -104,7 +105,7 @@ RSpec.describe "layouts/base" do
   describe "icons" do
     let(:current_user) { anonymous }
 
-    context "not in development environment" do
+    context "when not in development environment" do
       before do
         render
       end
@@ -189,12 +190,11 @@ RSpec.describe "layouts/base" do
     let(:a_token) { EnterpriseToken.new }
     let(:current_user) { anonymous }
 
-    context "EE is active and styles are present" do
+    context "when EE is active and styles are present", with_ee: %i[define_custom_style capture_external_links] do
       let(:custom_style) { create(:custom_style) }
       let(:primary_color) { create(:"design_color_primary-button-color") }
 
       before do
-        allow(EnterpriseToken).to receive(:allows_to?).with(:define_custom_style).and_return(true)
         allow(CustomStyle).to receive(:current).and_return(custom_style)
       end