From 820c4abbcc0e6707d0663f8322b1afd5209f013e Mon Sep 17 00:00:00 2001 From: Eric Schubert Date: Wed, 11 Sep 2024 13:42:26 +0200 Subject: [PATCH] [#53620] fixed failing tests --- .../application_row_component.html.erb | 7 ++- .../applications/index_component.html.erb | 10 ++-- .../concerns/requires_admin_guard.rb | 2 +- app/contracts/delete_contract.rb | 2 +- .../oauth_clients/create_contract.rb | 2 +- .../features/bcf/api_authorization_spec.rb | 2 +- .../concerns/manage_storages_guarded.rb | 2 +- .../oauth_applications_management_spec.rb | 48 ++++++++++--------- 8 files changed, 42 insertions(+), 33 deletions(-) diff --git a/app/components/oauth/applications/application_row_component.html.erb b/app/components/oauth/applications/application_row_component.html.erb index 5813d4cd4aab..0a7e9787336a 100644 --- a/app/components/oauth/applications/application_row_component.html.erb +++ b/app/components/oauth/applications/application_row_component.html.erb @@ -1,5 +1,5 @@ <%= - component_wrapper do + component_wrapper(data: { 'test-selector': "op-admin-oauth--application" }) do flex_layout(align_items: :center, justify_content: :space_between) do |oauth_application_container| oauth_application_container.with_column(flex_layout: true) do |application_information| application_information.with_column(mr: 2) do @@ -32,7 +32,10 @@ render(Primer::Alpha::ToggleSwitch.new( src: toggle_oauth_application_path(@application), csrf_token: form_authenticity_token, - checked: @application.enabled? + checked: @application.enabled?, + data: { + 'test-selector': "op-admin-oauth--application-enabled-toggle-switch" + } )) end end diff --git a/app/components/oauth/applications/index_component.html.erb b/app/components/oauth/applications/index_component.html.erb index 020862cae064..856ff7b0b136 100644 --- a/app/components/oauth/applications/index_component.html.erb +++ b/app/components/oauth/applications/index_component.html.erb @@ -2,7 +2,9 @@ component_wrapper do flex_layout do |index_container| index_container.with_row do - render(border_box_container(mb: 4)) do |component| + render(border_box_container(mb: 4, data: { + 'test-selector': "op-admin-oauth--built-in-applications" + })) do |component| component.with_header(font_weight: :bold) do render(Primer::Beta::Text.new) do t("oauth.header.builtin_applications") @@ -11,7 +13,9 @@ if @built_in_applications.empty? component.with_row do - render(Primer::Beta::Text.new) do + render(Primer::Beta::Text.new(data: { + 'test-selector': "op-admin-oauth--built-in-applications-placeholder" + })) do t("oauth.empty_application_lists") end end @@ -33,7 +37,7 @@ if @other_applications.empty? component.with_row do - render(Primer::Beta::Text.new) do + render(Primer::Beta::Text.new(data: { 'test-selector': "op-admin-oauth--applications-placeholder" })) do t("oauth.empty_application_lists") end end diff --git a/app/contracts/concerns/requires_admin_guard.rb b/app/contracts/concerns/requires_admin_guard.rb index c4950a783075..133ac24d0bb7 100644 --- a/app/contracts/concerns/requires_admin_guard.rb +++ b/app/contracts/concerns/requires_admin_guard.rb @@ -35,7 +35,7 @@ module RequiresAdminGuard # Adds an error if user is archived or not an admin. def validate_admin_only - unless user.active_admin? + unless user.admin? && user.active? errors.add :base, :error_unauthorized end end diff --git a/app/contracts/delete_contract.rb b/app/contracts/delete_contract.rb index 6deff8380103..d8f06edc14e9 100644 --- a/app/contracts/delete_contract.rb +++ b/app/contracts/delete_contract.rb @@ -56,7 +56,7 @@ def authorized? case permission when :admin - user.active_admin? + user.admin? && user.active? when Proc instance_exec(&permission) when Symbol diff --git a/app/contracts/oauth_clients/create_contract.rb b/app/contracts/oauth_clients/create_contract.rb index f8f439372b50..404436c4a1a9 100644 --- a/app/contracts/oauth_clients/create_contract.rb +++ b/app/contracts/oauth_clients/create_contract.rb @@ -47,7 +47,7 @@ class CreateContract < ::ModelContract private def validate_user_allowed - unless user.active_admin? + unless user.admin? && user.active? errors.add :base, :error_unauthorized end end diff --git a/modules/bim/spec/features/bcf/api_authorization_spec.rb b/modules/bim/spec/features/bcf/api_authorization_spec.rb index c02100f61589..5ae2fc055fea 100644 --- a/modules/bim/spec/features/bcf/api_authorization_spec.rb +++ b/modules/bim/spec/features/bcf/api_authorization_spec.rb @@ -46,7 +46,7 @@ def oauth_path(client_id) it "can create and later authorize and manage an OAuth application grant and then use the access token for the bcf api" do # Initially empty - expect(page).to have_css(".generic-table--empty-row", text: "There is currently nothing to display") + expect(page).to have_test_selector("op-admin-oauth--applications-placeholder") # Create application page.find_test_selector("op-admin-oauth--button-new", text: "OAuth application").click diff --git a/modules/storages/app/contracts/storages/storages/concerns/manage_storages_guarded.rb b/modules/storages/app/contracts/storages/storages/concerns/manage_storages_guarded.rb index 010ba0ab2497..34e85ad8d39b 100644 --- a/modules/storages/app/contracts/storages/storages/concerns/manage_storages_guarded.rb +++ b/modules/storages/app/contracts/storages/storages/concerns/manage_storages_guarded.rb @@ -52,7 +52,7 @@ module ManageStoragesGuarded # Small procedure to check that the current user is admin and active def validate_user_allowed_to_manage - unless user.active_admin? + unless user.admin? && user.active? errors.add :base, :error_unauthorized end end diff --git a/spec/features/admin/oauth/oauth_applications_management_spec.rb b/spec/features/admin/oauth/oauth_applications_management_spec.rb index 149c750943cc..faaab28a4884 100644 --- a/spec/features/admin/oauth/oauth_applications_management_spec.rb +++ b/spec/features/admin/oauth/oauth_applications_management_spec.rb @@ -39,7 +39,7 @@ visit oauth_applications_path # Initially empty - expect(page).to have_css(".generic-table--empty-row", text: "There is currently nothing to display") + expect(page).to have_test_selector("op-admin-oauth--applications-placeholder") # Create application page.find_test_selector("op-admin-oauth--button-new", text: "OAuth application").click @@ -52,7 +52,7 @@ expect(page).to have_css(".errorExplanation", text: "Redirect URI must be an absolute URI.") fill_in("application_redirect_uri", with: "") - # Fill rediret_uri which does not provide a Secure Context + # Fill redirect_uri which does not provide a Secure Context fill_in "application_redirect_uri", with: "http://example.org" click_on "Create" @@ -78,7 +78,7 @@ click_on "Save" # Show application - find("td a", text: "My API application").click + click_on "My API application" expect(page).to have_no_css(".attributes-key-value--key", text: "Client secret") expect(page).to have_no_css(".attributes-key-value--value code") @@ -90,7 +90,7 @@ end # Table is empty again - expect(page).to have_css(".generic-table--empty-row", text: "There is currently nothing to display") + expect(page).to have_test_selector("op-admin-oauth--applications-placeholder") end context "with a seeded application" do @@ -101,32 +101,34 @@ it "does not allow editing or deleting the seeded application" do visit oauth_applications_path - expect(page).to have_css("td.name", text: "OpenProject Mobile App") - expect(page).to have_css("td.builtin .icon-checkmark") - expect(page).to have_css("td.enabled .icon-checkmark") + app = Doorkeeper::Application.last - expect(page).to have_no_css("td.buttons", text: "Edit") - expect(page).to have_no_css("td.buttons", text: "Delete") + within_test_selector("op-admin-oauth--built-in-applications") do + expect(page).to have_test_selector("op-admin-oauth--application", count: 1) + expect(page).to have_link(text: "OpenProject Mobile App") + expect(page).to have_test_selector("op-admin-oauth--application-enabled-toggle-switch", text: "On") - expect(page).to have_css("td.buttons", text: "Deactivate") - click_link_or_button "Deactivate" + find_test_selector("op-admin-oauth--application-enabled-toggle-switch").click + expect(page).not_to have_test_selector("op-admin-oauth--application-enabled-toggle-switch", text: "Loading") + expect(page).to have_test_selector("op-admin-oauth--application-enabled-toggle-switch", text: "Off") - expect(page).to have_css("td.builtin .icon-checkmark") - expect(page).to have_no_css("td.enabled .icon-checkmark") + app.reload + expect(app).to be_builtin + expect(app).not_to be_enabled - app = Doorkeeper::Application.last - expect(app).to be_builtin - expect(app).not_to be_enabled + find_test_selector("op-admin-oauth--application-enabled-toggle-switch").click + expect(page).not_to have_test_selector("op-admin-oauth--application-enabled-toggle-switch", text: "Loading") + expect(page).to have_test_selector("op-admin-oauth--application-enabled-toggle-switch", text: "On") - expect(page).to have_css("td.buttons", text: "Activate") - click_link_or_button "Activate" + app.reload + expect(app).to be_builtin + expect(app).to be_enabled - expect(page).to have_css("td.builtin .icon-checkmark") - expect(page).to have_css("td.enabled .icon-checkmark") + click_on "OpenProject Mobile App" + end - app.reload - expect(app).to be_builtin - expect(app).to be_enabled + expect(page).to have_no_button("Edit") + expect(page).to have_no_button("Delete") visit edit_oauth_application_path(app) expect(page).to have_text "You are not authorized to access this page."