Skip to content

Commit

Permalink
Merge pull request #15653 from opf/implementation/55133-handle-edge-c…
Browse files Browse the repository at this point in the history
…ase-if-unlink-fails-render-a-primer-error-banner

Implementation/55133 handle edge case if unlink fails render a primer error banner
  • Loading branch information
akabiru authored May 29, 2024
2 parents 9e2beba + 95b6b0f commit 1472e55
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def not_required
# enabling a custom field which is required happens in an after_save hook within the custom field model itself
return if model.project_custom_field.nil? || !model.project_custom_field.required?

errors.add :custom_field_id, :invalid
errors.add :custom_field_id, :cannot_delete_mapping
end

def visbile_to_user
Expand Down
33 changes: 29 additions & 4 deletions app/controllers/admin/settings/project_custom_fields_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ module Admin::Settings
class ProjectCustomFieldsController < ::Admin::SettingsController
include CustomFields::SharedActions
include OpTurbo::ComponentStream
include ApplicationComponentStreams
include FlashMessagesOutputSafetyHelper
include Admin::Settings::ProjectCustomFields::ComponentStreams

menu_item :project_custom_fields_settings
Expand All @@ -41,6 +43,7 @@ class ProjectCustomFieldsController < ::Admin::SettingsController
before_action :prepare_custom_option_position, only: %i(update create)
before_action :find_custom_option, only: :delete_option
before_action :project_custom_field_mappings_query, only: %i[project_mappings unlink]
before_action :find_unlink_project_custom_field_mapping, only: :unlink
# rubocop:enable Rails/LexicallyScopedActionFilter

def show_local_breadcrumb
Expand Down Expand Up @@ -68,13 +71,18 @@ def edit; end
def project_mappings; end

def unlink
project = Project.find(permitted_params.project_custom_field_project_mapping[:project_id])
project_custom_field_mapping = @custom_field.project_custom_field_project_mappings.find_by(project:)
delete_service = ProjectCustomFieldProjectMappings::DeleteService
.new(user: current_user, model: project_custom_field_mapping)
.new(user: current_user, model: @project_custom_field_mapping)
.call

delete_service.on_success { render_unlink_response(project:) }
delete_service.on_success { render_unlink_response(project: @project) }

delete_service.on_failure do
update_flash_message_via_turbo_stream(
message: join_flash_messages(delete_service.errors.full_messages),
full: true, dismiss_scheme: :hide, scheme: :danger
)
end

respond_to_with_turbo_streams(status: delete_service.success? ? :ok : :unprocessable_entity)
end
Expand Down Expand Up @@ -150,6 +158,23 @@ def set_sections
.all
end

def find_unlink_project_custom_field_mapping
@project = Project.find(permitted_params.project_custom_field_project_mapping[:project_id])
@project_custom_field_mapping = @custom_field.project_custom_field_project_mappings.find_by!(project: @project)
rescue ActiveRecord::RecordNotFound
update_flash_message_via_turbo_stream(
message: t(:notice_file_not_found), full: true, dismiss_scheme: :hide, scheme: :danger
)
replace_via_turbo_stream(
component: Settings::ProjectCustomFields::ProjectCustomFieldMapping::TableComponent.new(
query: project_custom_field_mappings_query,
params: { custom_field: @custom_field }
)
)

respond_with_turbo_streams
end

def find_custom_field
@custom_field = ProjectCustomField.find(params[:id])
rescue ActiveRecord::RecordNotFound
Expand Down
16 changes: 6 additions & 10 deletions app/helpers/flash_messages_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ module FlashMessagesHelper
extend ActiveSupport::Concern

included do
# For .safe_join in join_flash_messages
include ActionView::Helpers::OutputSafetyHelper
include FlashMessagesOutputSafetyHelper
end

def render_primer_banner_message?
Expand All @@ -45,6 +44,11 @@ def render_primer_banner_message
render(BannerMessageComponent.new(**flash[:primer_banner].to_hash))
end

# Primer's flash message component wrapped in a component which is empty initially but can be updated via turbo stream
def render_streameable_primer_banner_message
render(FlashMessageComponent.new)
end

# Renders flash messages
def render_flash_messages
return if render_primer_banner_message?
Expand All @@ -63,14 +67,6 @@ def render_flash_messages
safe_join messages, "\n"
end

