diff --git a/app/assets/images/17_4_features.png b/app/assets/images/17_4_features.png deleted file mode 100644 index 7ec70ec37a9..00000000000 Binary files a/app/assets/images/17_4_features.png and /dev/null differ diff --git a/app/assets/images/17_5_features.png b/app/assets/images/17_5_features.png new file mode 100644 index 00000000000..5465c298751 Binary files /dev/null and b/app/assets/images/17_5_features.png differ diff --git a/app/helpers/mail_formatting_helper.rb b/app/helpers/mail_formatting_helper.rb new file mode 100644 index 00000000000..545da16ca2c --- /dev/null +++ b/app/helpers/mail_formatting_helper.rb @@ -0,0 +1,43 @@ +# 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. +#++ + +# Pin the external rendering channel so mailer templates never have to +# remember the `render_mode:` / `only_path:` / `static_html:` combination. +# Matching the `.html.erb` / `.text.erb` extension to the helper name keeps +# caller intent visible. +module MailFormattingHelper + def format_mail_html(*, **) + format_text(*, render_mode: :external_html, **) + end + + def format_mail_text(*, **) + format_text(*, render_mode: :external_text, **) + end +end diff --git a/app/mailers/application_mailer.rb b/app/mailers/application_mailer.rb index 70ac30581dd..4912ae22742 100644 --- a/app/mailers/application_mailer.rb +++ b/app/mailers/application_mailer.rb @@ -34,6 +34,7 @@ class ApplicationMailer < ActionMailer::Base helper :application, # for format_text :work_packages, # for css classes :custom_fields, # for show_value + :mail_formatting, # for format_mail_html / format_mail_text :mail_layout # for layouting include OpenProject::LocaleHelper diff --git a/app/models/work_package/semantic_identifier.rb b/app/models/work_package/semantic_identifier.rb index b743f058b42..43c9769d59e 100644 --- a/app/models/work_package/semantic_identifier.rb +++ b/app/models/work_package/semantic_identifier.rb @@ -121,6 +121,13 @@ module WorkPackage::SemanticIdentifier end end + # Returns formatted value for inline UI display. + # * Semantic mode: "PROJ-42" (no prefix — self-describing) + # * Classic mode: "#42" (hash-prefixed) + def self.format_display_id(display_id) + display_id.is_a?(String) && display_id.match?(/[A-Za-z]/) ? display_id : "##{display_id}" + end + # Returns the user-facing identifier for this work package. # In semantic mode: the project-based identifier (e.g. "PROJ-42") # In classic mode: the numeric database ID @@ -134,8 +141,7 @@ module WorkPackage::SemanticIdentifier # Semantic mode: "PROJ-42" (no prefix — self-describing) # Classic mode: "#42" (hash-prefixed) def formatted_id - did = display_id - did.is_a?(String) && did.match?(/[A-Za-z]/) ? did : "##{did}" + WorkPackage::SemanticIdentifier.format_display_id(display_id) end # Override ActiveRecord's default `to_param` so Rails URL helpers diff --git a/app/seeders/env_data/ldap_seeder.rb b/app/seeders/env_data/ldap_seeder.rb index cfed89656eb..f020665966f 100644 --- a/app/seeders/env_data/ldap_seeder.rb +++ b/app/seeders/env_data/ldap_seeder.rb @@ -29,9 +29,20 @@ # See COPYRIGHT and LICENSE files for more details. module EnvData class LdapSeeder < Seeder + KNOWN_CONNECTION_KEYS = %w[ + host port security tls_verify tls_certificate sync_users + filter basedn binduser bindpassword + login_mapping firstname_mapping lastname_mapping mail_mapping admin_mapping + groupfilter + ].freeze + + KNOWN_FILTER_KEYS = %w[base filter sync_users group_attribute].freeze + def seed_data! print_status " ↳ Creating LDAP connection" do Setting.seed_ldap.each do |name, options| + validate_options!(name, options) + ldap = LdapAuthSource.find_or_initialize_by(name:) print_ldap_status(ldap) @@ -51,6 +62,34 @@ module EnvData private + def validate_options!(name, options) + check_unknown_keys!(options, KNOWN_CONNECTION_KEYS, + scope: "LDAP connection '#{name}'") + + filters = options["groupfilter"] + return if filters.blank? + + filters.each do |filter_name, filter_options| + check_unknown_keys!(filter_options, KNOWN_FILTER_KEYS, + scope: "LDAP group filter '#{filter_name}' (connection '#{name}')") + end + end + + def check_unknown_keys!(options, known_keys, scope:) + unknown = options.keys - known_keys + return if unknown.empty? + + raise <<~MSG.strip + #{scope}: unknown configuration key(s): #{unknown.map { |k| env_form(k) }.join(', ')}. + Accepted keys: #{known_keys.map { |k| env_form(k) }.join(', ')}. + Note: in environment variable names, single underscores split path segments and double underscores encode a literal underscore (e.g. LOGIN__MAPPING, not LOGIN_MAPPING). + MSG + end + + def env_form(key) + key.gsub("_", "__").upcase + end + # rubocop:disable Metrics/AbcSize def upsert_settings(ldap, options) ldap.host = options["host"] diff --git a/app/services/work_packages/update_ancestors_service.rb b/app/services/work_packages/update_ancestors_service.rb index 7f4ac6c0d67..9466248c8bb 100644 --- a/app/services/work_packages/update_ancestors_service.rb +++ b/app/services/work_packages/update_ancestors_service.rb @@ -123,7 +123,8 @@ class WorkPackages::UpdateAncestorsService < BaseServices::BaseCallable def set_journal_note(work_packages) work_packages.each do |wp| - wp.journal_notes = I18n.t("work_package.updated_automatically_by_child_changes", child: "##{initiator_work_package.id}") + wp.journal_notes = I18n.t("work_package.updated_automatically_by_child_changes", + child: "##{initiator_work_package.id}") end end diff --git a/app/views/work_package_mailer/_work_package_details.html.erb b/app/views/work_package_mailer/_work_package_details.html.erb index 81b1173dd06..626d8f6210e 100644 --- a/app/views/work_package_mailer/_work_package_details.html.erb +++ b/app/views/work_package_mailer/_work_package_details.html.erb @@ -47,4 +47,4 @@ See COPYRIGHT and LICENSE files for more details. <% end %> -<%= format_text(work_package, :description, only_path: false) %> +<%= format_mail_html(work_package, :description) %> diff --git a/app/views/work_package_mailer/mentioned.html.erb b/app/views/work_package_mailer/mentioned.html.erb index 69831d69f34..a7b6d914862 100644 --- a/app/views/work_package_mailer/mentioned.html.erb +++ b/app/views/work_package_mailer/mentioned.html.erb @@ -26,7 +26,7 @@ >
- <%= format_text @journal.notes, only_path: false %> + <%= format_mail_html @journal.notes %>
diff --git a/app/views/work_package_mailer/mentioned.text.erb b/app/views/work_package_mailer/mentioned.text.erb index eee14ad5c8b..f93db4e5806 100644 --- a/app/views/work_package_mailer/mentioned.text.erb +++ b/app/views/work_package_mailer/mentioned.text.erb @@ -8,6 +8,10 @@ <%= "=" * ((@work_package.formatted_id + " " + @work_package.subject).length + 4) %> <%= I18n.t(:label_comment_added) %>: -<%= strip_tags @journal.notes %> +<%= format_mail_text( + @journal.notes, + object: @work_package, + project: @work_package.project + ) %> <%= "-" * 100 %> diff --git a/app/views/work_package_mailer/watcher_changed.html.erb b/app/views/work_package_mailer/watcher_changed.html.erb index 6089c2dd7b9..bd169b9be43 100644 --- a/app/views/work_package_mailer/watcher_changed.html.erb +++ b/app/views/work_package_mailer/watcher_changed.html.erb @@ -30,9 +30,8 @@ See COPYRIGHT and LICENSE files for more details.
<%= render partial: "work_package_details", locals: { work_package: @work_package } %>

