Merge branch 'dev' into bug/75031-imprecise-error-for-unallowed-ip-when-testing-jira-connection

This commit is contained in:
Andrej
2026-05-20 15:24:24 +02:00
committed by GitHub
28 changed files with 927 additions and 119 deletions
+2 -2
View File
@@ -428,6 +428,6 @@ gemfiles.each do |file|
send(:eval_gemfile, file) if File.readable?(file)
end
gem "openproject-octicons", "~>19.34.0"
gem "openproject-octicons_helper", "~>19.34.0"
gem "openproject-octicons", "~>19.35.0"
gem "openproject-octicons_helper", "~>19.35.0"
gem "openproject-primer_view_components", "~>0.85.0"
+7 -7
View File
@@ -904,10 +904,10 @@ GEM
validate_email
validate_url
webfinger (~> 2.0)
openproject-octicons (19.34.0)
openproject-octicons_helper (19.34.0)
openproject-octicons (19.35.0)
openproject-octicons_helper (19.35.0)
actionview
openproject-octicons (= 19.34.0)
openproject-octicons (= 19.35.0)
railties
openproject-primer_view_components (0.85.0)
actionview (>= 7.2.0)
@@ -1685,8 +1685,8 @@ DEPENDENCIES
openproject-job_status!
openproject-ldap_groups!
openproject-meeting!
openproject-octicons (~> 19.34.0)
openproject-octicons_helper (~> 19.34.0)
openproject-octicons (~> 19.35.0)
openproject-octicons_helper (~> 19.35.0)
openproject-openid_connect!
openproject-primer_view_components (~> 0.85.0)
openproject-recaptcha!
@@ -2066,8 +2066,8 @@ CHECKSUMS
openproject-job_status (1.0.0)
openproject-ldap_groups (1.0.0)
openproject-meeting (1.0.0)
openproject-octicons (19.34.0) sha256=4efe8a58a2d8051b79c94b37e9a7f04fd242a4da12b50f027c3c7f441a042adc
openproject-octicons_helper (19.34.0) sha256=12eb7af2214e21631369c76464ebaa30de788e1074c4b3bd0fcef7e74cb9edb4
openproject-octicons (19.35.0) sha256=a5033550d0961b4a8cb0993512a899716d633e17c2b5147bc6a9ed74f3952b38
openproject-octicons_helper (19.35.0) sha256=c32d142a4bb7fda739b16768aa8846fd88ffc1750509d8056f516056e8767361
openproject-openid_connect (1.0.0)
openproject-primer_view_components (0.85.0) sha256=16bc8358ef600f0465488a2e3c86991a9c69ed84580bd450c2dbec6f268eeaca
openproject-recaptcha (1.0.0)
@@ -37,8 +37,14 @@ module OpenProject
# header actions, collapsible behavior, and row rendering.
class BorderBoxListComponent < ApplicationComponent
include OpPrimer::ComponentHelpers
include Primer::FetchOrFallbackHelper
attr_reader :container, :collapsible, :current_user, :header_id, :footer_id
SCHEME_DEFAULT = :default
SCHEME_OPTIONS = [SCHEME_DEFAULT, :transparent].freeze
HEADER_PADDING_DEFAULT = :inherit
HEADER_PADDING_OPTIONS = [HEADER_PADDING_DEFAULT, :condensed, :default, :spacious].freeze
attr_reader :container, :scheme, :header_padding, :collapsible, :current_user, :header_id, :footer_id
alias_method :collapsible?, :collapsible
@@ -163,6 +169,13 @@ module OpenProject
# @param container [String, Symbol, Class, Object] value passed to
# `dom_target` to derive DOM ids for the list and related controls.
# @param scheme [Symbol] visual scheme. `:default` renders the standard
# BorderBox header. `:transparent` renders a transparent header with no
# separator line.
# @param header_padding [Symbol] optional vertical padding override for
# the header. `:inherit` keeps Primer's padding from the underlying
# BorderBox. `:condensed`, `:default`, and `:spacious` override only
# the header's block padding.
# @param interactive [Boolean] whether dynamic list updates should be
# announced politely to assistive technology. This affects the counter
# and an explicitly configured empty state; it does not create default
@@ -172,8 +185,10 @@ module OpenProject
# @param current_user [User] user context passed to work-package items.
# @param system_arguments [Hash] forwarded to `Primer::Beta::BorderBox`.
# Pass `id:` to set the box id; related ids are derived from it.
def initialize(
def initialize( # rubocop:disable Metrics/AbcSize
container:,
scheme: SCHEME_DEFAULT,
header_padding: HEADER_PADDING_DEFAULT,
interactive: false,
collapsible: false,
current_user: User.current,
@@ -182,6 +197,12 @@ module OpenProject
super()
@container = container
@scheme = ActiveSupport::StringInquirer.new(
fetch_or_fallback(SCHEME_OPTIONS, scheme, SCHEME_DEFAULT).to_s
)
@header_padding = ActiveSupport::StringInquirer.new(
fetch_or_fallback(HEADER_PADDING_OPTIONS, header_padding, HEADER_PADDING_DEFAULT).to_s
)
@interactive = interactive
@collapsible = collapsible
@current_user = current_user
@@ -189,6 +210,15 @@ module OpenProject
@system_arguments[:id] ||= dom_target(container)
@system_arguments[:list_id] = dom_target(@system_arguments[:id], :list)
@system_arguments[:classes] = class_names(
@system_arguments[:classes],
"op-border-box-list",
"op-border-box-list_transparent" => @scheme.transparent?,
"op-border-box-list_header-padding-condensed" => @header_padding.condensed?,
"op-border-box-list_header-padding-default" => @header_padding.default?,
"op-border-box-list_header-padding-spacious" => @header_padding.spacious?
)
@header_id = dom_target(@system_arguments[:id], :header)
@footer_id = dom_target(@system_arguments[:id], :footer)
end
@@ -8,11 +8,69 @@
// See COPYRIGHT and LICENSE files for more details.
//++
.op-border-box-list_transparent
> .Box-header
background-color: transparent
border-bottom: none
padding-bottom: var(--stack-padding-condensed)
> ul > .Box-row:first-of-type
border-top: none
.op-border-box-list
--op-border-box-list-header-row-gap: calc(var(--stack-gap-condensed) / 2)
&.Box--condensed
--op-border-box-list-header-row-gap: 0
&.Box--spacious
--op-border-box-list-header-row-gap: var(--stack-gap-condensed)
&.op-border-box-list_header-padding-condensed
--op-border-box-list-header-row-gap: 0
> .Box-header
padding-block: var(--stack-padding-condensed)
&.op-border-box-list_header-padding-default
--op-border-box-list-header-row-gap: calc(var(--stack-gap-condensed) / 2)
> .Box-header
padding-block: var(--stack-padding-normal)
&.op-border-box-list_header-padding-spacious
--op-border-box-list-header-row-gap: var(--stack-gap-condensed)
> .Box-header
padding-block: var(--stack-padding-spacious)
.Box-title
font-size: var(--text-title-size-medium)
&.op-border-box-list_transparent
&.op-border-box-list_header-padding-default,
&.op-border-box-list_header-padding-spacious
> .Box-header
padding-bottom: var(--stack-padding-condensed)
.op-border-box-list-header
display: grid
grid-template-columns: 1fr minmax(5rem, max-content) auto
grid-template-areas: "collapsible actions menu"
grid-template-columns: minmax(0, 1fr) minmax(5rem, max-content) auto
grid-template-areas: "heading actions menu" "description description description"
align-items: center
row-gap: var(--op-border-box-list-header-row-gap)
&_collapsible
grid-template-areas: "heading actions menu"
&--heading
min-width: 0
&--description
display: flex
align-items: center
gap: var(--stack-gap-normal)
color: var(--fgColor-muted)
&--actions,
&--menu
@@ -20,3 +78,11 @@
align-self: flex-start
// Unfortunately, the invisible button style bites us here again.
margin-top: -6px
&--heading-line
display: flex
align-items: center
gap: var(--stack-gap-condensed)
.Box-title
min-width: 0
@@ -27,56 +27,61 @@ See COPYRIGHT and LICENSE files for more details.
++# %>
<%= grid_layout("op-border-box-list-header", tag: :div) do |grid| %>
<% grid.with_area(:collapsible) do %>
<% if collapsible? %>
<%=
render(
Primer::OpenProject::BorderBox::CollapsibleHeader.new(
collapsible_id:,
collapsed:,
multi_line: true
)
) do |collapsible|
%>
<% collapsible.with_title(tag: title_tag) { title } %>
<% if render_count? %>
<% collapsible.with_count(**counter_arguments) %>
<% end %>
<% if description? %>
<% collapsible.with_description do %>
<%= description %>
<% end %>
<% end %>
<%= grid_layout(
"op-border-box-list-header",
tag: :div,
classes: { "op-border-box-list-header_collapsible" => collapsible? }
) do |grid| %>
<% if collapsible? %>
<% grid.with_area(:heading) do %>
<%=
render(
Primer::OpenProject::BorderBox::CollapsibleHeader.new(
collapsible_id:,
collapsed:,
multi_line: true
)
) do |collapsible|
%>
<% collapsible.with_title(tag: title_tag) { title } %>
<% if render_count? %>
<% collapsible.with_count(**counter_arguments) %>
<% end %>
<% else %>
<%= render(Primer::BaseComponent.new(tag: :div, classes: "CollapsibleHeader CollapsibleHeader--multi-line")) do %>
<%= render(Primer::BaseComponent.new(tag: :div, classes: "CollapsibleHeader-title-line")) do %>
<%= render(Primer::Beta::Truncate.new(tag: title_tag, classes: "CollapsibleHeader-title Box-title")) { title } %>
<% if render_count? %>
<%= render(Primer::Beta::Counter.new(**counter_arguments, classes: class_names(counter_arguments[:classes], "CollapsibleHeader-count"))) %>
<% end %>
<% end %>
<% if description? %>
<%= render(Primer::Beta::Text.new(color: :subtle, trim: true, classes: "CollapsibleHeader-description")) do %>
<%= description %>
<% end %>
<% if description? %>
<% collapsible.with_description do %>
<%= description %>
<% end %>
<% end %>
<% end %>
<% end %>
<% else %>
<% grid.with_area(:heading) do %>
<%= render(Primer::BaseComponent.new(tag: :div, classes: "op-border-box-list-header--heading-line")) do %>
<%= render(Primer::Beta::Truncate.new(tag: title_tag, classes: "Box-title")) { title } %>
<% if render_count? %>
<%= render(Primer::Beta::Counter.new(**counter_arguments)) %>
<% end %>
<% end %>
<% end %>
<% end %>
<% if actions? %>
<% grid.with_area(:actions) do %>
<% actions.each do |action| %>
<%= action %>
<% end %>
<% end %>
<% if description? && !collapsible? %>
<% grid.with_area(:description) do %>
<%= description %>
<% end %>
<% end %>
<% if menu? %>
<% grid.with_area(:menu) do %>
<%= menu %>
<% if actions? %>
<% grid.with_area(:actions) do %>
<% actions.each do |action| %>
<%= action %>
<% end %>
<% end %>
<% end %>
<% if menu? %>
<% grid.with_area(:menu) do %>
<%= menu %>
<% end %>
<% end %>
<% end %>
@@ -137,7 +137,7 @@ module Admin::Import::Jira
when Import::JiraClient::ApiError then t(:"admin.jira.test.api_error", status: error.status)
else
Rails.logger.error("Unexpected error testing Jira configuration: #{error.class} - #{error.message}")
t(:"admin.jira.test.error")
"#{t(:"admin.jira.test.error")}: #{error.message}"
end
render_error_flash_message_via_turbo_stream(message:)
end
+4
View File
@@ -268,6 +268,8 @@ module Import
raise SsrfError, I18n.t("admin.jira.client.ssrf_blocked")
rescue SsrfFilter::Error => e
raise ConnectionError, I18n.t("admin.jira.client.connection_error", message: e.message)
rescue OpenSSL::SSL::SSLError => e
raise ConnectionError, I18n.t("admin.jira.client.ssl_error", message: e.message)
rescue Timeout::Error => e
raise ConnectionError, I18n.t("admin.jira.client.connection_timeout", message: e.message)
ensure
@@ -292,6 +294,8 @@ module Import
raise SsrfError, I18n.t("admin.jira.client.ssrf_blocked")
rescue SsrfFilter::Error, SocketError, Errno::ECONNREFUSED, Errno::EHOSTUNREACH => e
raise ConnectionError, I18n.t("admin.jira.client.connection_error", message: e.message)
rescue OpenSSL::SSL::SSLError => e
raise ConnectionError, I18n.t("admin.jira.client.ssl_error", message: e.message)
rescue Timeout::Error => e
raise ConnectionError, I18n.t("admin.jira.client.connection_timeout", message: e.message)
end
+1
View File
@@ -168,6 +168,7 @@ en:
runs on an internal network, allow its IP via the OPENPROJECT_SSRF__PROTECTION__IP__ALLOWLIST
environment variable.
ssrf_block_doc_link: "Please see our [documentation](docs_url)."
ssl_error: "SSL error connecting to Jira server: %{message}"
parse_error: "Failed to parse Jira API response: %{message}"
api_error: "Jira API returned error status %{status}"
401_error: "Jira API returned a 401 error. Your authentication token may have expired or lack the required permissions. Please ensure the token belongs to a Jira administrator."
+7 -7
View File
@@ -56,7 +56,7 @@
"@ng-select/ng-option-highlight": "^21.8.2",
"@ng-select/ng-select": "^21.8.0",
"@ngneat/content-loader": "^7.0.0",
"@openproject/octicons-angular": "^19.34.0",
"@openproject/octicons-angular": "^19.35.0",
"@openproject/primer-view-components": "^0.85.0",
"@openproject/reactivestates": "^3.0.1",
"@primer/css": "^22.1.0",
@@ -7682,9 +7682,9 @@
"license": "BSD-3-Clause"
},
"node_modules/@openproject/octicons-angular": {
"version": "19.34.0",
"resolved": "https://registry.npmjs.org/@openproject/octicons-angular/-/octicons-angular-19.34.0.tgz",
"integrity": "sha512-RsTK48htb8zwb1C4M3quhZG6uGFWYPICR2rO9jckCpww4MgWQZKfFrSCH8r43+uOczjYorwktzn7CIJywGW9Rg==",
"version": "19.35.0",
"resolved": "https://registry.npmjs.org/@openproject/octicons-angular/-/octicons-angular-19.35.0.tgz",
"integrity": "sha512-oN6bkZeOcrWUAJtfuXsMHmHWpuJMxIt1gvToJpsDgOXFY9Wj1DVO2Di/hMYgG/8k+xv2UZ3kAgks43ENImgLmw==",
"dependencies": {
"tslib": "^2.3.0"
},
@@ -31149,9 +31149,9 @@
"integrity": "sha512-iFrvar5SOMtKFOSjYvs4z9UlLqDdJbMx0mgISLcPedv+g0ac5sgeETLGtipHCVIae6HJPclNEH5aCyD1RZaEHw=="
},
"@openproject/octicons-angular": {
"version": "19.34.0",
"resolved": "https://registry.npmjs.org/@openproject/octicons-angular/-/octicons-angular-19.34.0.tgz",
"integrity": "sha512-RsTK48htb8zwb1C4M3quhZG6uGFWYPICR2rO9jckCpww4MgWQZKfFrSCH8r43+uOczjYorwktzn7CIJywGW9Rg==",
"version": "19.35.0",
"resolved": "https://registry.npmjs.org/@openproject/octicons-angular/-/octicons-angular-19.35.0.tgz",
"integrity": "sha512-oN6bkZeOcrWUAJtfuXsMHmHWpuJMxIt1gvToJpsDgOXFY9Wj1DVO2Di/hMYgG/8k+xv2UZ3kAgks43ENImgLmw==",
"requires": {
"tslib": "^2.3.0"
}
+1 -1
View File
@@ -107,7 +107,7 @@
"@ng-select/ng-option-highlight": "^21.8.2",
"@ng-select/ng-select": "^21.8.0",
"@ngneat/content-loader": "^7.0.0",
"@openproject/octicons-angular": "^19.34.0",
"@openproject/octicons-angular": "^19.35.0",
"@openproject/primer-view-components": "^0.85.0",
"@openproject/reactivestates": "^3.0.1",
"@primer/css": "^22.1.0",
@@ -31,9 +31,6 @@
module OpenProject::TextFormatting
module Filters
class PatternMatcherFilter < HTML::Pipeline::Filter
# Skip text nodes that are within preformatted blocks
PREFORMATTED_BLOCKS = %w(pre code).to_set
class << self
def append_matcher(matcher)
matchers << matcher
@@ -45,15 +42,35 @@ module OpenProject::TextFormatting
end
def call
with_matcher_preloads(self.class.matchers.dup) { process_text_nodes }
doc
end
private
# Wraps the per-node loop in each matcher's `with_preloaded_resources`
# hook so matchers can warm per-render caches and save/restore them
# around nested `format_text` calls. Opt-in: matchers without the hook
# are skipped.
def with_matcher_preloads(matchers, &)
matcher = matchers.shift
return yield if matcher.nil?
if matcher.respond_to?(:with_preloaded_resources)
matcher.with_preloaded_resources(doc, context) { with_matcher_preloads(matchers, &) }
else
with_matcher_preloads(matchers, &)
end
end
def process_text_nodes
doc.search(".//text()").each do |node|
next if has_ancestor?(node, PREFORMATTED_BLOCKS)
next if has_ancestor?(node, OpenProject::TextFormatting::PreformattedBlocks::BLOCKS)
self.class.matchers.each do |matcher|
matcher.call(node, doc:, context:)
end
end
doc
end
end
end
@@ -79,8 +79,8 @@ module OpenProject::TextFormatting::Matchers
render_work_package_macro(display_id, detailed: detailed?)
else
# Plain link needs the WP record for the `formatted_id` label and
# hover-card URL. Unresolved → literal text rather than a broken URL.
wp = WorkPackage.find_by_display_id(display_id)
# hover-card URL. Cache miss → literal text rather than a broken URL.
wp = OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.work_package_for(display_id)
return nil unless wp
render_work_package_link(wp, fallback_id: display_id)
@@ -88,7 +88,7 @@ module OpenProject::TextFormatting::Matchers
end
def render_for_numeric(wp_id)
wp = WorkPackage.find_by_display_id(wp_id)
wp = OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.work_package_for(wp_id)
if quickinfo?
# Prefer the resolved WP's display_id so `##1234` typed in semantic
@@ -69,6 +69,9 @@ module OpenProject::TextFormatting
# identifier:version:1.0.0
# identifier:source:some/file
class ResourceLinksMatcher < RegexMatcher
WORK_PACKAGES_LOOKUP_KEY = :text_formatting_work_packages_lookup
private_constant :WORK_PACKAGES_LOOKUP_KEY
include ::OpenProject::TextFormatting::Truncation
# used for the work package quick links
include WorkPackagesHelper
@@ -127,6 +130,90 @@ module OpenProject::TextFormatting
]
end
# Returns the preloaded WorkPackage for the given identifier (numeric
# or semantic), or nil if no preload is active (classic mode, no `#N`
# references) or the WP couldn't be resolved.
def self.work_package_for(identifier)
RequestStore.store[WORK_PACKAGES_LOOKUP_KEY]&.[](identifier.to_s)
end
# Doc-level preload called by `PatternMatcherFilter`. Save/restores
# the lookup so a nested `format_text` (e.g. custom-field formatter
# re-entering the pipeline) doesn't clobber the outer render. Classic
# mode skips the load — `display_id` collapses to numeric, so the
# link handler can render from the matched id alone.
def self.with_preloaded_resources(doc, _context)
previous = RequestStore.store[WORK_PACKAGES_LOOKUP_KEY]
return yield unless Setting::WorkPackageIdentifier.semantic_mode_active?
identifiers = collect_work_package_identifiers(doc)
return yield if identifiers.empty?
RequestStore.store[WORK_PACKAGES_LOOKUP_KEY] = build_lookup(identifiers)
yield
ensure
RequestStore.store[WORK_PACKAGES_LOOKUP_KEY] = previous
end
def self.collect_work_package_identifiers(doc)
identifiers = Set.new
doc.search(".//text()").each do |node|
next if OpenProject::TextFormatting::PreformattedBlocks.ancestor?(node)
node.to_s.scan(regexp) do
extract_work_package_identifier(Regexp.last_match)&.then { identifiers << it }
end
end
identifiers
end
# Returns the WP identifier for any `#N` / `##N` / `###N` (or
# semantic-shape) reference. Returns nil for prefixed resource links
# (`version#3`, `message#12`) and `:`-separator resources. Leading-zero
# numerics ("0123") pass through here — the link handler rejects them
# at render time, so a non-resolving cache entry is harmless.
def self.extract_work_package_identifier(match)
parts = parse_match(match)
identifier = parts[:identifier]
return nil unless parts[:prefix].nil? && parts[:sep]&.start_with?("#") && identifier.present?
identifier
end
# 1 SELECT in the common case. A second targeted SELECT fires for
# historical aliases — the loaded WP row carries only its current
# identifier, so unmapped inputs must be filled in from
# `WorkPackageSemanticAlias`. The visible-id set passes through
# to the alias fold-in explicitly so each query enforces
# visibility at its own boundary, leaving no implicit trust for
# the cache to leak through.
def self.build_lookup(identifiers)
work_packages = WorkPackage.visible.where_display_id_in(*identifiers).select(:id, :identifier).to_a
lookup = index_by_id_and_identifier(work_packages)
fold_in_alias_keys(lookup, identifiers, visible_wp_ids: work_packages.map(&:id))
lookup
end
def self.index_by_id_and_identifier(work_packages)
work_packages.each_with_object({}) do |wp, lookup|
lookup[wp.id.to_s] = wp
lookup[wp.identifier] = wp if wp.identifier.present?
end
end
private_class_method :index_by_id_and_identifier
def self.fold_in_alias_keys(lookup, identifiers, visible_wp_ids:)
unmapped = identifiers.map(&:to_s) - lookup.keys
return if unmapped.empty? || visible_wp_ids.empty?
WorkPackageSemanticAlias
.where(work_package_id: visible_wp_ids, identifier: unmapped)
.pluck(:identifier, :work_package_id)
.each { |ident, wp_id| lookup[ident] = lookup[wp_id.to_s] }
end
private_class_method :fold_in_alias_keys
# Flattens the three alternation branches into a single `:sep` /
# `:identifier` shape so callers don't branch on which one matched.
def self.parse_match(match)
@@ -0,0 +1,52 @@
# frozen_string_literal: true
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
#++
module OpenProject
module TextFormatting
# `<pre>`/`<code>` ancestry skip. Filters call
# `has_ancestor?(node, BLOCKS)` via HTML::Pipeline's instance helper;
# matchers without that helper call `ancestor?(node)` here.
module PreformattedBlocks
BLOCKS = %w[pre code].to_set.freeze
module_function
def ancestor?(node)
ancestor = node.parent
until ancestor.nil? || ancestor.fragment? || ancestor.document?
return true if BLOCKS.include?(ancestor.name)
ancestor = ancestor.parent
end
false
end
end
end
end
@@ -32,17 +32,33 @@ module OpenProject
module Common
# @logical_path OpenProject/Common
class BorderBoxListComponentPreview < ViewComponent::Preview
DEFAULT_DESCRIPTION = "Coordinate launch work and keep stakeholders aligned."
TRANSPARENT_DESCRIPTION = "Sprint goals, scope, and timing for the next iteration."
PLAYGROUND_DESCRIPTION =
"Preview a longer header description to check wrapping, spacing, and alignment with actions in this list."
# @label Default
# @param padding [Symbol] select [default, condensed, spacious]
# @param header_padding [Symbol] select [inherit, condensed, default, spacious]
# @param description text
# @param interactive toggle
# @param collapsible toggle
def default(interactive: false, collapsible: false)
def default(
padding: :default,
header_padding: :inherit,
description: DEFAULT_DESCRIPTION,
interactive: false,
collapsible: false
)
render OpenProject::Common::BorderBoxListComponent.new(
container: "border-box-list-preview",
padding:,
header_padding:,
interactive: boolean_preview_param(interactive),
collapsible: boolean_preview_param(collapsible)
) do |list|
list.with_header(title: "Things we're building", count: true) do |header|
header.with_description { "There's lots to look forward to" }
header.with_description_content(description)
header.with_action_button do |button|
button.with_leading_visual_icon(icon: :pencil)
"Edit"
@@ -61,15 +77,64 @@ module OpenProject
end
end
# @label Transparent scheme
# @param padding [Symbol] select [default, condensed, spacious]
# @param header_padding [Symbol] select [inherit, condensed, default, spacious]
# @param description text
# @param interactive toggle
# @param collapsible [Boolean] toggle
def transparent(
padding: :default,
header_padding: :inherit,
description: TRANSPARENT_DESCRIPTION,
interactive: false,
collapsible: false
)
render OpenProject::Common::BorderBoxListComponent.new(
container: "border-box-list-transparent-preview",
scheme: :transparent,
padding:,
header_padding:,
interactive: boolean_preview_param(interactive),
collapsible: boolean_preview_param(collapsible)
) do |list|
list.with_header(title: "Sprint backlog", count: true) do |header|
header.with_description_content(description)
header.with_action_button do |button|
button.with_leading_visual_icon(icon: :rocket)
"Start sprint"
end
header.with_menu(button_aria_label: "Sprint actions") do |menu|
menu.with_item(label: "Edit sprint") do |menu_item|
menu_item.with_leading_visual_icon(icon: :pencil)
end
end
end
list.with_item { "User authentication stories" }
list.with_item { "Dashboard improvements" }
list.with_item { "API documentation" }
end
end
# @label With work package items
# @param padding [Symbol] select [default, condensed, spacious]
# @param header_padding [Symbol] select [inherit, condensed, default, spacious]
# @param interactive toggle
# @param collapsible toggle
def with_work_package_items(interactive: false, collapsible: false)
def with_work_package_items(
padding: :default,
header_padding: :inherit,
interactive: false,
collapsible: false
)
work_packages = WorkPackage.includes(:project).limit(2).to_a
return preview_message("No work packages in the database.") if work_packages.empty?
render OpenProject::Common::BorderBoxListComponent.new(
container: "border-box-list-work-package-preview",
padding:,
header_padding:,
interactive: boolean_preview_param(interactive),
collapsible: boolean_preview_param(collapsible)
) do |list|
@@ -79,22 +144,30 @@ module OpenProject
end
# @label Playground
# @param collapsible toggle
# @param title_tag [Symbol] select [h2, h3, h4, h5]
# @param count [Symbol] select [inferred, hidden, explicit, zero]
# @param count_scheme [Symbol] select [primary, secondary]
# @param hide_zero_count toggle
# @param padding [Symbol] select [default, condensed, spacious]
# @param header_padding [Symbol] select [inherit, condensed, default, spacious]
# @param description text
# @param interactive toggle
# @param collapsible toggle
def playground(
collapsible: false,
title_tag: :h4,
count: :inferred,
count_scheme: :primary,
hide_zero_count: true,
interactive: false
padding: :default,
header_padding: :inherit,
description: PLAYGROUND_DESCRIPTION,
interactive: false,
collapsible: false
)
render OpenProject::Common::BorderBoxListComponent.new(
container: "border-box-list-playground-preview",
padding:,
header_padding:,
interactive: boolean_preview_param(interactive),
collapsible: boolean_preview_param(collapsible)
) do |list|
@@ -108,7 +181,7 @@ module OpenProject
aria: { label: "Visible list item count" }
}
) do |header|
header.with_description { "Advanced header options" }
header.with_description_content(description)
end
list.with_item { "First item" }
@@ -119,11 +192,15 @@ module OpenProject
# @label Empty state
# List with a header and an empty state (Blankslate), no items.
# @param padding [Symbol] select [default, condensed, spacious]
# @param header_padding [Symbol] select [inherit, condensed, default, spacious]
# @param interactive toggle
# @param collapsible toggle
def empty_state(interactive: false, collapsible: false)
def empty_state(padding: :default, header_padding: :inherit, interactive: false, collapsible: false)
render OpenProject::Common::BorderBoxListComponent.new(
container: "border-box-list-empty-preview",
padding:,
header_padding:,
interactive: boolean_preview_param(interactive),
collapsible: boolean_preview_param(collapsible)
) do |list|
@@ -34,7 +34,9 @@ See COPYRIGHT and LICENSE files for more details.
container: inbox_container,
current_user:,
interactive: true,
scheme: :transparent,
padding: :condensed,
header_padding: :spacious,
test_selector: "backlog-inbox",
data: {
generic_drag_and_drop_target: "container",
@@ -44,6 +46,8 @@ See COPYRIGHT and LICENSE files for more details.
}
)
) do |list| %>
<% list.with_header(title: t(".title"), count: work_packages.size) %>
<% list.with_empty_state(
title: t(".blankslate_title"),
description: t(".blankslate_description"),
@@ -44,17 +44,14 @@ See COPYRIGHT and LICENSE files for more details.
) do |list| %>
<% list.with_header(title: sprint.name) do |header| %>
<% header.with_description do %>
<%= render(
Primer::Beta::Text.new(
color: :subtle, mr: 3, classes: "velocity",
aria: { live: "polite" }
)
) do %>
<%= render(Backlogs::SprintStatusBadgeComponent.new(sprint:)) %>
<%= render(Primer::Beta::Text.new(classes: "velocity", aria: { live: "polite" })) do %>
<%= story_points_total %>&nbsp;<%= t(:"backlogs.points_label", count: story_points_total) %>
<% end %>
<% if sprint.date_range_set? %>
<%= render(Primer::Beta::Text.new(color: :subtle, role: :group)) do %>
<%= render(Primer::Beta::Text.new(role: :group, ml: :auto, mr: 2)) do %>
<%= render(Primer::Beta::Octicon.new(:calendar, size: :small, mr: 1)) %>
<%= format_date_range([sprint.start_date, sprint.finish_date]) %>
<% end %>
@@ -63,7 +60,7 @@ See COPYRIGHT and LICENSE files for more details.
<% if show_start_sprint_action? %>
<% header.with_action_button(**start_sprint_button_arguments) do |button| %>
<% button.with_leading_visual_icon(icon: :play) %>
<% button.with_leading_visual_icon(icon: :rocket) %>
<% if start_sprint_disabled_reason.present? %>
<% button.with_tooltip(
text: start_sprint_disabled_reason,
@@ -75,7 +72,7 @@ See COPYRIGHT and LICENSE files for more details.
<% end %>
<% elsif show_finish_sprint_action? %>
<% header.with_action_button(**finish_sprint_button_arguments) do |button| %>
<% button.with_leading_visual_icon(icon: :check) %>
<% button.with_leading_visual_icon(icon: :"op-flag") %>
<%= t(".label_complete_sprint") %>
<% end %>
<% end %>
@@ -0,0 +1,5 @@
<%=
render(Primer::Beta::Label.new(size: :medium, font_weight: :bold, **@system_arguments)) do
I18n.t(:"activerecord.attributes.sprint.statuses.#{@sprint.status}")
end
%>
@@ -0,0 +1,52 @@
# frozen_string_literal: true
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
#++
module Backlogs
class SprintStatusBadgeComponent < ApplicationComponent
include OpPrimer::ComponentHelpers
def initialize(sprint:, **system_arguments)
super()
@sprint = sprint
@system_arguments = system_arguments.merge(bg: status_color)
end
private
def status_color
case @sprint.status
when "active" then :success
when "completed" then :done
else :default
end
end
end
end
@@ -62,12 +62,14 @@ module Backlogs
@system_arguments = system_arguments
@system_arguments[:padding] = :condensed
@system_arguments[:header_padding] = :spacious
merge_drag_and_drop_data! if drag_and_drop
@list = OpenProject::Common::BorderBoxListComponent.new(
container:,
current_user:,
interactive: true,
scheme: :transparent,
**@system_arguments
)
end
+3 -2
View File
@@ -132,6 +132,7 @@ en:
button_complete_sprint: "Complete sprint"
inbox_component:
title: "Inbox"
blankslate_title: "Backlog inbox is empty"
blankslate_description: "All open work packages in this project will automatically appear here."
show_more:
@@ -163,8 +164,8 @@ en:
blankslate_title: "%{name} is empty"
blankslate_description: "No items planned yet. Drag items here to add them."
label_actions: "Sprint actions"
label_start_sprint: "Start"
label_complete_sprint: "Complete"
label_start_sprint: "Start sprint"
label_complete_sprint: "Complete sprint"
start_sprint_disabled_reason_active_sprint: "Another sprint is already active."
start_sprint_disabled_reason_missing_dates: "Start and finish dates are required in order to start the sprint."
action_menu:
@@ -75,6 +75,27 @@ RSpec.describe Backlogs::InboxComponent, type: :component do
end
end
describe "header" do
let(:work_packages) do
[
create(:work_package, subject: "First item", project:, story_points: 2, position: 1),
create(:work_package, subject: "Second item", project:, story_points: 4, position: 2)
]
end
it "renders the inbox title" do
expect(page).to have_heading "Inbox", level: 4
end
it "renders the work-package count" do
expect(page).to have_css(
".Counter",
text: "2",
aria: { label: I18n.t(:label_x_items, count: 2) }
)
end
end
describe "empty state" do
let(:work_packages) { [] }
@@ -139,6 +160,14 @@ RSpec.describe Backlogs::InboxComponent, type: :component do
expect(page).to have_text("Show #{middle_count} more items")
end
it "renders the full work-package count in the header" do
expect(page).to have_css(
".Counter",
text: total.to_s,
aria: { label: I18n.t(:label_x_items, count: total) }
)
end
it "renders show-more targeting the full backlog turbo frame with all=1" do
show_link = page.find("##{show_more_id}")
expect(show_link[:href]).to include("all=1")
@@ -166,6 +166,44 @@ RSpec.describe Backlogs::SprintComponent, type: :component do
end
end
describe "sprint status badge in header description" do
context "when the sprint is in planning" do
let(:sprint) do
create(:sprint, project:, name: "Sprint 1",
start_date: Date.tomorrow, finish_date: Date.tomorrow + 7,
status: "in_planning")
end
it "renders the status badge" do
expect(rendered_component).to have_css(".Label", text: "In planning")
end
end
context "when the sprint is active" do
let(:sprint) do
create(:sprint, project:, name: "Sprint 1",
start_date: Date.yesterday, finish_date: Date.tomorrow,
status: "active")
end
it "renders the status badge" do
expect(rendered_component).to have_css(".Label", text: "Active")
end
end
context "when the sprint is completed" do
let(:sprint) do
create(:sprint, project:, name: "Sprint 1",
start_date: 2.weeks.ago, finish_date: 1.week.ago,
status: "completed")
end
it "renders the status badge" do
expect(rendered_component).to have_css(".Label", text: "Completed")
end
end
end
describe "sprint actions in header" do
context "when the sprint is in planning with date range set" do
let(:sprint) do
@@ -175,7 +213,7 @@ RSpec.describe Backlogs::SprintComponent, type: :component do
end
it "renders the start-sprint link enabled" do
expect(rendered_component).to have_link("Start")
expect(rendered_component).to have_link("Start sprint")
end
end
@@ -187,7 +225,7 @@ RSpec.describe Backlogs::SprintComponent, type: :component do
end
it "renders the start-sprint button as disabled" do
expect(rendered_component).to have_selector(:link_or_button, "Start", aria: { disabled: true })
expect(rendered_component).to have_selector(:link_or_button, "Start sprint", aria: { disabled: true })
end
end
@@ -200,7 +238,7 @@ RSpec.describe Backlogs::SprintComponent, type: :component do
let!(:task_board) { create(:board_grid_with_query, project:, linked: sprint) }
it "renders the complete-sprint link" do
expect(rendered_component).to have_link("Complete")
expect(rendered_component).to have_link("Complete sprint")
end
context "when params[:all] is true" do
@@ -210,7 +248,7 @@ RSpec.describe Backlogs::SprintComponent, type: :component do
it "preserves ?all=1 on the complete-sprint link" do
expect(rendered_component).to have_link(
"Complete",
"Complete sprint",
href: finish_project_backlogs_sprint_path(project, sprint, all: 1)
)
end
@@ -139,6 +139,12 @@ RSpec.describe Backlogs::WorkPackageCardListComponent, type: :component do
expect(rendered_component).to have_css(".Box-header")
end
it "keeps condensed row padding with spacious header padding" do
expect(rendered_component).to have_css(
".Box.Box--condensed.op-border-box-list_header-padding-spacious"
)
end
it "renders the provided title" do
expect(rendered_component).to have_heading "Sprint 1", level: 4
end
+21 -6
View File
@@ -77,7 +77,9 @@ module Pages
end
def sprint_names_in_order
page.find_all("#sprint_backlogs_container > section .CollapsibleHeader-title").map(&:text)
within_sprint_backlogs do
headed_section_titles(id_prefix: "backlogs-sprint-component-")
end
end
def expect_work_packages_in_sprint_in_order(sprint,
@@ -301,7 +303,7 @@ module Pages
def drag_inbox_item_to_sprint(work_package, sprint)
moved_element = find(draggable_work_package_selector(work_package))
target_element = find(sprint_selector(sprint))
target_element = find(list_body_selector(sprint_selector(sprint)))
wait_for_turbo_stream do
moved_element.native.drag_to(target_element.native, delay: 0.1)
end
@@ -311,7 +313,7 @@ module Pages
def drag_sprint_item_to_inbox(work_package)
moved_element = find(draggable_work_package_selector(work_package))
target_element = find("#inbox_project_#{project.id}")
target_element = find(list_body_selector("#inbox_project_#{project.id}"))
moved_element.native.drag_to(target_element.native, delay: 0.1)
rescue Capybara::Cuprite::ObsoleteNode
retry
@@ -335,7 +337,9 @@ module Pages
end
def bucket_names_in_order
page.find_all("#owner_backlogs_container section .CollapsibleHeader-title").map(&:text)
within_owner_backlogs do
headed_section_titles(id_prefix: "backlogs-bucket-component-")
end
end
def expect_bucket_names_in_order(*bucket_names)
@@ -423,7 +427,7 @@ module Pages
def drag_work_package_to_backlog_bucket(work_package, bucket)
moved_element = find(draggable_work_package_selector(work_package))
target_element = find(bucket_selector(bucket))
target_element = find(list_body_selector(bucket_selector(bucket)))
wait_for_turbo_stream do
moved_element.native.drag_to(target_element.native, delay: 0.1)
@@ -434,7 +438,7 @@ module Pages
def drag_work_package_to_backlog_inbox(work_package)
moved_element = find(draggable_work_package_selector(work_package))
target_element = find(backlog_inbox_selector)
target_element = find(list_body_selector(backlog_inbox_selector))
wait_for_turbo_stream do
moved_element.native.drag_to(target_element.native, delay: 0.1)
@@ -648,6 +652,17 @@ module Pages
test_selector("backlog-inbox")
end
def list_body_selector(container_selector)
"#{container_selector} > ul"
end
def headed_section_titles(id_prefix:)
page
.all(:section, section_element: :section, heading_level: 4)
.select { |section| section[:id].to_s.start_with?(id_prefix) }
.map { |section| section.first(:heading, level: 4).text }
end
def story_selector(story)
"#story_#{story.id}"
end
@@ -0,0 +1,101 @@
# frozen_string_literal: true
#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
#++
require "rails_helper"
require Rails.root.join("lookbook/previews/open_project/common/border_box_list_component_preview").to_s
RSpec.describe OpenProject::Common::BorderBoxListComponentPreview, type: :component do
it "renders a realistic default preview description" do
render_preview(:default, from: described_class)
expect(page).to have_text("Coordinate launch work and keep stakeholders aligned.")
end
it "renders the default preview with the provided description text" do
render_preview(
:default,
from: described_class,
params: {
description: "Default preview description that demonstrates how text wraps near the header action buttons."
}
)
expect(page).to have_text(
"Default preview description that demonstrates how text wraps near the header action buttons."
)
end
it "renders a realistic transparent preview description" do
render_preview(:transparent, from: described_class)
expect(page).to have_text("Sprint goals, scope, and timing for the next iteration.")
end
it "renders the transparent preview with the provided description text" do
render_preview(
:transparent,
from: described_class,
params: {
description: "Transparent preview description that demonstrates how text wraps near the sprint action buttons."
}
)
expect(page).to have_text(
"Transparent preview description that demonstrates how text wraps near the sprint action buttons."
)
end
it "renders the playground preview with the provided description text" do
render_preview(
:playground,
from: described_class,
params: {
description: "A longer playground description that demonstrates wrapping behavior in the list header preview."
}
)
expect(page).to have_text(
"A longer playground description that demonstrates wrapping behavior in the list header preview."
)
end
it "renders the playground preview with a header padding override" do
render_preview(
:playground,
from: described_class,
params: {
padding: :condensed,
header_padding: :default
}
)
expect(page).to have_css(".Box.Box--condensed.op-border-box-list_header-padding-default")
end
end
@@ -195,7 +195,7 @@ RSpec.describe OpenProject::Common::BorderBoxListComponent, type: :component do
end
expect(rendered).to have_heading("My title", level: 4)
expect(rendered).to have_text("Some description")
expect(rendered).to have_css(".op-border-box-list-header--description", text: "Some description")
end
it "renders multiple action buttons" do
@@ -766,5 +766,89 @@ RSpec.describe OpenProject::Common::BorderBoxListComponent, type: :component do
expect(rendered).to have_css("collapsible-header")
end
it "adds a collapsible modifier without rendering a grid description container" do
rendered = render_inline(
described_class.new(container: "explicit-collapse", collapsible: true)
) do |list|
list.with_header(title: "Collapsible header") do |header|
header.with_description { "Collapsible description" }
end
list.with_item { "row" }
end
expect(rendered).to have_css(
".op-border-box-list-header.op-border-box-list-header_collapsible"
)
expect(rendered).to have_no_css(".op-border-box-list-header--description")
expect(rendered).to have_text("Collapsible description")
end
end
describe "scheme" do
it "defaults to :default" do
rendered = render_inline(
described_class.new(container: "scheme-default")
) do |list|
list.with_header(title: "Default")
list.with_item { "row" }
end
expect(rendered).to have_no_css(".op-border-box-list_transparent")
end
it "applies the transparent CSS class when scheme is :transparent" do
rendered = render_inline(
described_class.new(container: "scheme-transparent", scheme: :transparent)
) do |list|
list.with_header(title: "Transparent")
list.with_item { "row" }
end
expect(rendered).to have_css(".Box.op-border-box-list_transparent")
end
it "keeps collapsible independent of the transparent scheme" do
rendered = render_inline(
described_class.new(container: "transparent-collapse", scheme: :transparent, collapsible: true)
) do |list|
list.with_header(title: "Transparent collapsible")
list.with_item { "row" }
end
expect(rendered).to have_css(".Box.op-border-box-list_transparent")
expect(rendered).to have_css("collapsible-header")
end
end
describe "header padding" do
it "inherits the underlying BorderBox header padding by default" do
rendered = render_inline(
described_class.new(container: "header-padding-inherit")
) do |list|
list.with_header(title: "Inherited padding")
list.with_item { "row" }
end
expect(rendered).to have_css(".Box.op-border-box-list")
expect(rendered).to have_no_css("[class*='op-border-box-list_header-padding-']")
end
it "adds a header padding modifier when configured" do
rendered = render_inline(
described_class.new(container: "header-padding-default", padding: :condensed, header_padding: :default)
) do |list|
list.with_header(title: "Default header padding")
list.with_item { "row" }
end
expect(rendered).to have_css(".Box.Box--condensed.op-border-box-list_header-padding-default")
end
it "raises for unsupported values in test" do
expect do
described_class.new(container: "header-padding-unsupported", header_padding: :unsupported)
end.to raise_error Primer::FetchOrFallbackHelper::InvalidValueError
end
end
end
@@ -72,21 +72,83 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages
end
end
describe "per-reference query cost",
describe ".with_preloaded_resources save/restore semantics",
with_flag: { semantic_work_package_ids: true },
with_settings: { work_packages_identifier: "semantic" } do
# A custom-field formatter or recursive markdown render may invoke the
# text-formatting pipeline while an outer render is mid-iteration. The
# lookup must save on entry and restore on exit so the outer render's
# remaining `#N` matchers still see its WPs after the inner call returns.
include_context "with author signed in"
let(:project) { create(:project, identifier: "NESTED") }
let(:outer_wp) { create(:work_package, project:, author:) }
let(:inner_wp) { create(:work_package, project:, author:) }
let(:matcher) { OpenProject::TextFormatting::Matchers::ResourceLinksMatcher }
before do
outer_wp.allocate_and_register_semantic_id
inner_wp.allocate_and_register_semantic_id
end
it "preserves the outer lookup across a nested call" do
outer = outer_wp.reload
inner = inner_wp.reload
outer_doc = Nokogiri::HTML.fragment("##{outer.id}")
inner_doc = Nokogiri::HTML.fragment("##{inner.id}")
matcher.with_preloaded_resources(outer_doc, {}) do
expect(matcher.work_package_for(outer.id)).to eq(outer)
matcher.with_preloaded_resources(inner_doc, {}) do
expect(matcher.work_package_for(inner.id)).to eq(inner)
end
expect(matcher.work_package_for(outer.id))
.to eq(outer), "outer lookup should be restored after nested call"
end
expect(matcher.work_package_for(outer.id)).to be_nil
end
end
describe "classic mode is query-free",
with_flag: { semantic_work_package_ids: false },
with_settings: { work_packages_identifier: "classic" } do
# Rendering a `#N` reference in classic mode must not run any
# WorkPackage SELECTs: the preload is a no-op when `display_id` and
# `formatted_id` would collapse to the numeric form, so the link
# handler can build the link from the matched id alone.
include_context "with author signed in"
let(:project) { create(:project, identifier: "classicproj") }
it "does not query work_packages when rendering #N" do
wps = create_list(:work_package, 3, project:, author:)
ids_text = wps.map { |wp| "##{wp.id}" }.join(" ")
recorder = ActiveRecord::QueryRecorder.new { format_text(ids_text) }
wp_selects = recorder.log.grep(/FROM "work_packages"/i)
expect(wp_selects).to be_empty,
"classic mode added unexpected WP SELECTs:\n#{wp_selects.join("\n")}"
end
end
describe "N+1 query bound",
with_flag: { semantic_work_package_ids: true },
with_settings: { work_packages_identifier: "semantic" } do
include_context "with author signed in"
let(:project) { create(:project, identifier: "NPLUSONE") }
it "issues one work_packages SELECT per `#N` reference" do
it "loads referenced work packages with a single SELECT regardless of count" do
wps = create_list(:work_package, 5, project:, author:)
ids_text = wps.map { |wp| "##{wp.id}" }.join(" ")
recorder = ActiveRecord::QueryRecorder.new { format_text(ids_text) }
wp_selects = recorder.log.grep(/FROM "work_packages"/i)
expect(wp_selects.size).to eq(5),
"expected one SELECT per reference, got #{wp_selects.size}:\n#{wp_selects.join("\n")}"
expect(wp_selects.size).to eq(1),
"expected exactly one work_packages SELECT, got #{wp_selects.size}:\n#{wp_selects.join("\n")}"
end
end
@@ -140,7 +202,7 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages
end
context "with mixed numeric and semantic references in one render" do
it "resolves both with one work_packages SELECT per reference" do
it "resolves both with a single work_packages SELECT" do
wps = create_list(:work_package, 2, project:, author:)
wps.each(&:allocate_and_register_semantic_id)
loaded = wps.map(&:reload)
@@ -150,8 +212,8 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages
recorder = ActiveRecord::QueryRecorder.new { rendered = format_text(text) }
wp_selects = recorder.log.grep(/FROM "work_packages"/i)
expect(wp_selects.size).to eq(2),
"expected one SELECT per reference, got #{wp_selects.size}:\n#{wp_selects.join("\n")}"
expect(wp_selects.size).to eq(1),
"expected exactly one work_packages SELECT, got #{wp_selects.size}:\n#{wp_selects.join("\n")}"
# Both render with the user-facing display_id, regardless of which
# form the user typed.
@@ -161,7 +223,7 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages
end
context "with a historical alias reference" do
it "resolves via the alias table without a separate alias round-trip" do
it "resolves via the alias table with two round-trips total" do
wp = work_package.reload
# Simulate a project rename: the WP keeps its current MACROPROJ-N
# identifier on the row, but a historical OLD-prefix alias row
@@ -172,14 +234,13 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages
rendered = nil
recorder = ActiveRecord::QueryRecorder.new { rendered = format_text("see #OLDPROJ-1") }
# `find_by_display_id` runs one WP SELECT whose WHERE clause is
# `identifier = ? OR EXISTS (SELECT 1 FROM aliases …)`, so the
# alias table is consulted in-line — no separate alias-only SELECT.
wp_selects = recorder.log.grep(/FROM "work_packages"/i)
alias_only_selects = recorder.log.grep(/FROM "work_package_semantic_aliases"/i)
.grep_v(/FROM "work_packages"/i)
expect(wp_selects.size).to eq(1)
expect(alias_only_selects).to be_empty
# Two database round-trips: (1) `where_display_id_in` runs a
# single WP SELECT whose WHERE clause includes an EXISTS
# subquery against the alias table; (2) a sidecar alias pluck
# maps the historical input string back to its WP for the
# cache. A third statement would indicate an N+1 regression.
expect(recorder.log.size).to eq(2)
expect(recorder.log.last).to match(/FROM "work_package_semantic_aliases"/)
# Renders against the WP's CURRENT display_id, not the historical
# alias the user typed — old content stays alive but points at the
@@ -234,4 +295,78 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages
expect(rendered).to include("version#PROJ-1")
end
end
describe "visibility scoping",
with_flag: { semantic_work_package_ids: true },
with_settings: { work_packages_identifier: "semantic" } do
# The lookup cache must scope through `WorkPackage.visible` —
# anything it surfaces ends up in the rendered link, so an
# unscoped cache would let any user read back the project
# identifier of a WP just by guessing its primary key, semantic
# identifier, or historical alias.
include_context "with author signed in"
let(:project) { create(:project, identifier: "VISIBLE") }
let(:hidden_project) { create(:project, identifier: "HIDDEN") }
let(:visible_wp) { create(:work_package, project:, author:) }
let(:hidden_wp) { create(:work_package, project: hidden_project) }
before do
visible_wp.allocate_and_register_semantic_id
hidden_wp.allocate_and_register_semantic_id
end
context "with a semantic-shaped ref to an inaccessible work package" do
it "renders literal text and never surfaces the WP's display id" do
wp = hidden_wp.reload
rendered = format_text("see ##{wp.display_id} here")
expect(rendered).to include("##{wp.display_id}")
expect(rendered).not_to include(%(href="/work_packages/#{wp.display_id}"))
expect(rendered).not_to include("opce-macro-wp-quickinfo")
end
end
context "with a numeric ref to an inaccessible work package in semantic mode" do
it "renders the numeric label and href without upgrading to the semantic identifier" do
wp = hidden_wp.reload
rendered = format_text("see ##{wp.id} here")
# The link still renders — `#42` was already in the user's input —
# but the upgrade to the WP's `formatted_id` / `display_id` (which
# would leak the project identifier) does not happen.
expect(rendered).to include(%(href="/work_packages/#{wp.id}"))
expect(rendered).to include(">##{wp.id}<")
expect(rendered).not_to include(%(href="/work_packages/#{wp.display_id}"))
expect(rendered).not_to include(">#{wp.formatted_id}<")
end
end
context "with a historical alias for an inaccessible work package" do
it "renders literal text and does not resolve via the alias table" do
wp = hidden_wp.reload
WorkPackageSemanticAlias.create!(work_package_id: wp.id, identifier: "OLDHIDDEN-1")
rendered = format_text("see #OLDHIDDEN-1 here")
expect(rendered).to include("#OLDHIDDEN-1")
expect(rendered).not_to include(%(href="/work_packages/#{wp.display_id}"))
expect(rendered).not_to include(">#{wp.formatted_id}<")
end
end
context "with visible and invisible refs mixed in one input" do
it "renders the visible ref normally and falls back to literal text for the invisible one" do
visible = visible_wp.reload
hidden = hidden_wp.reload
rendered = format_text("see ##{visible.display_id} and ##{hidden.display_id}")
expect(rendered).to include(%(href="/work_packages/#{visible.display_id}"))
expect(rendered).to include(">#{visible.formatted_id}<")
expect(rendered).not_to include(%(href="/work_packages/#{hidden.display_id}"))
expect(rendered).to include("##{hidden.display_id}")
end
end
end
end