Fix display of error messages in dialog when uploading tokens

And rewrite EnterpriseTokensController specs to test it properly.

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
 

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

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 %>

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

spec/controllers/enterprises_controller_spec.rb

@@ -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

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