- <%= format_text( + <%= format_mail_html( t(:text_latest_note, note: last_work_package_note(@work_package)), - only_path: false, object: @work_package, project: @work_package.project ) %> diff --git a/app/views/work_package_mailer/watcher_changed.text.erb b/app/views/work_package_mailer/watcher_changed.text.erb index 7de3fc8bccc..841f65c0fe3 100644 --- a/app/views/work_package_mailer/watcher_changed.text.erb +++ b/app/views/work_package_mailer/watcher_changed.text.erb @@ -31,4 +31,8 @@ See COPYRIGHT and LICENSE files for more details. ---------------------------------------- <%= render partial: "work_package_details", locals: { work_package: @work_package } %> -<%= t(:text_latest_note, note: last_work_package_note(@work_package)) %> +<%= format_mail_text( + t(:text_latest_note, note: last_work_package_note(@work_package)), + object: @work_package, + project: @work_package.project + ) %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 297f9fcb60d..20f1f55c4df 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3649,16 +3649,18 @@ en: learn_about: "Learn more about all new features" missing: "There are no highlighted features yet." # We need to include the version to invalidate outdated translations in other locales - "17_4": + "17_5": new_features_title: > The release contains various new features and improvements, such as: new_features_list: - line_0: "Jira Migrator with support for basic custom fields." - line_1: Backlog buckets for structuring and prioritizing work packages during backlog refinement. - line_2: Easier drag and drop and improved move options in the Backlogs module. - line_3: Sprint Start and Complete buttons in the sprint header. - line_4: Copy workflow settings between roles. - line_5: "'My Meetings' widget on the Home and Project Overview pages." + line_0: "Project-based work package identifiers for clearer references." + line_1: Jira Migrator support for Jira identifiers, due dates, and more. + line_2: Option to exclude work package types from Backlogs. + line_3: Redesigned sprint views. + line_4: Improved work package linking across Documents and text editors. + line_5: More flexible meeting schedules and reduced email notification noise. + line_6: Nested groups for organizational structures and inherited permissions. + line_7: Improved administration interfaces for workflows, users, and type configuration. links: upgrade_enterprise_edition: "Upgrade to Enterprise edition" postgres_migration: "Migrating your installation to PostgreSQL" diff --git a/docs/installation-and-operations/configuration/README.md b/docs/installation-and-operations/configuration/README.md index 3b2121878a9..e028fb8e4d9 100644 --- a/docs/installation-and-operations/configuration/README.md +++ b/docs/installation-and-operations/configuration/README.md @@ -229,43 +229,65 @@ The connection can be set with the following options. Please note that "EXAMPLE" The name of the LDAP connection is derived from the ENV key behind `SEED_LDAP_`, so you need to take care to use only valid characters. If you need to place an underscore, use a double underscore to encode it e.g., `my__ldap`. -The following options are possible +#### Naming rule for option keys + +The same encoding applies to **option keys** (the part of the env var after the connection name): + +- A single underscore (`_`) separates path segments. +- A double underscore (`__`) encodes a literal underscore inside a single key. + +For example, the attribute mapping for the login attribute must be passed as `LOGIN__MAPPING`. Writing `LOGIN_MAPPING` would create a nested `login.mapping` hash and would have been silently wrong in OpenProject versions earlier than 17.5. + +Starting with OpenProject 17.5 the seeder validates the keys it sees and raises an error listing any unknown key, so typos no longer go unnoticed. Use exactly the keys shown below. + +#### Full example + +The example below shows every supported option for a connection named `EXAMPLE`. ```shell # Host name of the connection -OPENPROJECT_SEED_LDAP_EXAMPLE_HOST="localhost" +OPENPROJECT_SEED_LDAP_EXAMPLE_HOST="ldap.example.com" + # Port of the connection OPENPROJECT_SEED_LDAP_EXAMPLE_PORT="389" -# LDAP security options. One of the following -# plain_ldap: Unencrypted connection, no TLS/SSL -# simple_tls: Using deprecated LDAPS/SSL (often in combination with port 636) -# start_tls: LDAPv3 start_tls call using standard unencrypted port (e.g., 389) before upgrading connection + +# LDAP security mode. One of: +# plain_ldap: Unencrypted connection, no TLS/SSL +# simple_tls: Deprecated LDAPS/SSL (often combined with port 636) +# start_tls: LDAPv3 STARTTLS on the standard unencrypted port (e.g., 389) OPENPROJECT_SEED_LDAP_EXAMPLE_SECURITY="start_tls" -# Whether to verify the certificate/chain of the LDAP connection. true/false (True by default) + +# Whether to verify the LDAP server's certificate chain. true/false (true by default). OPENPROJECT_SEED_LDAP_EXAMPLE_TLS__VERIFY="true" -# Optionally, provide a certificate of the connection + +# Optionally pin a server certificate (PEM-encoded). OPENPROJECT_SEED_LDAP_EXAMPLE_TLS__CERTIFICATE="-----BEGIN CERTIFICATE-----\nMII....\n-----END CERTIFICATE-----" -# The admin LDAP bind account with read access + +# Bind DN (the LDAP account used for read access during search/bind). OPENPROJECT_SEED_LDAP_EXAMPLE_BINDUSER="uid=admin,ou=system" -# Password for the bind account + +# Password for the bind account. OPENPROJECT_SEED_LDAP_EXAMPLE_BINDPASSWORD="secret" -# BASE DN of the connection + +# Base DN of the directory subtree used for user search. OPENPROJECT_SEED_LDAP_EXAMPLE_BASEDN="dc=example,dc=com" -# Optional filter string to restrict which users may log in to OpenProject -# (relevant when for automatic creation of users is active) + +# Optional LDAP filter restricting which users may log in to OpenProject +# (used when automatic user creation is active). OPENPROJECT_SEED_LDAP_EXAMPLE_FILTER="(uid=*)" -# Whether to create found and matching users automatically when they log in + +# Whether to create matching users on the fly when they log in for the first time. OPENPROJECT_SEED_LDAP_EXAMPLE_SYNC__USERS="true" -# Attribute mapping for the OpenProject login attribute + +# Attribute mappings: which LDAP attribute should populate each OpenProject field. +# Remember the double underscore in these keys. OPENPROJECT_SEED_LDAP_EXAMPLE_LOGIN__MAPPING="uid" -# Attribute mapping for the OpenProject first name attribute OPENPROJECT_SEED_LDAP_EXAMPLE_FIRSTNAME__MAPPING="givenName" -# Attribute mapping for the OpenProject last name attribute OPENPROJECT_SEED_LDAP_EXAMPLE_LASTNAME__MAPPING="sn" -# Attribute mapping for the OpenProject mail attribute OPENPROJECT_SEED_LDAP_EXAMPLE_MAIL__MAPPING="mail" -# Attribute mapping for the OpenProject admin attribute -# Leave empty or remove to not derive admin status from an attribute + +# Optional: derive admin status from an LDAP attribute. Leave empty or remove +# to keep admin status managed manually in OpenProject. OPENPROJECT_SEED_LDAP_EXAMPLE_ADMIN__MAPPING="" ``` diff --git a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/cell/edit-cell-handler.ts b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/cell/edit-cell-handler.ts index 378dbbf74eb..64d178fd0e8 100644 --- a/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/cell/edit-cell-handler.ts +++ b/frontend/src/app/features/work-packages/components/wp-fast-table/handlers/cell/edit-cell-handler.ts @@ -1,5 +1,10 @@ import { Injector } from '@angular/core'; -import { displayClassName, editableClassName, readOnlyClassName } from 'core-app/shared/components/fields/display/display-field-renderer'; +import { + displayClassName, + displayTriggerLink, + editableClassName, + readOnlyClassName, +} from 'core-app/shared/components/fields/display/display-field-renderer'; import { HalResourceEditingService } from 'core-app/shared/components/fields/edit/services/hal-resource-editing.service'; import { getPosition } from 'core-app/shared/helpers/set-click-position/set-click-position'; import { InjectField } from 'core-app/shared/helpers/angular/inject-field.decorator'; @@ -38,6 +43,15 @@ export class EditCellHandler extends ClickOrEnterHandler implements TableEventHa protected processEvent(table:WorkPackageTable, evt:MouseEvent|KeyboardEvent):void { debugLog('Starting editing on cell: ', evt.target); + + // Don't intercept clicks on anchor elements - let the browser follow the link + const clickTarget = evt.target as HTMLElement; + const foundElement = clickTarget.closest(`a:not(.${displayTriggerLink}),macro`); + if (foundElement) { + return; + } + + evt.preventDefault(); // Locate the cell from event @@ -61,7 +75,7 @@ export class EditCellHandler extends ClickOrEnterHandler implements TableEventHa // Get any existing edit state for this work package const form = table.editing.startEditing(workPackage, classIdentifier); - let positionOffset = 0; + let positionOffset = 0; if (evt.type === 'click') { // Get the position where the user clicked. positionOffset = getPosition(evt as MouseEvent); diff --git a/lib/open_project/text_formatting.rb b/lib/open_project/text_formatting.rb index c46d9ffcbfe..57b9251ab57 100644 --- a/lib/open_project/text_formatting.rb +++ b/lib/open_project/text_formatting.rb @@ -43,20 +43,25 @@ module OpenProject # @!macro format_text_params # @param [Project] project a Project context. - # @param [Boolean] only_path whether to generate links with relative URLs. + # @param [Symbol] render_mode the rendering channel (`:in_app_html`, + # `:external_html`, `:external_text`). Resolves the `only_path`, + # `static_html` and `plain_text` context flags as a set. Prefer this + # over passing the primitives individually. See {RenderMode}. + # @param [Boolean] only_path explicit override for the resolved `only_path`. # @param [User] current_user the current user context. # @param [:plain, :rich] format the text format. # `:plain` will return plain text. # `:rich` will render raw Markdown as HTML. # @param ** [Hash] additional context to pass to the underlying rendering - # pipeline. + # pipeline. Explicit `static_html:` / `plain_text:` here override the + # values resolved from `render_mode:`. # rubocop:disable Layout/LineLength ## # Formats text according to system settings and provided params. # - # @overload format_text(text, object: nil, project: @project || object.try(:project), only_path: true, current_user: User.current, format: :rich, **) + # @overload format_text(text, object: nil, project: @project || object.try(:project), render_mode: :in_app_html, only_path: nil, current_user: User.current, format: :rich, **) # # @param [String] text the raw text to be formatted, typically Markdown. # @param [Object] object an object context. @@ -64,10 +69,10 @@ module OpenProject # # @example Setting a project context explicitly # format_text("## Hello world", project: current_project) - # @example Generating links with full URLs - # format_text("[Projects](/projects)", only_path: false) + # @example Rendering for an external surface (mailer, RSS, export) + # format_text("see #42", render_mode: :external_html) # - # @overload format_text(object, attribute, project: @project || object.try(:project), only_path: true, current_user: User.current, format: :rich, **) + # @overload format_text(object, attribute, project: @project || object.try(:project), render_mode: :in_app_html, only_path: nil, current_user: User.current, format: :rich, **) # # @param [Object] object an object, typically a model # (i.e. `ActiveRecord::Base` descendent). @@ -79,7 +84,8 @@ module OpenProject # format_text(issue, :description, options) # # @return [String] the formatted text as an HTML-safe String. - def format_text(*args, object: nil, project: nil, only_path: true, current_user: User.current, format: :rich, **) + def format_text(*args, object: nil, project: nil, render_mode: :in_app_html, + only_path: nil, current_user: User.current, format: :rich, **kwargs) case args.size when 1 attribute = nil @@ -94,15 +100,22 @@ module OpenProject project ||= @project || object.try(:project) + resolved = RenderMode.resolve( + render_mode, + only_path:, + static_html: kwargs.delete(:static_html), + plain_text: kwargs.delete(:plain_text) + ) + Renderer.format_text( text, - **, + **kwargs, format:, object:, request: try(:request), current_user:, attribute:, - only_path:, + **resolved, project: ) end diff --git a/lib/open_project/text_formatting/filters/mention_filter.rb b/lib/open_project/text_formatting/filters/mention_filter.rb index 4563289e07b..f1c8defddda 100644 --- a/lib/open_project/text_formatting/filters/mention_filter.rb +++ b/lib/open_project/text_formatting/filters/mention_filter.rb @@ -37,6 +37,8 @@ module OpenProject::TextFormatting include OpenProject::StaticRouting::UrlHelpers def call + preload_mentions + doc.search("mention").each do |mention| anchor = mention_anchor(mention) mention.replace(anchor) if anchor @@ -47,6 +49,41 @@ module OpenProject::TextFormatting private + # WP labels resolve regardless of viewer (so an inaccessible WP + # still renders its current formatted_id); a separate id pluck + # gates anchor-vs-text. Principals collapse the two concerns into + # one visibility-scoped fetch — invisible users and groups fall + # back to the literal envelope text. + def preload_mentions + preload_work_package_mentions + preload_principal_mentions + end + + def preload_work_package_mentions + ids = mention_ids_for("work_package") + if ids.empty? + @mentioned_work_packages = {} + @visible_mentioned_ids = Set.new + return + end + + scope = WorkPackage.where(id: ids) + scope = scope.includes(:type, :status) if context[:static_html] + @mentioned_work_packages = scope.index_by(&:id) + @visible_mentioned_ids = WorkPackage.visible.where(id: ids).pluck(:id).to_set + end + + def preload_principal_mentions + user_ids = mention_ids_for("user") + group_ids = mention_ids_for("group") + @mentioned_users = user_ids.empty? ? {} : User.visible.where(id: user_ids).index_by(&:id) + @mentioned_groups = group_ids.empty? ? {} : Group.visible.where(id: group_ids).index_by(&:id) + end + + def mention_ids_for(type) + doc.css(%(mention[data-type="#{type}"])).filter_map { mention_id(it)&.to_i }.uniq + end + def mention_anchor(mention) mention_instance = class_from_mention(mention) @@ -75,45 +112,61 @@ module OpenProject::TextFormatting end def work_package_mention(work_package, mention) - # Render the mention with the same label and URL convention used for - # `#N` text references elsewhere in the markdown pipeline. - display_id = work_package.display_id + return Nokogiri::XML::Text.new(work_package.formatted_id, mention.document) if text_only?(work_package) case mention.text.count("#") - when 3 - ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo", - "", - data: { id: work_package.id, display_id:, detailed: true } - when 2 - ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo", - "", - data: { id: work_package.id, display_id:, detailed: false } - else - link_to(work_package.formatted_id, - work_package_path_or_url(id: display_id, only_path: context[:only_path]), - class: "issue work_package", - data: { - hover_card_trigger_target: "trigger", - hover_card_url: hover_card_work_package_path(display_id) - }) + when 3 then work_package_quickinfo(work_package, detailed: true) + when 2 then work_package_quickinfo(work_package, detailed: false) + else work_package_link(work_package) end end - def class_from_mention(mention) - mention_class = case mention.attributes["data-type"].value - when "user" - User - when "group" - Group - when "work_package" - WorkPackage - else - raise ArgumentError - end + # The hover-card endpoint a quickinfo would link to is unreachable + # for plain-text recipients and for viewers without view permission. + def text_only?(work_package) + context[:plain_text] || @visible_mentioned_ids.exclude?(work_package.id) + end - mention_class - .visible - .find_by(id: mention_id(mention)) || fallback_text(mention) + def work_package_quickinfo(work_package, detailed:) + return work_package_static_macro(work_package, detailed:) if context[:static_html] + + ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo", + "", + data: { id: work_package.id, + display_id: work_package.display_id, + detailed: } + end + + # Uses the WP's current `formatted_id` rather than the envelope text, + # so a renamed identifier doesn't leave a stale label in the mailer. + def work_package_static_macro(work_package, detailed:) + label = OpenProject::TextFormatting::Helpers::StaticMacroLabel + .call(work_package, label: work_package.formatted_id, detailed:) + + link_to(label, + work_package_path_or_url(id: work_package.display_id, only_path: context[:only_path]), + class: "issue work_package") + end + + def work_package_link(work_package) + display_id = work_package.display_id + link_to(work_package.formatted_id, + work_package_path_or_url(id: display_id, only_path: context[:only_path]), + class: "issue work_package", + data: { + hover_card_trigger_target: "trigger", + hover_card_url: hover_card_work_package_path(display_id) + }) + end + + def class_from_mention(mention) + id = mention_id(mention)&.to_i + case mention.attributes["data-type"].value + when "user" then @mentioned_users[id] + when "group" then @mentioned_groups[id] + when "work_package" then @mentioned_work_packages[id] + else raise ArgumentError + end || fallback_text(mention) end ## diff --git a/lib/open_project/text_formatting/filters/plain_text_output_filter.rb b/lib/open_project/text_formatting/filters/plain_text_output_filter.rb new file mode 100644 index 00000000000..34a2bbdada9 --- /dev/null +++ b/lib/open_project/text_formatting/filters/plain_text_output_filter.rb @@ -0,0 +1,41 @@ +# 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::TextFormatting + module Filters + # Terminal stage when the rich pipeline is rendering for `text/plain` + # bodies — collapses the DOM to its visible text so no HTML escapes. + class PlainTextOutputFilter < HTML::Pipeline::Filter + def call + doc.text + end + end + end +end diff --git a/lib/open_project/text_formatting/formats/markdown/formatter.rb b/lib/open_project/text_formatting/formats/markdown/formatter.rb index 6c6cc3c1906..49696a41303 100644 --- a/lib/open_project/text_formatting/formats/markdown/formatter.rb +++ b/lib/open_project/text_formatting/formats/markdown/formatter.rb @@ -31,11 +31,45 @@ require "task_list/filter" module OpenProject::TextFormatting::Formats::Markdown class Formatter < OpenProject::TextFormatting::Formats::BaseFormatter + RICH_FILTERS = [ + OpenProject::TextFormatting::Filters::SettingMacrosFilter, + OpenProject::TextFormatting::Filters::MarkdownFilter, + OpenProject::TextFormatting::Filters::SanitizationFilter, + OpenProject::TextFormatting::Filters::TaskListFilter, + OpenProject::TextFormatting::Filters::TableOfContentsFilter, + OpenProject::TextFormatting::Filters::MacroFilter, + OpenProject::TextFormatting::Filters::MentionFilter, + OpenProject::TextFormatting::Filters::PatternMatcherFilter, + OpenProject::TextFormatting::Filters::SyntaxHighlightFilter, + OpenProject::TextFormatting::Filters::AttachmentFilter, + OpenProject::TextFormatting::Filters::AutolinkFilter, + OpenProject::TextFormatting::Filters::AutolinkCustomProtocolsFilter, + OpenProject::TextFormatting::Filters::RelativeLinkFilter, + OpenProject::TextFormatting::Filters::LinkAttributeFilter, + OpenProject::TextFormatting::Filters::ExternalLinkCaptureFilter, + OpenProject::TextFormatting::Filters::FigureWrappedFilter, + OpenProject::TextFormatting::Filters::BemCssFilter + ].freeze + + # `text/plain` mailer bodies share the matcher and mention stages so + # work-package references resolve consistently with the HTML channel, + # then `PlainTextOutputFilter` collapses the DOM to text. Filters that + # only shape HTML (TOC, syntax highlight, autolink, link-attribute, + # figure, BEM) are omitted because `doc.text` would discard their work. + TEXT_FILTERS = [ + OpenProject::TextFormatting::Filters::SettingMacrosFilter, + OpenProject::TextFormatting::Filters::MarkdownFilter, + OpenProject::TextFormatting::Filters::SanitizationFilter, + OpenProject::TextFormatting::Filters::MentionFilter, + OpenProject::TextFormatting::Filters::PatternMatcherFilter, + OpenProject::TextFormatting::Filters::PlainTextOutputFilter + ].freeze + def to_html(text) result = pipeline.call(text, context) output = result[:output].to_s - output.html_safe + context[:plain_text] ? output : output.html_safe # rubocop:disable Rails/OutputSafety end def to_document(text) @@ -43,25 +77,7 @@ module OpenProject::TextFormatting::Formats::Markdown end def filters - [ - OpenProject::TextFormatting::Filters::SettingMacrosFilter, - OpenProject::TextFormatting::Filters::MarkdownFilter, - OpenProject::TextFormatting::Filters::SanitizationFilter, - OpenProject::TextFormatting::Filters::TaskListFilter, - OpenProject::TextFormatting::Filters::TableOfContentsFilter, - OpenProject::TextFormatting::Filters::MacroFilter, - OpenProject::TextFormatting::Filters::MentionFilter, - OpenProject::TextFormatting::Filters::PatternMatcherFilter, - OpenProject::TextFormatting::Filters::SyntaxHighlightFilter, - OpenProject::TextFormatting::Filters::AttachmentFilter, - OpenProject::TextFormatting::Filters::AutolinkFilter, - OpenProject::TextFormatting::Filters::AutolinkCustomProtocolsFilter, - OpenProject::TextFormatting::Filters::RelativeLinkFilter, - OpenProject::TextFormatting::Filters::LinkAttributeFilter, - OpenProject::TextFormatting::Filters::ExternalLinkCaptureFilter, - OpenProject::TextFormatting::Filters::FigureWrappedFilter, - OpenProject::TextFormatting::Filters::BemCssFilter - ] + context[:plain_text] ? TEXT_FILTERS : RICH_FILTERS end def self.format diff --git a/lib/open_project/text_formatting/helpers/static_macro_label.rb b/lib/open_project/text_formatting/helpers/static_macro_label.rb new file mode 100644 index 00000000000..2c33e269044 --- /dev/null +++ b/lib/open_project/text_formatting/helpers/static_macro_label.rb @@ -0,0 +1,46 @@ +# 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::TextFormatting + module Helpers + # Anchor text for the static-HTML form of a WP quickinfo macro: + # `[status ]type label: subject`. Used in channels (HTML mailers, + # server-side previews) that can't hydrate the `` widget. + module StaticMacroLabel + def self.call(work_package, label:, detailed:) + parts = [] + parts << work_package.status&.name if detailed + parts << work_package.type&.name + parts << label + "#{parts.compact.join(' ')}: #{work_package.subject}" + end + end + end +end diff --git a/lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb b/lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb index 4a3f85a492e..1f44e340e5f 100644 --- a/lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb +++ b/lib/open_project/text_formatting/matchers/link_handlers/work_packages.rb @@ -78,41 +78,56 @@ module OpenProject::TextFormatting::Matchers # Both quickinfo and plain link need the WP record so the rendered # HTML can carry the record id in `data-id`. Unresolved WP → # literal text rather than a broken reference. - wp = OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.work_package_for(display_id) + wp = preload_cache.fetch(display_id) return nil unless wp if quickinfo? - render_work_package_macro(id: wp.id, display_id: wp.display_id, detailed: detailed?) + render_work_package_macro(work_package: wp, fallback_id: display_id, detailed: detailed?) else render_work_package_link(wp, fallback_id: display_id) end end def render_for_numeric(wp_id) - wp = OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.work_package_for(wp_id) + wp = preload_cache.fetch(wp_id) if quickinfo? - # Prefer the resolved WP's identifiers; fall back to the matched - # id when no preload is available (classic mode). - record_id = wp&.id || wp_id - display_id = wp&.display_id || wp_id - render_work_package_macro(id: record_id, display_id:, detailed: detailed?) + render_work_package_macro(work_package: wp, fallback_id: wp_id, detailed: detailed?) else render_work_package_link(wp, fallback_id: wp_id) end end - def render_work_package_macro(id:, display_id:, detailed: false) + def render_work_package_macro(work_package:, fallback_id:, detailed: false) + id = work_package&.id || fallback_id + display_id = work_package&.display_id || fallback_id + label = WorkPackage::SemanticIdentifier.format_display_id(display_id) + + return label if text_only?(work_package) + return render_static_work_package_macro(work_package, label, detailed:) if context[:static_html] + ApplicationController.helpers.content_tag "opce-macro-wp-quickinfo", "", data: { id:, display_id:, detailed: } end + # The label keeps what the author wrote (possibly a historical + # alias) so the rendered text matches the source markdown. + def render_static_work_package_macro(work_package, label, detailed:) + return label unless work_package + + link_to(OpenProject::TextFormatting::Helpers::StaticMacroLabel.call(work_package, label:, detailed:), + work_package_path_or_url(id: work_package.display_id, only_path: context[:only_path]), + class: "issue work_package") + end + def render_work_package_link(work_package, fallback_id:) # Fall back to the bare `#N` shape when no WP is provided (classic mode, # render path bypassing `PatternMatcherFilter`) rather than running a # per-link query inside the renderer. label = work_package&.formatted_id || "##{fallback_id}" + return label if text_only?(work_package) + href_id = work_package&.display_id || fallback_id link_to(label, @@ -123,6 +138,16 @@ module OpenProject::TextFormatting::Matchers hover_card_url: hover_card_work_package_path(href_id) }) end + + # A nil WP means classic mode skipped the preload, or the reference + # didn't resolve — neither case needs visibility gating. + def text_only?(work_package) + context[:plain_text] || (work_package && !preload_cache.visible?(work_package.id)) + end + + def preload_cache + OpenProject::TextFormatting::Matchers::ResourceLinksMatcher.current_cache + end end end end diff --git a/lib/open_project/text_formatting/matchers/resource_links_matcher.rb b/lib/open_project/text_formatting/matchers/resource_links_matcher.rb index c49bfa3056c..667fb9afe19 100644 --- a/lib/open_project/text_formatting/matchers/resource_links_matcher.rb +++ b/lib/open_project/text_formatting/matchers/resource_links_matcher.rb @@ -69,8 +69,31 @@ 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 + # Unscoped label resolution (`lookup`) paired with viewer-scoped + # link gating (`visible_ids`), so the link handler renders the + # same label for everyone and decides anchor-vs-text per viewer. + class WorkPackagePreloadCache + attr_reader :lookup, :visible_ids + + def initialize(lookup:, visible_ids:) + @lookup = lookup + @visible_ids = visible_ids + end + + def fetch(identifier) + lookup[identifier.to_s] + end + + def visible?(work_package_id) + visible_ids.include?(work_package_id) + end + + # Frozen singleton, not a factory — callers must not mutate it. + EMPTY = new(lookup: {}.freeze, visible_ids: Set.new.freeze).freeze + end + + CACHE_KEY = :text_formatting_work_package_preload_cache + private_constant :CACHE_KEY include ::OpenProject::TextFormatting::Truncation # used for the work package quick links @@ -130,33 +153,33 @@ 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. Lookup keys are always - # strings — see `index_by_id_and_identifier`. - def self.work_package_for(identifier) - RequestStore.store.dig(WORK_PACKAGES_LOOKUP_KEY, identifier.to_s) + def self.current_cache + RequestStore.store[CACHE_KEY] || WorkPackagePreloadCache::EMPTY 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? + # Save/restore so a nested `format_text` (e.g. a custom-field + # formatter re-entering the pipeline) doesn't clobber the outer + # render's cache. + def self.with_preloaded_resources(doc, context) + previous = RequestStore.store[CACHE_KEY] + return yield unless preload_required?(context) identifiers = collect_work_package_identifiers(doc) return yield if identifiers.empty? - RequestStore.store[WORK_PACKAGES_LOOKUP_KEY] = build_lookup(identifiers) + RequestStore.store[CACHE_KEY] = build_cache(identifiers, context) yield ensure - RequestStore.store[WORK_PACKAGES_LOOKUP_KEY] = previous + RequestStore.store[CACHE_KEY] = previous end + # Semantic mode needs the row to map `PROJ-7` to an id; static-HTML + # output needs `type`/`subject` to compose the quickinfo anchor. + def self.preload_required?(context) + Setting::WorkPackageIdentifier.semantic? || context[:static_html] + end + private_class_method :preload_required? + def self.collect_work_package_identifiers(doc) identifiers = Set.new doc.search(".//text()").each do |node| @@ -182,19 +205,25 @@ module OpenProject::TextFormatting 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 + # Two SELECTs in the common case (unscoped fetch + visibility id + # pluck), a third when historical aliases need resolving. Static- + # HTML output additionally needs `:type` and `:status` to compose + # the anchor for `##`/`###` macros. + def self.build_cache(identifiers, context = {}) + scope = WorkPackage.where_display_id_in(*identifiers) + scope = if context[:static_html] + scope.includes(:type, :status) + else + scope.select(:id, :identifier) + end + work_packages = scope.to_a + all_wp_ids = work_packages.map(&:id) + visible_ids = WorkPackage.visible.where(id: all_wp_ids).pluck(:id).to_set lookup = index_by_id_and_identifier(work_packages) - fold_in_alias_keys(lookup, identifiers, visible_wp_ids: work_packages.map(&:id)) - lookup + fold_in_alias_keys(lookup, identifiers, all_wp_ids:) + WorkPackagePreloadCache.new(lookup:, visible_ids:) end + private_class_method :build_cache # Keys are stringified at write time so callers can read with a single # `identifier.to_s` regardless of whether the input is a numeric id or @@ -207,12 +236,12 @@ module OpenProject::TextFormatting end private_class_method :index_by_id_and_identifier - def self.fold_in_alias_keys(lookup, identifiers, visible_wp_ids:) + def self.fold_in_alias_keys(lookup, identifiers, all_wp_ids:) unmapped = identifiers.map(&:to_s) - lookup.keys - return if unmapped.empty? || visible_wp_ids.empty? + return if unmapped.empty? || all_wp_ids.empty? WorkPackageSemanticAlias - .where(work_package_id: visible_wp_ids, identifier: unmapped) + .where(work_package_id: all_wp_ids, identifier: unmapped) .pluck(:identifier, :work_package_id) .each { |ident, wp_id| lookup[ident] = lookup[wp_id.to_s] } end diff --git a/lib/open_project/text_formatting/render_mode.rb b/lib/open_project/text_formatting/render_mode.rb new file mode 100644 index 00000000000..fd78397b739 --- /dev/null +++ b/lib/open_project/text_formatting/render_mode.rb @@ -0,0 +1,64 @@ +# 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 + # Maps a high-level rendering channel (`:in_app_html`, `:external_html`, + # `:external_text`) onto the primitive `only_path` / `static_html` / + # `plain_text` context flags that the filter pipeline reads. + # + # External surfaces always need absolute URLs *and* static rendering for + # JS-dependent components — the two flags are a coupled set. A single + # mode value is the canonical API; the primitives stay available as + # per-flag escape hatches for callers that need an asymmetric mix. + module RenderMode + DEFAULTS = { + in_app_html: { only_path: true, static_html: false, plain_text: false }.freeze, + external_html: { only_path: false, static_html: true, plain_text: false }.freeze, + external_text: { only_path: false, static_html: false, plain_text: true }.freeze + }.freeze + + module_function + + def resolve(mode, only_path: nil, static_html: nil, plain_text: nil) + defaults = DEFAULTS.fetch(mode) do + raise ArgumentError, "Unknown render_mode: #{mode.inspect}. " \ + "Expected one of #{DEFAULTS.keys.inspect}." + end + + { + only_path: only_path.nil? ? defaults[:only_path] : only_path, + static_html: static_html.nil? ? defaults[:static_html] : static_html, + plain_text: plain_text.nil? ? defaults[:plain_text] : plain_text + } + end + end + end +end diff --git a/modules/meeting/app/controllers/meetings_controller.rb b/modules/meeting/app/controllers/meetings_controller.rb index 078a116ac45..fbebcf7051b 100644 --- a/modules/meeting/app/controllers/meetings_controller.rb +++ b/modules/meeting/app/controllers/meetings_controller.rb @@ -54,6 +54,7 @@ class MeetingsController < ApplicationController include OpTurbo::FlashStreamHelper include Meetings::AgendaComponentStreams include MetaTagsHelper + include FlashMessagesOutputSafetyHelper menu_item :new_meeting, only: %i[new create] @@ -311,12 +312,14 @@ class MeetingsController < ApplicationController @meeting.in_progress! end - if @meeting.errors.any? - update_sidebar_state_component_via_turbo_stream - else - update_all_via_turbo_stream - update_backlog_via_turbo_stream(collapsed: nil) - end + update_all_via_turbo_stream + update_backlog_via_turbo_stream(collapsed: nil) + + respond_with_turbo_streams + rescue ActiveRecord::StaleObjectError + @meeting.errors.add(:base, :error_conflict) + update_sidebar_state_component_via_turbo_stream + render_error_flash_message_via_turbo_stream(message: join_flash_messages(@meeting.errors)) respond_with_turbo_streams end diff --git a/modules/meeting/spec/requests/meetings_status_spec.rb b/modules/meeting/spec/requests/meetings_status_spec.rb index 1da353f59ca..2d5a6fd679a 100644 --- a/modules/meeting/spec/requests/meetings_status_spec.rb +++ b/modules/meeting/spec/requests/meetings_status_spec.rb @@ -50,5 +50,15 @@ RSpec.describe "Meeting requests", expect(meeting.reload).to be_in_progress end + + it "handles a stale object gracefully (Bug #68703)" do + allow_any_instance_of(Meeting).to receive(:in_progress!).and_raise(ActiveRecord::StaleObjectError) # rubocop:disable RSpec/AnyInstance + + put change_state_project_meeting_path(project, meeting, state: "in_progress"), + as: :turbo_stream + + expect(response).to have_http_status(:ok) + expect(meeting.reload).to be_open + end end end diff --git a/spec/helpers/mail_formatting_helper_spec.rb b/spec/helpers/mail_formatting_helper_spec.rb new file mode 100644 index 00000000000..73c689b50d5 --- /dev/null +++ b/spec/helpers/mail_formatting_helper_spec.rb @@ -0,0 +1,75 @@ +# 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 "spec_helper" + +RSpec.describe MailFormattingHelper do + shared_let(:project) { create(:project, identifier: "macroproj") } + shared_let(:work_package) { create(:work_package, project:, subject: "test task") } + shared_let(:admin) { create(:admin) } + + before { login_as(admin) } + + describe "#format_mail_html" do + subject(:rendered) { helper.format_mail_html("see ###{work_package.id}") } + + it "renders the quickinfo macro as a static anchor (not the Angular custom element)" do + expect(rendered).to include(%(class="issue work_package)) + expect(rendered).not_to include("]*>\s*#{Regexp.escape(wp.formatted_id)}\s*}) + expect(rendered).not_to include(%(href="/work_packages/#{wp.display_id}")) + expect(rendered).not_to include("opce-macro-wp-quickinfo") + end + + it "renders a quickinfo envelope (`##`) as plain text too" do + wp = hidden_wp.reload + rendered = format_text(mention_tag(wp, sep: "##")) + + expect(rendered).to include(wp.formatted_id) + expect(rendered).not_to include("opce-macro-wp-quickinfo") + end + end + + context "in plain-text rendering mode", + with_settings: { work_packages_identifier: "semantic" } do + # `plain_text: true` must collapse mention envelopes to their current + # `formatted_id` so the `text/plain` mailer doesn't leak `` + # HTML or stale envelope text. + let(:project) { create(:project, identifier: "MACROPROJ") } + let(:work_package) { create(:work_package, project:, author:) } + + before { work_package.allocate_and_register_semantic_id } + + it "renders the formatted_id without an anchor or quickinfo" do + wp = work_package.reload + rendered = format_text(mention_tag(wp), plain_text: true) + + expect(rendered).to include(wp.formatted_id) + expect(rendered).not_to include("@#{user.name}) + end + + it "loads many mentioned users with a single users SELECT keyed by id" do + users = create_list(:user, 5, member_with_roles: { project => role }) + tags = users.map { |u| user_mention_tag(u) }.join + + recorder = ActiveRecord::QueryRecorder.new { format_text(tags) } + # Match SELECTs whose primary FROM is users (the column projection + # starts with `"users"."..."`), so permission subqueries with a + # nested `FROM "users"` don't get counted. + batched = recorder.log.grep(/\ASELECT "users"\.[^,]+,.*FROM "users"/i) + + expect(batched.size).to eq(1), + "expected exactly one batched users SELECT, got #{batched.size}:\n#{batched.join("\n")}" + end + end end end diff --git a/spec/lib/open_project/text_formatting/formats/markdown/static_html_rendering_spec.rb b/spec/lib/open_project/text_formatting/formats/markdown/static_html_rendering_spec.rb new file mode 100644 index 00000000000..f17f3ce4407 --- /dev/null +++ b/spec/lib/open_project/text_formatting/formats/markdown/static_html_rendering_spec.rb @@ -0,0 +1,224 @@ +# 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 "spec_helper" + +RSpec.describe "Markdown static-HTML rendering" do # rubocop:disable RSpec/DescribeClass + subject(:formatted) { render(input) } + + def render(text) + OpenProject::TextFormatting::Renderer.format_text( + text, + format: :rich, + **context.merge(static_html: true) + ) + end + + let(:context) { { only_path: false } } + + shared_let(:project) { create(:project, identifier: "demo") } + shared_let(:type) { create(:type, name: "Task") } + shared_let(:status) { create(:status, name: "New") } + shared_let(:work_package) do + create(:work_package, project:, type:, status:, subject: "Cats V Dogs") + end + shared_let(:admin) { create(:admin) } + + before { login_as(admin) } + + describe "no JS-hydrated custom elements" do + let(:input) { "see #{'##'}#{work_package.id} for details" } + + it "never emits " do + expect(formatted).not_to include("opce-macro-wp-quickinfo") + end + end + + describe "basic mention (#N) — no behaviour change" do + let(:input) { "see ##{work_package.id} please" } + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before { work_package.update_columns(identifier: "DEMO-1", sequence_number: 1) } + + it "renders the formatted_id as an anchor" do + expect(formatted).to include(">DEMO-1<") + expect(formatted).to include('href="') + end + end + end + + describe "quickinfo macro (##N) — type + id + subject in static anchor" do + let(:input) { "see #{'##'}#{work_package.id}" } + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before { work_package.update_columns(identifier: "DEMO-1", sequence_number: 1) } + + it "renders type + formatted_id + subject as a single anchor" do + expect(formatted).to match(%r{]*>Task DEMO-1: Cats V Dogs}) + end + + it "links to the work package show path" do + expect(formatted).to include(%(href="http)) + expect(formatted).to include("/work_packages/DEMO-1") + end + end + + context "in classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "renders type + #N + subject as a single anchor" do + expect(formatted).to match(%r{]*>Task ##{work_package.id}: Cats V Dogs}) + end + end + end + + describe "detailed macro (###N) — status + type + id + subject" do + let(:input) { "see #{'###'}#{work_package.id}" } + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before { work_package.update_columns(identifier: "DEMO-1", sequence_number: 1) } + + it "renders status + type + formatted_id + subject as a single anchor" do + expect(formatted).to match(%r{]*>New Task DEMO-1: Cats V Dogs}) + end + end + + context "in classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "renders status + type + #N + subject as a single anchor" do + expect(formatted).to match(%r{]*>New Task ##{work_package.id}: Cats V Dogs}) + end + end + end + + describe "inaccessible work package" do + shared_let(:other_project) { create(:project, identifier: "secret") } + shared_let(:reader_role) { create(:project_role, permissions: %i[view_work_packages]) } + shared_let(:reader) { create(:user, member_with_roles: { project => reader_role }) } + shared_let(:hidden_wp) do + create(:work_package, project: other_project, type:, status:, subject: "Hidden") + end + + before { login_as(reader) } + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before { hidden_wp.update_columns(identifier: "SECRET-1", sequence_number: 1) } + + it "renders the bare identifier label without an anchor for ##N" do + rendered = render("see #{'##'}#{hidden_wp.id}") + expect(rendered).to include("SECRET-1") + expect(rendered).not_to match(%r{]*>[^<]*SECRET-1}) + expect(rendered).not_to include("Hidden") + end + + it "renders the bare identifier label without an anchor for ###N" do + rendered = render("see #{'###'}#{hidden_wp.id}") + expect(rendered).to include("SECRET-1") + expect(rendered).not_to match(%r{]*>[^<]*SECRET-1}) + end + end + + context "in classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "renders the bare #N label without an anchor for ##N" do + rendered = render("see #{'##'}#{hidden_wp.id}") + expect(rendered).to include("##{hidden_wp.id}") + expect(rendered).not_to match(%r{]*>[^<]*##{hidden_wp.id}}) + expect(rendered).not_to include("Hidden") + end + + it "renders the bare #N label without an anchor for ###N" do + rendered = render("see #{'###'}#{hidden_wp.id}") + expect(rendered).to include("##{hidden_wp.id}") + expect(rendered).not_to match(%r{]*>[^<]*##{hidden_wp.id}}) + end + end + end + + describe "work-package mention envelope" do + let(:mention_attrs) do + %(class="mention" data-id="#{work_package.id}" data-type="work_package" data-display-id="DEMO-1") + end + let(:input) { %(check ##DEMO-1) } + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before { work_package.update_columns(identifier: "DEMO-1", sequence_number: 1) } + + it "renders ## quickinfo envelopes as a static anchor with type + id + subject" do + expect(formatted).to match(%r{]*>Task DEMO-1: Cats V Dogs}) + expect(formatted).not_to include("opce-macro-wp-quickinfo") + end + end + end + + # Public document exports and other non-authenticated rendering paths + # invoke the static-HTML pipeline with `User.current == User.anonymous`. + # In that context every non-public WP is invisible, so any mention must + # collapse to its current `formatted_id` as plain text — no anchor, no + # subject leak. + describe "anonymous current_user" do + shared_let(:private_project) { create(:project, identifier: "private", public: false) } + shared_let(:private_wp) do + create(:work_package, project: private_project, type:, status:, subject: "Top Secret") + end + + around do |example| + User.execute_as(User.anonymous) { example.run } + end + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before { private_wp.update_columns(identifier: "PRIVATE-1", sequence_number: 1) } + + it "does not raise and renders the identifier text" do + expect { render("see #{'##'}#{private_wp.id}") } + .not_to raise_error + + rendered = render("see #{'##'}#{private_wp.id}") + expect(rendered).to include("PRIVATE-1") + end + end + + context "in classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "does not raise and renders the #N text" do + expect { render("see #{'##'}#{private_wp.id}") } + .not_to raise_error + + rendered = render("see #{'##'}#{private_wp.id}") + expect(rendered).to include("##{private_wp.id}") + end + end + end +end diff --git a/spec/lib/open_project/text_formatting/formats/markdown/text_rendering_spec.rb b/spec/lib/open_project/text_formatting/formats/markdown/text_rendering_spec.rb new file mode 100644 index 00000000000..66be86e5ac3 --- /dev/null +++ b/spec/lib/open_project/text_formatting/formats/markdown/text_rendering_spec.rb @@ -0,0 +1,153 @@ +# 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 "spec_helper" + +RSpec.describe "Markdown plain-text rendering" do # rubocop:disable RSpec/DescribeClass + subject(:formatted) { render(input).strip } + + def render(text) + OpenProject::TextFormatting::Renderer.format_text(text, plain_text: true) + end + + describe "plain markdown" do + let(:input) { "Hello *world*" } + + it "renders text without HTML tags" do + expect(formatted).to eq("Hello world") + end + end + + describe "with an inline work-package reference" do + shared_let(:project) { create(:project, identifier: "demo") } + shared_let(:work_package) { create(:work_package, project:, subject: "task") } + shared_let(:admin) { create(:admin) } + + let(:input) { "see ##{work_package.id} please" } + + before { login_as(admin) } + + context "in classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "renders the hash-prefixed numeric id" do + expect(formatted).to eq("see ##{work_package.id} please") + end + end + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before do + work_package.update_columns(identifier: "DEMO-1", sequence_number: 1) + end + + it "renders the bare semantic identifier" do + expect(formatted).to eq("see DEMO-1 please") + end + end + end + + describe "with a quickinfo macro reference" do + shared_let(:project) { create(:project, identifier: "demo") } + shared_let(:work_package) { create(:work_package, project:, subject: "task") } + shared_let(:admin) { create(:admin) } + + before { login_as(admin) } + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before do + work_package.update_columns(identifier: "DEMO-1", sequence_number: 1) + end + + it "renders ##N as bare semantic identifier" do + expect(render("see #{'##'}#{work_package.id} please").strip).to eq("see DEMO-1 please") + end + + it "renders ###N as bare semantic identifier" do + expect(render("see #{'###'}#{work_package.id} please").strip).to eq("see DEMO-1 please") + end + end + + context "in classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "renders ##N as the hash-prefixed numeric id" do + expect(render("see #{'##'}#{work_package.id} please").strip).to eq("see ##{work_package.id} please") + end + + it "renders ###N as the hash-prefixed numeric id" do + expect(render("see #{'###'}#{work_package.id} please").strip).to eq("see ##{work_package.id} please") + end + end + end + + describe "with a work-package mention envelope" do + shared_let(:project) { create(:project, identifier: "demo") } + shared_let(:work_package) { create(:work_package, project:, subject: "task") } + shared_let(:admin) { create(:admin) } + + let(:mention_attrs) do + %(class="mention" data-id="#{work_package.id}" data-type="work_package" data-display-id="DEMO-1") + end + let(:input) { %(check #DEMO-1) } + + before { login_as(admin) } + + context "in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before do + work_package.update_columns(identifier: "DEMO-1", sequence_number: 1) + end + + it "unwraps to the bare semantic identifier" do + expect(formatted).to eq("check DEMO-1") + end + end + + context "in classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "unwraps to the hash-prefixed numeric id" do + expect(formatted).to eq("check ##{work_package.id}") + end + end + + context "with a ##-shaped mention text in semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + let(:input) { %(check ##DEMO-1) } + + before do + work_package.update_columns(identifier: "DEMO-1", sequence_number: 1) + end + + it "unwraps to the bare semantic identifier" do + expect(formatted).to eq("check DEMO-1") + end + end + end +end diff --git a/spec/lib/open_project/text_formatting/matchers/link_handlers/work_packages_spec.rb b/spec/lib/open_project/text_formatting/matchers/link_handlers/work_packages_spec.rb index ed0b41e2333..a819e110a82 100644 --- a/spec/lib/open_project/text_formatting/matchers/link_handlers/work_packages_spec.rb +++ b/spec/lib/open_project/text_formatting/matchers/link_handlers/work_packages_spec.rb @@ -94,17 +94,17 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages inner_doc = Nokogiri::HTML.fragment("##{inner.id}") matcher.with_preloaded_resources(outer_doc, {}) do - expect(matcher.work_package_for(outer.id)).to eq(outer) + expect(matcher.current_cache.fetch(outer.id)).to eq(outer) matcher.with_preloaded_resources(inner_doc, {}) do - expect(matcher.work_package_for(inner.id)).to eq(inner) + expect(matcher.current_cache.fetch(inner.id)).to eq(inner) end - expect(matcher.work_package_for(outer.id)) + expect(matcher.current_cache.fetch(outer.id)) .to eq(outer), "outer lookup should be restored after nested call" end - expect(matcher.work_package_for(outer.id)).to be_nil + expect(matcher.current_cache.fetch(outer.id)).to be_nil end end @@ -134,15 +134,17 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages include_context "with author signed in" let(:project) { create(:project, identifier: "NPLUSONE") } - it "loads referenced work packages with a single SELECT regardless of count" do + it "loads referenced work packages with a fixed two-SELECT preload 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(1), - "expected exactly one work_packages SELECT, got #{wp_selects.size}:\n#{wp_selects.join("\n")}" + # One unscoped fetch by identifier (label resolution) plus one + # visibility-scoped pluck on the resulting ids (link gating). + expect(wp_selects.size).to eq(2), + "expected exactly two work_packages SELECTs, got #{wp_selects.size}:\n#{wp_selects.join("\n")}" end end @@ -199,7 +201,7 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages end context "with mixed numeric and semantic references in one render" do - it "resolves both with a single work_packages SELECT" do + it "resolves both with the fixed two-SELECT preload" do wps = create_list(:work_package, 2, project:, author:) wps.each(&:allocate_and_register_semantic_id) loaded = wps.map(&:reload) @@ -209,8 +211,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(1), - "expected exactly one work_packages SELECT, got #{wp_selects.size}:\n#{wp_selects.join("\n")}" + expect(wp_selects.size).to eq(2), + "expected exactly two work_packages SELECTs, got #{wp_selects.size}:\n#{wp_selects.join("\n")}" # Both render with the user-facing display_id, regardless of which # form the user typed. @@ -220,7 +222,7 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages end context "with a historical alias reference" do - it "resolves via the alias table with two round-trips total" do + it "resolves via the alias table with bounded round-trips" 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 @@ -231,17 +233,17 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages rendered = nil recorder = ActiveRecord::QueryRecorder.new { rendered = format_text("see #OLDPROJ-1") } - # Two targeted 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. Scoped greps ignore incidental Setting/permission - # queries — a second match on either grep would indicate an + # Bounded round-trips: (1) `where_display_id_in` runs an unscoped + # WP SELECT whose WHERE includes an EXISTS subquery against the + # alias table, (2) a visibility-scoped id pluck for link gating, + # (3) a sidecar alias pluck maps the historical input string back + # to its WP for the cache. Scoped greps ignore incidental + # Setting/permission queries — additional matches indicate an # N+1 regression. wp_selects = recorder.log.grep(/FROM "work_packages"/) alias_selects = recorder.log.grep(/FROM "work_package_semantic_aliases"/) .grep_v(/FROM "work_packages"/) - expect(wp_selects.size).to eq(1) + expect(wp_selects.size).to eq(2) expect(alias_selects.size).to eq(1) # Renders against the WP's CURRENT display_id, not the historical @@ -298,11 +300,11 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages describe "visibility scoping", 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. + # Label resolution is unscoped so notification recipients see the same + # identifier shape as authors, but anchors are still gated by + # `WorkPackage.visible` — the link handler emits a plain-text label + # for inaccessible WPs rather than a navigable URL or hover-card + # endpoint. include_context "with author signed in" let(:project) { create(:project, identifier: "VISIBLE") } @@ -316,46 +318,44 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages 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 + it "renders the formatted_id as plain text with no anchor or quickinfo" do wp = hidden_wp.reload rendered = format_text("see ##{wp.display_id} here") - expect(rendered).to include("##{wp.display_id}") + expect(rendered).to include(wp.formatted_id) + expect(rendered).not_to match(%r{]*>\s*#{Regexp.escape(wp.formatted_id)}\s*}) 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 + it "upgrades the label to the formatted_id but does not render an anchor" 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).to include(wp.formatted_id) + expect(rendered).not_to match(%r{]*>\s*#{Regexp.escape(wp.formatted_id)}\s*}) + expect(rendered).not_to include(%(href="/work_packages/#{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 + it "resolves the alias and renders the current formatted_id as plain text" 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).to include(wp.formatted_id) + expect(rendered).not_to match(%r{]*>\s*#{Regexp.escape(wp.formatted_id)}\s*}) 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 + it "renders the visible ref as an anchor and the invisible ref as plain-text label" do visible = visible_wp.reload hidden = hidden_wp.reload rendered = format_text("see ##{visible.display_id} and ##{hidden.display_id}") @@ -364,7 +364,8 @@ RSpec.describe OpenProject::TextFormatting::Matchers::LinkHandlers::WorkPackages 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}") + expect(rendered).to include(hidden.formatted_id) + expect(rendered).not_to match(%r{]*>\s*#{Regexp.escape(hidden.formatted_id)}\s*}) end end end diff --git a/spec/lib/open_project/text_formatting/render_mode_spec.rb b/spec/lib/open_project/text_formatting/render_mode_spec.rb new file mode 100644 index 00000000000..843e3fa989f --- /dev/null +++ b/spec/lib/open_project/text_formatting/render_mode_spec.rb @@ -0,0 +1,98 @@ +# 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 "spec_helper" + +RSpec.describe OpenProject::TextFormatting::RenderMode do + describe ".resolve" do + context "with :in_app_html" do + it "produces the in-app default trio" do + expect(described_class.resolve(:in_app_html)).to eq( + only_path: true, + static_html: false, + plain_text: false + ) + end + end + + context "with :external_html" do + it "produces absolute URLs and static-HTML rendering" do + expect(described_class.resolve(:external_html)).to eq( + only_path: false, + static_html: true, + plain_text: false + ) + end + end + + context "with :external_text" do + it "produces absolute URLs and plain-text output" do + expect(described_class.resolve(:external_text)).to eq( + only_path: false, + static_html: false, + plain_text: true + ) + end + end + + context "with an explicit primitive flag" do + it "lets only_path override while keeping the rest of the mode's defaults" do + expect(described_class.resolve(:external_html, only_path: true)).to eq( + only_path: true, + static_html: true, + plain_text: false + ) + end + + it "lets an explicit false override the mode's true default" do + expect(described_class.resolve(:external_html, static_html: false)).to eq( + only_path: false, + static_html: false, + plain_text: false + ) + end + + it "ignores a nil override (treats it as 'not passed')" do + expect(described_class.resolve(:external_html, plain_text: nil)).to eq( + only_path: false, + static_html: true, + plain_text: false + ) + end + end + + context "with an unknown mode" do + it "raises ArgumentError naming the bad value" do + expect { described_class.resolve(:nonsense) } + .to raise_error(ArgumentError, /render_mode.*nonsense/) + end + end + end +end diff --git a/spec/mailers/work_package_mailer_spec.rb b/spec/mailers/work_package_mailer_spec.rb index a2028ce1e74..93fab35f2f2 100644 --- a/spec/mailers/work_package_mailer_spec.rb +++ b/spec/mailers/work_package_mailer_spec.rb @@ -129,6 +129,42 @@ RSpec.describe WorkPackageMailer do expect(mail["X-OpenProject-WorkPackage-Assignee"].value) .to eql work_package.assigned_to.login end + + describe "rendering a journal note containing a WP reference" do + shared_let(:persisted_project) { create(:project, identifier: "demo") } + shared_let(:persisted_recipient) { create(:admin) } + shared_let(:referenced_wp) { create(:work_package, project: persisted_project, subject: "child") } + shared_let(:parent_wp) { create(:work_package, project: persisted_project, subject: "parent") } + + let(:persisted_journal) do + create(:work_package_journal, + journable: parent_wp, + user: persisted_recipient, + version: parent_wp.journals.maximum(:version).to_i + 1, + notes: "see ##{referenced_wp.id}") + end + let(:mail) { described_class.mentioned(persisted_recipient, persisted_journal) } + + context "with classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "renders the hash-prefixed numeric id in the text body" do + expect(mail.text_part.body.to_s).to include("##{referenced_wp.id}") + end + end + + context "with semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before do + referenced_wp.update_columns(identifier: "DEMO-1", sequence_number: 1) + end + + it "renders the bare semantic identifier in the text body" do + body = mail.text_part.body.to_s + expect(body).to include("DEMO-1") + expect(body).not_to match(/##{referenced_wp.id}\b/) + end + end + end end describe "#watcher_changed" do @@ -209,5 +245,135 @@ RSpec.describe WorkPackageMailer do .to eql "op.work_package-#{work_package.id}@example.net" end end + + describe "rendering the latest comment containing a WP reference" do + shared_let(:persisted_project) { create(:project, identifier: "demo") } + shared_let(:persisted_recipient) { create(:admin) } + shared_let(:referenced_wp) { create(:work_package, project: persisted_project, subject: "child") } + shared_let(:parent_wp) { create(:work_package, project: persisted_project, subject: "parent") } + + let(:mail) do + create(:work_package_journal, + journable: parent_wp, + user: persisted_recipient, + version: parent_wp.journals.maximum(:version).to_i + 1, + notes: "Updated automatically by changing values within child work package ##{referenced_wp.id}") + described_class.watcher_changed(parent_wp, persisted_recipient, persisted_recipient, "added") + end + + context "with classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "renders the hash-prefixed numeric id in the text body" do + expect(mail.text_part.body.to_s).to include("##{referenced_wp.id}") + end + end + + context "with semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before do + referenced_wp.update_columns(identifier: "DEMO-1", sequence_number: 1) + end + + it "renders the bare semantic identifier in the text body" do + body = mail.text_part.body.to_s + expect(body).to include("DEMO-1") + expect(body).not_to match(/##{referenced_wp.id}\b/) + end + + it "renders the bare semantic identifier in the html body" do + body = mail.html_part.body.to_s + expect(body).to include("DEMO-1") + end + end + end + + describe "rendering a cross-project WP reference to a recipient without visibility", + with_settings: { work_packages_identifier: "semantic" } do + shared_let(:parent_project) { create(:project, identifier: "parent-proj") } + shared_let(:child_project) { create(:project, identifier: "child-proj") } + shared_let(:parent_wp) { create(:work_package, project: parent_project, subject: "parent") } + shared_let(:child_wp) { create(:work_package, project: child_project, subject: "child") } + shared_let(:reader_role) { create(:project_role, permissions: %i[view_work_packages]) } + shared_let(:reader) { create(:user, member_with_roles: { parent_project => reader_role }) } + + let(:mail) do + child_wp.update_columns(identifier: "CHILDPROJ-1", sequence_number: 1) + create(:work_package_journal, + journable: parent_wp, + user: reader, + version: parent_wp.journals.maximum(:version).to_i + 1, + notes: "Updated automatically by changing values within child work package ##{child_wp.id}") + described_class.watcher_changed(parent_wp, reader, reader, "added") + end + + it "renders the semantic identifier as plain text in the text body" do + body = mail.text_part.body.to_s + expect(body).to include("CHILDPROJ-1") + expect(body).not_to match(/##{child_wp.id}\b/) + end + + it "renders the semantic identifier without an anchor in the html body" do + body = mail.html_part.body.to_s + expect(body).to include("CHILDPROJ-1") + expect(body).not_to include(%(href="/work_packages/#{child_wp.id}")) + expect(body).not_to include(%(href="/work_packages/CHILDPROJ-1")) + end + end + + describe "rendering a quickinfo/detailed macro in the latest comment" do + shared_let(:persisted_project) { create(:project, identifier: "demo") } + shared_let(:persisted_recipient) { create(:admin) } + shared_let(:macro_type) { create(:type, name: "Task") } + shared_let(:macro_status) { create(:status, name: "New") } + shared_let(:referenced_wp) do + create(:work_package, + project: persisted_project, + type: macro_type, + status: macro_status, + subject: "Cats V Dogs") + end + shared_let(:parent_wp) { create(:work_package, project: persisted_project, subject: "parent") } + + let(:mail) do + create(:work_package_journal, + journable: parent_wp, + user: persisted_recipient, + version: parent_wp.journals.maximum(:version).to_i + 1, + notes: "ref ##{referenced_wp.id} ##{'#'}#{referenced_wp.id} ###{'#'}#{referenced_wp.id}") + described_class.watcher_changed(parent_wp, persisted_recipient, persisted_recipient, "added") + end + + context "with semantic mode", + with_settings: { work_packages_identifier: "semantic" } do + before { referenced_wp.update_columns(identifier: "DEMO-1", sequence_number: 1) } + + it "renders ## quickinfo as a static anchor with type + id + subject" do + body = mail.html_part.body.to_s + expect(body).to match(%r{]*>Task DEMO-1: Cats V Dogs}) + end + + it "renders ### detailed as a static anchor with status + type + id + subject" do + body = mail.html_part.body.to_s + expect(body).to match(%r{]*>New Task DEMO-1: Cats V Dogs}) + end + + it "never leaks into the html body" do + expect(mail.html_part.body.to_s).not_to include("opce-macro-wp-quickinfo") + end + end + + context "with classic mode", + with_settings: { work_packages_identifier: "classic" } do + it "renders ## quickinfo as a static anchor with type + #N + subject" do + body = mail.html_part.body.to_s + expect(body).to match(%r{]*>Task ##{referenced_wp.id}: Cats V Dogs}) + end + + it "renders ### detailed as a static anchor with status + type + #N + subject" do + body = mail.html_part.body.to_s + expect(body).to match(%r{]*>New Task ##{referenced_wp.id}: Cats V Dogs}) + end + end + end end end diff --git a/spec/models/work_package/semantic_identifier_spec.rb b/spec/models/work_package/semantic_identifier_spec.rb index 229cba5fa67..c07349d42c1 100644 --- a/spec/models/work_package/semantic_identifier_spec.rb +++ b/spec/models/work_package/semantic_identifier_spec.rb @@ -606,6 +606,20 @@ RSpec.describe WorkPackage::SemanticIdentifier do end end + describe ".format_display_id" do + it "returns the semantic identifier unchanged when it carries letters" do + expect(described_class.format_display_id("MYPROJ-1")).to eq("MYPROJ-1") + end + + it "hash-prefixes a numeric integer" do + expect(described_class.format_display_id(42)).to eq("#42") + end + + it "hash-prefixes a numeric string" do + expect(described_class.format_display_id("42")).to eq("#42") + end + end + describe "#to_param" do include Rails.application.routes.url_helpers diff --git a/spec/seeders/env_data/ldap_seeder_spec.rb b/spec/seeders/env_data/ldap_seeder_spec.rb index 2632bde287e..ff61d024f26 100644 --- a/spec/seeders/env_data/ldap_seeder_spec.rb +++ b/spec/seeders/env_data/ldap_seeder_spec.rb @@ -248,4 +248,51 @@ RSpec.describe EnvData::LdapSeeder do expect(names).to contain_exactly("another") end end + + context "when an unknown key is provided", + :settings_reset, + with_env: { + OPENPROJECT_SEED_LDAP_FOO_HOST: "localhost", + OPENPROJECT_SEED_LDAP_FOO_PORT: "12389", + OPENPROJECT_SEED_LDAP_FOO_SECURITY: "plain_ldap", + OPENPROJECT_SEED_LDAP_FOO_BINDUSER: "uid=admin,ou=system", + OPENPROJECT_SEED_LDAP_FOO_BINDPASSWORD: "secret", + OPENPROJECT_SEED_LDAP_FOO_BASEDN: "dc=example,dc=com", + OPENPROJECT_SEED_LDAP_FOO_LOGIN__MAPPING: "uid", + OPENPROJECT_SEED_LDAP_FOO_FIRSTNAME__MAPPING: "givenName", + OPENPROJECT_SEED_LDAP_FOO_LASTNAME__MAPPING: "sn", + OPENPROJECT_SEED_LDAP_FOO_MAIL__MAPPING: "mail", + OPENPROJECT_SEED_LDAP_FOO_NONSENSE: "boom" + } do + it "raises an error naming the unknown key without creating the record" do + reset(:seed_ldap) + + expect { seeder.seed! } + .to raise_error(/LDAP connection 'foo'.*NONSENSE/m) + .and not_change(LdapAuthSource, :count) + end + end + + context "when a typo'd nested key would be silently swallowed", + :settings_reset, + with_env: { + OPENPROJECT_SEED_LDAP_FOO_HOST: "localhost", + OPENPROJECT_SEED_LDAP_FOO_PORT: "12389", + OPENPROJECT_SEED_LDAP_FOO_SECURITY: "plain_ldap", + OPENPROJECT_SEED_LDAP_FOO_BINDUSER: "uid=admin,ou=system", + OPENPROJECT_SEED_LDAP_FOO_BINDPASSWORD: "secret", + OPENPROJECT_SEED_LDAP_FOO_BASEDN: "dc=example,dc=com", + OPENPROJECT_SEED_LDAP_FOO_LOGIN_MAPPING: "uid", + OPENPROJECT_SEED_LDAP_FOO_FIRSTNAME__MAPPING: "givenName", + OPENPROJECT_SEED_LDAP_FOO_LASTNAME__MAPPING: "sn", + OPENPROJECT_SEED_LDAP_FOO_MAIL__MAPPING: "mail" + } do + it "raises rather than silently producing a nil login attribute" do + reset(:seed_ldap) + + expect { seeder.seed! } + .to raise_error(/LDAP connection 'foo'.*LOGIN/m) + .and not_change(LdapAuthSource, :count) + end + end end diff --git a/spec/services/work_packages/update_ancestors_service_spec.rb b/spec/services/work_packages/update_ancestors_service_spec.rb index 73606ebb077..3a9ad8fc105 100644 --- a/spec/services/work_packages/update_ancestors_service_spec.rb +++ b/spec/services/work_packages/update_ancestors_service_spec.rb @@ -1385,4 +1385,45 @@ RSpec.describe WorkPackages::UpdateAncestorsService, end end end + + describe "auto-generated journal note when a child triggers an ancestor recompute", + with_settings: { work_package_done_ratio: "status" } do + shared_let_work_packages(<<~TABLE) + hierarchy | status | work | ∑ work | remaining work | ∑ remaining work | % complete | ∑ % complete + parent | Open | 10h | 15h | 10h | 15h | 0% | 0% + child | Open | 5h | | 5h | | 0% | + TABLE + + # The journal note always stores the primary-key reference (`#42`). + # Render-time resolution in the formatter pipeline turns it into + # `#PROJ-7` in semantic mode and `#42` in classic mode, so the stored + # text survives project-identifier renames. + context "in classic mode", + with_settings: { work_package_done_ratio: "status", work_packages_identifier: "classic" } do + it "writes the child's hash-prefixed primary key into the parent's journal note" do + set_attributes_on(child, status: closed_status) + call_update_ancestors_service(child) + + note = parent.reload.journals.last.notes + expect(note).to include("##{child.id}") + end + end + + context "in semantic mode", + with_settings: { work_package_done_ratio: "status", work_packages_identifier: "semantic" } do + before do + child.allocate_and_register_semantic_id + end + + it "writes the child's hash-prefixed primary key, not its semantic identifier" do + set_attributes_on(child, status: closed_status) + call_update_ancestors_service(child) + + wp = child.reload + note = parent.reload.journals.last.notes + expect(note).to include("##{wp.id}") + expect(note).not_to include(wp.identifier) + end + end + end end diff --git a/spec/support/components/work_packages/table_configuration_modal.rb b/spec/support/components/work_packages/table_configuration_modal.rb index 9813c0b74b7..ee2076b937f 100644 --- a/spec/support/components/work_packages/table_configuration_modal.rb +++ b/spec/support/components/work_packages/table_configuration_modal.rb @@ -80,8 +80,10 @@ module Components end def save - find('[data-test-selector="spot-modal-wp-table-configuration-save-button"]').click - expect_closed + retry_block do + find('[data-test-selector="spot-modal-wp-table-configuration-save-button"]').click + expect_closed + end if using_cuprite? wait_for_reload