From d9ef662173d136cb2d77205db908251b48bda725 Mon Sep 17 00:00:00 2001 From: Christophe Bliard Date: Tue, 3 Jun 2025 18:33:13 +0200 Subject: [PATCH] Fix display of error messages in dialog when uploading tokens And rewrite EnterpriseTokensController specs to test it properly. --- .../enterprise_tokens/form_component.html.erb | 7 +- .../enterprise_tokens_controller.rb | 2 +- app/views/enterprise_tokens/index.html.erb | 4 +- .../enterprise_tokens_controller_spec.rb | 234 ++++++++++++++++++ .../enterprises_controller_spec.rb | 204 --------------- .../enterprises/_current.html.erb_spec.rb | 9 +- 6 files changed, 247 insertions(+), 213 deletions(-) create mode 100644 spec/controllers/enterprise_tokens_controller_spec.rb delete mode 100644 spec/controllers/enterprises_controller_spec.rb diff --git a/app/components/admin/enterprise_tokens/form_component.html.erb b/app/components/admin/enterprise_tokens/form_component.html.erb index 3e0fe5330a0..1cf70814013 100644 --- a/app/components/admin/enterprise_tokens/form_component.html.erb +++ b/app/components/admin/enterprise_tokens/form_component.html.erb @@ -10,9 +10,12 @@ } ) do |f| flex_layout(mb: 3) do |modal_body| - if @token.errors[:base].present? + errors_outside_fields = @token.errors.reject { |error| error.attribute == :encoded_token } + if errors_outside_fields.any? modal_body.with_row do - render(Primer::Alpha::Banner.new(mb: 3, icon: :stop, scheme: :danger)) { @token.errors[:base].join("\n") } + render(Primer::Alpha::Banner.new(mb: 3, icon: :stop, scheme: :danger)) do + errors_outside_fields.map(&:full_message).join("\n") + end end end diff --git a/app/controllers/enterprise_tokens_controller.rb b/app/controllers/enterprise_tokens_controller.rb index 588d30c641b..24eaf307de9 100644 --- a/app/controllers/enterprise_tokens_controller.rb +++ b/app/controllers/enterprise_tokens_controller.rb @@ -102,7 +102,7 @@ class EnterpriseTokensController < ApplicationController private def find_token - EnterpriseToken.find(params[:id]) + @token = EnterpriseToken.find(params[:id]) end def check_user_limit diff --git a/app/views/enterprise_tokens/index.html.erb b/app/views/enterprise_tokens/index.html.erb index 07140ac4f3e..5478a43969d 100644 --- a/app/views/enterprise_tokens/index.html.erb +++ b/app/views/enterprise_tokens/index.html.erb @@ -30,9 +30,9 @@ <%= error_messages_for "token" if @token %> -<% if @current_token.present? %> +<% if EnterpriseToken.all_tokens.any? %> <%= render Admin::EnterpriseTokens::TableComponent.new(rows: EnterpriseToken.all_tokens) %> - <%= render partial: "current" %> + <%= render partial: "current" if @current_token.present? %> <% else %> <%= render partial: "info" %> <% end %> diff --git a/spec/controllers/enterprise_tokens_controller_spec.rb b/spec/controllers/enterprise_tokens_controller_spec.rb new file mode 100644 index 00000000000..91a1daee4c1 --- /dev/null +++ b/spec/controllers/enterprise_tokens_controller_spec.rb @@ -0,0 +1,234 @@ +# 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 EnterpriseTokensController do + include EnterpriseTokenFactory + + let(:token_attributes) do + { + subscriber: "Foobar subscriber", + mail: "foo@example.org", + starts_at: Date.current, + expires_at: nil + } + end + + before do + login_as user + end + + context "with an admin user" do + let(:user) { build(:admin) } + + describe "#index" do + render_views + + context "when an Enterprise token is active" do + let!(:enterprise_token) { create_enterprise_token(**token_attributes) } + + before do + get :index + end + + shared_examples "it renders the enterprise tokens list" do + it "renders the enterprise tokens list" do + expect(response).to be_successful + expect(response).to render_template "index" + expect(response.body).to have_text(enterprise_token.subscriber) + end + end + + include_examples "it renders the enterprise tokens list" + + context "with version >= 2.0" do + let(:token_attributes) { super().merge version: "2.0" } + + context "with correct domain", with_settings: { host_name: "community.openproject.com" } do + let(:token_attributes) { super().merge domain: "community.openproject.com" } + + include_examples "it renders the enterprise tokens list" + + it "displays the domain name" do + expect(response.body).to have_text(token_attributes[:domain]) + end + end + + context "with wrong domain", with_settings: { host_name: "community.openproject.com" } do + let(:token_attributes) { super().merge domain: "non-matching-domain.openproject.com" } + + include_examples "it renders the enterprise tokens list" + + it "displays the domain with an invalid message", skip: "TODO: there is no warning displayed yet" do + expect(controller).to set_flash.now[:error].to(/.*localhost.*does not match.*community.openproject.com/) + end + end + end + + context "with version < 2.0" do + let(:token_attributes) { super().merge version: "1.0.3" } + + context "with wrong domain", with_settings: { host_name: "community.openproject.com" } do + let(:token_attributes) { super().merge domain: "localhost" } + + include_examples "it renders the enterprise tokens list" + + it "doesn't show any warnings or errors", skip: "TODO: there is no warning displayed yet" do + expect(controller).not_to set_flash.now + end + end + end + end + + context "when no token exists" do + before do + get :index + end + + it "renders a perks page", skip: "TODO" do + expect(response.body).to have_css ".upsell-benefits" + end + end + end + + describe "#create" do + let(:encoded_token) { "foo" } + let(:params) do + { + enterprise_token: { encoded_token: } + } + end + let(:format) { nil } + + before do + mock_token_object(encoded_token, **token_attributes) + + post :create, params:, format: + end + + context "with valid token input" do + let(:token_attributes) { super().merge(domain: Setting.host_name) } + + # TODO: is it still relevant to test html rendering? + it "saves the token and redirects to index" do + expect(EnterpriseToken.count).to eq 1 + expect(controller).to set_flash[:notice].to I18n.t(:notice_successful_update) + expect(response).to redirect_to action: :index + end + end + + context "with unreadable token input" do + let(:token_attributes) { super().merge(domain: "non-matching-domain.openproject.com") } + let(:params) do + { + enterprise_token: { encoded_token: "I am an unreadable token 🙈" } + } + end + + # TODO: is it still relevant to test html rendering? + it "renders with error" do + expect(response).to have_http_status(:unprocessable_entity) + expect(response).to render_template "enterprise_tokens/index" + end + + context "with turbo stream" do + let(:format) { :turbo_stream } + + it "renders the form with an error message for the unreadable token" do + expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_turbo_stream action: "update", target: "admin-enterprise-tokens-form-component" + expected_error_message = I18n.t("activerecord.errors.models.enterprise_token.unreadable") + expect(response.body).to include(ERB::Util.html_escape(expected_error_message)) + end + end + end + + context "with token with invalid domain" do + let(:token_attributes) { super().merge(domain: "non-matching-domain.openproject.com") } + + # TODO: is it still relevant to test html rendering? + it "renders with error" do + expect(response).to have_http_status(:unprocessable_entity) + expect(response).to render_template "enterprise_tokens/index" + end + + context "with turbo stream" do + let(:format) { :turbo_stream } + + it "renders the form with an error message for the invalid domain" do + expect(response).to have_http_status(:unprocessable_entity) + expect(response).to have_turbo_stream action: "update", target: "admin-enterprise-tokens-form-component" + expect(response.body).to include("Domain is invalid") + end + end + end + end + + describe "#destroy" do + context "when a token exists" do + before do + enterprise_token = create_enterprise_token(**token_attributes) + + delete :destroy, params: { id: enterprise_token.id } + end + + it "redirects to index" do + expect(controller).to set_flash[:notice].to I18n.t(:notice_successful_delete) + expect(response).to redirect_to action: :index + end + end + + context "when no token exists" do + before do + delete :destroy, params: { id: 42 } + end + + it "renders 404" do + expect(response).to have_http_status(:not_found) + end + end + end + end + + context "with a regular user" do + let(:user) { build(:user) } + + describe "#index" do + before do + get :index + end + + it "is forbidden" do + expect(response).to have_http_status :forbidden + end + end + end +end diff --git a/spec/controllers/enterprises_controller_spec.rb b/spec/controllers/enterprises_controller_spec.rb deleted file mode 100644 index 043e0de56a1..00000000000 --- a/spec/controllers/enterprises_controller_spec.rb +++ /dev/null @@ -1,204 +0,0 @@ -# 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 EnterprisesController do - let(:a_token) { EnterpriseToken.new } - - let(:token_attributes) do - { - subscriber: "Foobar", - mail: "foo@example.org", - starts_at: Date.today, - version: "1", - expires_at: nil - } - end - - let(:token_object) do - OpenProject::Token.new token_attributes - end - - before do - login_as user - allow(a_token).to receive(:token_object).and_return(token_object) - end - - context "with admin" do - let(:user) { build(:admin) } - - describe "#show" do - render_views - - context "when token exists" do - before do - allow(EnterpriseToken).to receive(:current).and_return(a_token) - get :show - end - - shared_examples "it renders the EE overview" do - it "renders the overview" do - expect(response).to be_successful - expect(response).to render_template "show" - expect(response).to render_template partial: "enterprises/_current" - expect(response).to render_template partial: "enterprises/_form" - end - end - - it_behaves_like "it renders the EE overview" - - context "with version >= 2.0" do - let(:token_attributes) { super().merge version: "2.0" } - - context "with correct domain", with_settings: { host_name: "community.openproject.com" } do - let(:token_attributes) { super().merge domain: "community.openproject.com" } - - it_behaves_like "it renders the EE overview" - - it "doesn't show any warnings or errors" do - expect(controller).not_to set_flash.now - end - end - - context "with wrong domain", with_settings: { host_name: "community.openproject.com" } do - let(:token_attributes) { super().merge domain: "localhost" } - - it_behaves_like "it renders the EE overview" - - it "shows an invalid domain error" do - expect(controller).to set_flash.now[:error].to(/.*localhost.*does not match.*community.openproject.com/) - end - end - end - - context "with version < 2.0" do - let(:token_attributes) { super().merge version: "1.0.3" } - - context "with wrong domain", with_settings: { host_name: "community.openproject.com" } do - let(:token_attributes) { super().merge domain: "localhost" } - - it_behaves_like "it renders the EE overview" - - it "doesn't show any warnings or errors" do - expect(controller).not_to set_flash.now - end - end - end - end - - context "when no token exists" do - before do - allow(EnterpriseToken).to receive(:current).and_return(nil) - get :show - end - - it "still renders #show with form" do - expect(response).not_to render_template partial: "enterprises/_current" - expect(response.body).to have_css ".upsell-benefits" - end - end - end - - describe "#create" do - let(:params) do - { - enterprise_token: { encoded_token: "foo" } - } - end - - before do - allow(EnterpriseToken).to receive(:current).and_return(nil) - allow(EnterpriseToken).to receive(:new).and_return(a_token) - expect(a_token).to receive(:encoded_token=).with("foo") - expect(a_token).to receive(:save).and_return(valid) - - post :create, params: - end - - context "valid token input" do - let(:valid) { true } - - it "redirects to index" do - expect(controller).to set_flash[:notice].to I18n.t(:notice_successful_update) - expect(response).to redirect_to action: :show - end - end - - context "invalid token input" do - let(:valid) { false } - - it "renders with error" do - expect(response).not_to be_redirect - expect(response).to render_template "enterprises/show" - end - end - end - - describe "#destroy" do - context "when a token exists" do - before do - expect(EnterpriseToken).to receive(:current).and_return(a_token) - expect(a_token).to receive(:destroy) - - delete :destroy - end - - it "redirects to show" do - expect(controller).to set_flash[:notice].to I18n.t(:notice_successful_delete) - expect(response).to redirect_to action: :show - end - end - - context "when no token exists" do - before do - expect(EnterpriseToken).to receive(:current).and_return(nil) - delete :destroy - end - - it "renders 404" do - expect(response).to have_http_status(:not_found) - end - end - end - end - - context "regular user" do - let(:user) { build(:user) } - - before do - get :show - end - - it "is forbidden" do - expect(response).to have_http_status :forbidden - end - end -end diff --git a/spec/views/admin/enterprises/_current.html.erb_spec.rb b/spec/views/admin/enterprises/_current.html.erb_spec.rb index 66679342bfb..dc0565220e2 100644 --- a/spec/views/admin/enterprises/_current.html.erb_spec.rb +++ b/spec/views/admin/enterprises/_current.html.erb_spec.rb @@ -30,7 +30,8 @@ require "spec_helper" -RSpec.describe "admin/enterprises/_current" do +# TODO: delete this file once the partial "current" is removed +RSpec.describe "enterprise_tokens/current" do let(:current_user) { create(:admin) } let(:ee_token) { "v1_expired_with_7_days_reprieve_at_2021_09_01.token" } let(:current_time) { DateTime.now } @@ -38,13 +39,13 @@ RSpec.describe "admin/enterprises/_current" do before do allow(User).to receive(:current).and_return current_user - encoded = File.read Rails.root.join("spec/fixtures/ee_tokens/#{ee_token}") - token = EnterpriseToken.new(encoded_token: encoded) + encoded = Rails.root.join("spec/fixtures/ee_tokens/#{ee_token}").read + token = EnterpriseToken.new(id: 42, encoded_token: encoded) assign :current_token, token Timecop.travel(current_date) do - render partial: "enterprises/current" + render partial: "enterprise_tokens/current" end end