def join_flash_messages(messages)
if messages.respond_to?(:join)
safe_join(messages, "<br />".html_safe)
else
messages
end
end

def render_flash_message(type, message, html_options = {}) # rubocop:disable Metrics/AbcSize
if type.to_s == "notice"
type = "success"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,21 @@
#
# See COPYRIGHT and LICENSE files for more details.
#++
#

module FlashMessagesOutputSafetyHelper
extend ActiveSupport::Concern

included do
# For .safe_join in join_flash_messages
include ActionView::Helpers::OutputSafetyHelper
end

module ProjectCustomFieldProjectMappings
class DeleteContract < ::DeleteContract
delete_permission :select_project_custom_fields
def join_flash_messages(messages)
if messages.respond_to?(:join)
safe_join(messages, "<br />".html_safe)
else
messages
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,10 @@

module ProjectCustomFieldProjectMappings
class DeleteService < ::BaseServices::Delete
# Mappings have custom deletion rules that are similar to the update rules all derived from the base contract
# Reuse the update contract to ensure that the deletion rules are consistent with the update rules
def default_contract_class
ProjectCustomFieldProjectMappings::UpdateContract
end
end
end
1 change: 1 addition & 0 deletions app/views/layouts/base.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ See COPYRIGHT and LICENSE files for more details.
<div class="content-overlay"></div>
<main id="content-wrapper" class="<%= initial_classes %>">
<%= render_primer_banner_message %>
<%= render_streameable_primer_banner_message %>
<% if show_decoration %>
<div id="breadcrumb" class="<%= initial_classes %><%= show_breadcrumb ? ' -show' : '' %>">
<%= you_are_here_info %>
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,7 @@ Project attributes and sections are defined in the <a href=%{admin_settings_url}
before_or_equal_to: "must be before or equal to %{date}."
blank: "can't be blank."
blank_nested: "needs to have the property '%{property}' set."
cannot_delete_mapping: "is required. Cannot be deleted."
cant_link_a_work_package_with_a_descendant: "A work package cannot be linked to one of its subtasks."
circular_dependency: "This relation would create a circular dependency."
confirmation: "doesn't match %{attribute}."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
class Storages::Admin::StoragesController < ApplicationController
using Storages::Peripherals::ServiceResultRefinements

include FlashMessagesHelper
include FlashMessagesOutputSafetyHelper
include OpTurbo::ComponentStream

# See https://guides.rubyonrails.org/layouts_and_rendering.html for reference on layout
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# frozen_string_literal: true

#-- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2012-2024 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"
require "contracts/shared/model_contract_shared_context"

RSpec.describe ProjectCustomFieldProjectMappings::BaseContract do
include_context "ModelContract shared context"

let(:contract) { described_class.new(project_custom_field_mapping, user) }
let(:user) { build_stubbed(:admin) }
let(:project) { build_stubbed(:project) }
let(:project_custom_field) { build_stubbed(:project_custom_field) }
let(:project_custom_field_mapping) { build_stubbed(:project_custom_field_project_mapping, project:, project_custom_field:) }

before { User.current = user }

context "when the custom field is required" do
let(:project_custom_field) { build_stubbed(:project_custom_field, is_required: true) }

it_behaves_like "contract is invalid"
end

context "with non-visible custom field and admin user" do
let(:project_custom_field) { build_stubbed(:project_custom_field, visible: false) }

before do
allow(ProjectCustomField).to receive(:all).and_return([project_custom_field])
end

it_behaves_like "contract is valid"
end

context "with non-visible custom field and non-admin user" do
let(:user) { build_stubbed(:user) }
let(:project_custom_field) { build_stubbed(:project_custom_field, visible: false) }

before do
allow(ProjectCustomField).to receive(:where).with(visible: true).and_return([project_custom_field])
end

it_behaves_like "contract is invalid"
end

include_examples "contract reuses the model errors"
end
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,8 @@
RSpec.describe ProjectCustomFieldProjectMappings::DeleteService do
it_behaves_like "BaseServices delete service" do
let(:factory) { :project_custom_field_project_mapping }
let(:contract_class) do
"#{namespace}::UpdateContract".constantize
end
end
end

0 comments on commit 1472e55

Please sign in to comment.