From 465d2d604db63f2fd46cc4476aa4660ebb88f23b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 3 Feb 2023 16:58:03 +0100 Subject: [PATCH 1/3] Move login items tests to the component This way we reduce the number of system tests or, in some cases, requests during system tests, making the tests faster. We're still testing the interaction with the menu when users have the right permissions. --- .../admin_login_items_component_spec.rb | 132 ++++++++++++++++++ spec/system/admin_spec.rb | 13 -- spec/system/moderation_spec.rb | 37 ----- spec/system/officing_spec.rb | 49 ------- spec/system/sdg_management/sdg_spec.rb | 27 ---- spec/system/sdg_management_spec.rb | 19 --- .../valuation/budget_investments_spec.rb | 7 - spec/system/valuation_spec.rb | 37 ----- 8 files changed, 132 insertions(+), 189 deletions(-) create mode 100644 spec/components/layout/admin_login_items_component_spec.rb delete mode 100644 spec/system/sdg_management/sdg_spec.rb diff --git a/spec/components/layout/admin_login_items_component_spec.rb b/spec/components/layout/admin_login_items_component_spec.rb new file mode 100644 index 000000000..def3ff068 --- /dev/null +++ b/spec/components/layout/admin_login_items_component_spec.rb @@ -0,0 +1,132 @@ +require "rails_helper" + +describe Layout::AdminLoginItemsComponent do + it "is not rendered for anonymous users" do + render_inline Layout::AdminLoginItemsComponent.new(nil) + + expect(page).not_to be_rendered + end + + it "is not rendered for regular users" do + user = create(:user, :level_two) + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).not_to be_rendered + end + + it "shows access to all places except officing to administrators" do + user = create(:administrator).user + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).to have_link "Menu" + + page.find("ul") do |menu| + expect(menu).to have_link "Administration" + expect(menu).to have_link "Moderation" + expect(menu).to have_link "Valuation" + expect(menu).to have_link "Management" + expect(menu).to have_link "SDG content" + expect(menu).to have_link count: 5 + end + end + + it "shows the moderation link to moderators" do + user = create(:moderator).user + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).to have_link "Menu" + + page.find("ul") do |menu| + expect(menu).to have_link "Moderation" + expect(menu).to have_link count: 1 + end + end + + it "shows the valuation link to valuators" do + user = create(:valuator).user + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).to have_link "Menu" + + page.find("ul") do |menu| + expect(menu).to have_link "Valuation" + expect(menu).to have_link count: 1 + end + end + + it "shows the management link to managers" do + user = create(:manager).user + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).to have_link "Menu" + + page.find("ul") do |menu| + expect(menu).to have_link "Management" + expect(menu).to have_link count: 1 + end + end + + it "shows the SDG content link to SDG managers" do + user = create(:sdg_manager).user + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).to have_link "Menu" + + page.find("ul") do |menu| + expect(menu).to have_link "SDG content" + expect(menu).to have_link count: 1 + end + end + + it "does not show the SDG content link when the SDG feature is disabled" do + Setting["feature.sdg"] = false + user = create(:administrator).user + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).to have_link "Menu" + + page.find("ul") do |menu| + expect(menu).to have_link "Administration" + expect(menu).to have_link "Moderation" + expect(menu).to have_link "Valuation" + expect(menu).to have_link "Management" + expect(menu).to have_link count: 4 + end + end + + it "shows the officing link to poll officers" do + user = create(:poll_officer, polls: [create(:poll)]).user + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).to have_link "Menu" + + page.find("ul") do |menu| + expect(menu).to have_link "Polling officers" + expect(menu).to have_link count: 1 + end + end + + it "shows several links to users with several roles" do + user = create(:user) + create(:moderator, user: user) + create(:manager, user: user) + + render_inline Layout::AdminLoginItemsComponent.new(user) + + expect(page).to have_link "Menu" + + page.find("ul") do |menu| + expect(menu).to have_link "Moderation" + expect(menu).to have_link "Management" + expect(menu).to have_link count: 2 + end + end +end diff --git a/spec/system/admin_spec.rb b/spec/system/admin_spec.rb index 88e8000da..1be11bb6e 100644 --- a/spec/system/admin_spec.rb +++ b/spec/system/admin_spec.rb @@ -68,19 +68,6 @@ describe "Admin" do expect(page).not_to have_content "You do not have permission to access this page" end - scenario "Admin access links", :admin do - Setting["feature.sdg"] = true - - visit root_path - click_link "Menu" - - expect(page).to have_link("Administration") - expect(page).to have_link("Moderation") - expect(page).to have_link("Valuation") - expect(page).to have_link("Management") - expect(page).to have_link("SDG content") - end - scenario "Admin dashboard", :admin do visit root_path diff --git a/spec/system/moderation_spec.rb b/spec/system/moderation_spec.rb index acb1a2690..2e4040cb6 100644 --- a/spec/system/moderation_spec.rb +++ b/spec/system/moderation_spec.rb @@ -6,11 +6,6 @@ describe "Moderation" do scenario "Access as regular user is not authorized" do login_as(user) - visit root_path - - expect(page).not_to have_link("Menu") - expect(page).not_to have_link("Moderation") - visit moderation_root_path expect(page).not_to have_current_path(moderation_root_path) @@ -22,11 +17,6 @@ describe "Moderation" do create(:valuator, user: user) login_as(user) - visit root_path - click_link "Menu" - - expect(page).not_to have_link("Moderation") - visit moderation_root_path expect(page).not_to have_current_path(moderation_root_path) @@ -38,11 +28,6 @@ describe "Moderation" do create(:manager, user: user) login_as(user) - visit root_path - click_link "Menu" - - expect(page).not_to have_link("Moderation") - visit moderation_root_path expect(page).not_to have_current_path(moderation_root_path) @@ -54,11 +39,6 @@ describe "Moderation" do create(:sdg_manager, user: user) login_as(user) - visit root_path - click_link "Menu" - - expect(page).not_to have_link("Moderation") - visit moderation_root_path expect(page).not_to have_current_path(moderation_root_path) @@ -70,11 +50,6 @@ describe "Moderation" do create(:poll_officer, user: user) login_as(user) - visit root_path - click_link "Menu" - - expect(page).not_to have_link("Moderation") - visit moderation_root_path expect(page).not_to have_current_path(moderation_root_path) @@ -106,18 +81,6 @@ describe "Moderation" do expect(page).not_to have_content "You do not have permission to access this page" end - scenario "Moderation access links" do - create(:moderator, user: user) - login_as(user) - - visit root_path - click_link "Menu" - - expect(page).to have_link("Moderation") - expect(page).not_to have_link("Administration") - expect(page).not_to have_link("Valuation") - end - context "Moderation dashboard" do before do Setting["org_name"] = "OrgName" diff --git a/spec/system/officing_spec.rb b/spec/system/officing_spec.rb index c66a9188e..3d92c2016 100644 --- a/spec/system/officing_spec.rb +++ b/spec/system/officing_spec.rb @@ -6,10 +6,6 @@ describe "Poll Officing" do scenario "Access as regular user is not authorized" do login_as(user) - visit root_path - - expect(page).not_to have_content "Menu" - expect(page).not_to have_link "Polling officers" visit officing_root_path @@ -20,13 +16,7 @@ describe "Poll Officing" do scenario "Access as moderator is not authorized" do create(:moderator, user: user) - login_as(user) - visit root_path - click_link "Menu" - - expect(page).to have_link "Moderation" - expect(page).not_to have_link "Polling officers" visit officing_root_path @@ -37,13 +27,7 @@ describe "Poll Officing" do scenario "Access as manager is not authorized" do create(:manager, user: user) - login_as(user) - visit root_path - click_link "Menu" - - expect(page).to have_link "Management" - expect(page).not_to have_link "Polling officers" visit officing_root_path @@ -55,13 +39,7 @@ describe "Poll Officing" do scenario "Access as SDG manager is not authorized" do Setting["feature.sdg"] = true create(:sdg_manager, user: user) - login_as(user) - visit root_path - click_link "Menu" - - expect(page).to have_link "SDG content" - expect(page).not_to have_link "Polling officers" visit officing_root_path @@ -72,13 +50,7 @@ describe "Poll Officing" do scenario "Access as a valuator is not authorized" do create(:valuator, user: user) - login_as(user) - visit root_path - click_link "Menu" - - expect(page).to have_link "Valuation" - expect(page).not_to have_link "Polling officers" visit officing_root_path @@ -90,14 +62,7 @@ describe "Poll Officing" do scenario "Access as an administrator is not authorized" do create(:administrator, user: user) create(:poll) - login_as(user) - visit root_path - - click_link "Menu" - - expect(page).to have_link "Administration" - expect(page).not_to have_link "Polling officers" visit officing_root_path @@ -133,20 +98,6 @@ describe "Poll Officing" do expect(page).not_to have_content "You do not have permission to access this page" end - scenario "Poll officer access links" do - create(:poll) - create(:poll_officer, user: user) - login_as(user) - visit root_path - - click_link "Menu" - - expect(page).to have_link("Polling officers") - expect(page).not_to have_link("Valuation") - expect(page).not_to have_link("Administration") - expect(page).not_to have_link("Moderation") - end - scenario "Officing dashboard" do create(:poll_officer, user: user) create(:poll) diff --git a/spec/system/sdg_management/sdg_spec.rb b/spec/system/sdg_management/sdg_spec.rb deleted file mode 100644 index f8b37319d..000000000 --- a/spec/system/sdg_management/sdg_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -require "rails_helper" - -describe "SDG Management" do - before { login_as(create(:administrator).user) } - - context "SDG feature flag is enabled" do - before { Setting["feature.sdg"] = true } - - scenario "shows the SDG content link" do - visit root_path - click_link "Menu" - - expect(page).to have_link "SDG content" - end - end - - context "SDG feature is disabled" do - before { Setting["feature.sdg"] = false } - - scenario "does not show the SDG Content link" do - visit root_path - click_link "Menu" - - expect(page).not_to have_link "SDG content" - end - end -end diff --git a/spec/system/sdg_management_spec.rb b/spec/system/sdg_management_spec.rb index fe058cf10..41be8606b 100644 --- a/spec/system/sdg_management_spec.rb +++ b/spec/system/sdg_management_spec.rb @@ -8,10 +8,7 @@ describe "SDGManagement" do context "Access" do scenario "Access as regular user is not authorized" do login_as(user) - visit root_path - expect(page).not_to have_link("Menu") - expect(page).not_to have_link("SDG content") visit sdg_management_root_path expect(page).not_to have_current_path(sdg_management_root_path) @@ -22,10 +19,7 @@ describe "SDGManagement" do scenario "Access as manager is not authorized" do create(:manager, user: user) login_as(user) - visit root_path - click_on "Menu" - expect(page).not_to have_link("SDG content") visit sdg_management_root_path expect(page).not_to have_current_path(sdg_management_root_path) @@ -46,19 +40,6 @@ describe "SDGManagement" do end end - scenario "Valuation access links" do - create(:sdg_manager, user: user) - - login_as(user) - visit root_path - click_on "Menu" - - expect(page).to have_link("SDG content") - expect(page).not_to have_link("Administration") - expect(page).not_to have_link("Moderation") - expect(page).not_to have_link("Valuation") - end - scenario "Valuation dashboard" do create(:sdg_manager, user: user) diff --git a/spec/system/valuation/budget_investments_spec.rb b/spec/system/valuation/budget_investments_spec.rb index e02ea6b5d..38fe053aa 100644 --- a/spec/system/valuation/budget_investments_spec.rb +++ b/spec/system/valuation/budget_investments_spec.rb @@ -20,13 +20,6 @@ describe "Valuation budget investments" do end end - scenario "Display link to valuation section" do - visit root_path - click_link "Menu" - - expect(page).to have_link "Valuation", href: valuation_root_path - end - describe "Index" do scenario "Index shows budget investments assigned to current valuator" do investment1 = create(:budget_investment, :visible_to_valuators, budget: budget, valuators: [valuator]) diff --git a/spec/system/valuation_spec.rb b/spec/system/valuation_spec.rb index 2359ee46a..8fc6c7b29 100644 --- a/spec/system/valuation_spec.rb +++ b/spec/system/valuation_spec.rb @@ -6,10 +6,6 @@ describe "Valuation" do context "Access" do scenario "Access as regular user is not authorized" do login_as(user) - visit root_path - - expect(page).not_to have_link("Menu") - expect(page).not_to have_link("Valuation") visit valuation_root_path @@ -22,11 +18,6 @@ describe "Valuation" do create(:moderator, user: user) login_as(user) - visit root_path - click_link "Menu" - - expect(page).not_to have_link("Valuation") - visit valuation_root_path expect(page).not_to have_current_path(valuation_root_path) @@ -38,11 +29,6 @@ describe "Valuation" do create(:manager, user: user) login_as(user) - visit root_path - click_link "Menu" - - expect(page).not_to have_link("Valuation") - visit valuation_root_path expect(page).not_to have_current_path(valuation_root_path) @@ -54,11 +40,6 @@ describe "Valuation" do create(:sdg_manager, user: user) login_as(user) - visit root_path - click_link "Menu" - - expect(page).not_to have_link("Valuation") - visit valuation_root_path expect(page).not_to have_current_path(valuation_root_path) @@ -70,11 +51,6 @@ describe "Valuation" do create(:poll_officer, user: user) login_as(user) - visit root_path - click_link "Menu" - - expect(page).not_to have_link("Valuation") - visit valuation_root_path expect(page).not_to have_current_path(valuation_root_path) @@ -109,19 +85,6 @@ describe "Valuation" do end end - scenario "Valuation access links" do - create(:valuator, user: user) - create(:budget) - login_as(user) - - visit root_path - click_link "Menu" - - expect(page).to have_link("Valuation") - expect(page).not_to have_link("Administration") - expect(page).not_to have_link("Moderation") - end - scenario "Valuation dashboard" do create(:valuator, user: user) create(:budget) From 2e9c4de061e42d001233557c1653c8b3cdc07fcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 3 Feb 2023 16:59:43 +0100 Subject: [PATCH 2/3] Unify specs testing access to admin menu items In most sections, we had two specs testing what happens after accessing one of the privileged areas. We're grouping the expectations and so we've only got one test per area, making these tests faster. --- spec/system/admin_spec.rb | 14 ++++---------- spec/system/moderation_spec.rb | 26 +++++--------------------- spec/system/officing_spec.rb | 24 ++++++------------------ spec/system/sdg_management_spec.rb | 19 ++++--------------- spec/system/valuation_spec.rb | 19 +++---------------- 5 files changed, 22 insertions(+), 80 deletions(-) diff --git a/spec/system/admin_spec.rb b/spec/system/admin_spec.rb index 1be11bb6e..a0f31c04d 100644 --- a/spec/system/admin_spec.rb +++ b/spec/system/admin_spec.rb @@ -62,22 +62,16 @@ describe "Admin" do end scenario "Access as administrator is authorized", :admin do - visit admin_root_path - - expect(page).to have_current_path(admin_root_path) - expect(page).not_to have_content "You do not have permission to access this page" - end - - scenario "Admin dashboard", :admin do visit root_path click_link "Menu" click_link "Administration" expect(page).to have_current_path(admin_root_path) - expect(page).to have_css("#admin_menu") - expect(page).not_to have_css("#moderation_menu") - expect(page).not_to have_css("#valuation_menu") + expect(page).to have_css "#admin_menu" + expect(page).not_to have_css "#moderation_menu" + expect(page).not_to have_css "#valuation_menu" + expect(page).not_to have_content "You do not have permission to access this page" end scenario "Admin menu does not hide active elements", :admin do diff --git a/spec/system/moderation_spec.rb b/spec/system/moderation_spec.rb index 2e4040cb6..99b4b0086 100644 --- a/spec/system/moderation_spec.rb +++ b/spec/system/moderation_spec.rb @@ -58,6 +58,7 @@ describe "Moderation" do end scenario "Access as a moderator is authorized" do + Setting["org_name"] = "OrgName" create(:moderator, user: user) login_as(user) @@ -66,6 +67,10 @@ describe "Moderation" do click_link "Moderation" expect(page).to have_current_path(moderation_root_path) + expect(page).to have_link "Go back to OrgName" + expect(page).to have_css "#moderation_menu" + expect(page).not_to have_css "#admin_menu" + expect(page).not_to have_css "#valuation_menu" expect(page).not_to have_content "You do not have permission to access this page" end @@ -80,25 +85,4 @@ describe "Moderation" do expect(page).to have_current_path(moderation_root_path) expect(page).not_to have_content "You do not have permission to access this page" end - - context "Moderation dashboard" do - before do - Setting["org_name"] = "OrgName" - end - - scenario "Contains correct elements" do - create(:moderator, user: user) - login_as(user) - - visit root_path - click_link "Menu" - click_link "Moderation" - - expect(page).to have_link("Go back to OrgName") - expect(page).to have_current_path(moderation_root_path) - expect(page).to have_css("#moderation_menu") - expect(page).not_to have_css("#admin_menu") - expect(page).not_to have_css("#valuation_menu") - end - end end diff --git a/spec/system/officing_spec.rb b/spec/system/officing_spec.rb index 3d92c2016..f3c791309 100644 --- a/spec/system/officing_spec.rb +++ b/spec/system/officing_spec.rb @@ -85,7 +85,7 @@ describe "Poll Officing" do expect(page).not_to have_content "You do not have permission to access this page" end - scenario "Access as an poll officer is authorized" do + scenario "Access as a poll officer is authorized" do create(:poll_officer, user: user) create(:poll) login_as(user) @@ -95,26 +95,14 @@ describe "Poll Officing" do click_link "Polling officers" expect(page).to have_current_path(officing_root_path) + expect(page).to have_css "#officing_menu" + expect(page).not_to have_link "Polling officers" + expect(page).not_to have_css "#valuation_menu" + expect(page).not_to have_css "#admin_menu" + expect(page).not_to have_css "#moderation_menu" expect(page).not_to have_content "You do not have permission to access this page" end - scenario "Officing dashboard" do - create(:poll_officer, user: user) - create(:poll) - login_as(user) - visit root_path - - click_link "Menu" - click_link "Polling officers" - - expect(page).to have_current_path(officing_root_path) - expect(page).to have_css("#officing_menu") - expect(page).not_to have_link("Polling officers") - expect(page).not_to have_css("#valuation_menu") - expect(page).not_to have_css("#admin_menu") - expect(page).not_to have_css("#moderation_menu") - end - scenario "Officing dashboard available for multiple sessions", :with_frozen_time do poll = create(:poll) booth = create(:poll_booth) diff --git a/spec/system/sdg_management_spec.rb b/spec/system/sdg_management_spec.rb index 41be8606b..7b7e6bfef 100644 --- a/spec/system/sdg_management_spec.rb +++ b/spec/system/sdg_management_spec.rb @@ -36,22 +36,11 @@ describe "SDGManagement" do click_on "SDG content" expect(page).to have_current_path(sdg_management_root_path) + expect(page).to have_css ".sdg-content-menu" + expect(page).not_to have_css "#valuation_menu" + expect(page).not_to have_css "#admin_menu" + expect(page).not_to have_css "#moderation_menu" expect(page).not_to have_content "You do not have permission to access this page" end end - - scenario "Valuation dashboard" do - create(:sdg_manager, user: user) - - login_as(user) - visit root_path - click_on "Menu" - click_on "SDG content" - - expect(page).to have_current_path(sdg_management_root_path) - expect(page).to have_css(".sdg-content-menu") - expect(page).not_to have_css("#valuation_menu") - expect(page).not_to have_css("#admin_menu") - expect(page).not_to have_css("#moderation_menu") - end end diff --git a/spec/system/valuation_spec.rb b/spec/system/valuation_spec.rb index 8fc6c7b29..53bf6c2bc 100644 --- a/spec/system/valuation_spec.rb +++ b/spec/system/valuation_spec.rb @@ -68,6 +68,9 @@ describe "Valuation" do click_link "Valuation" expect(page).to have_current_path(valuation_root_path) + expect(page).to have_css "#valuation_menu" + expect(page).not_to have_css "#admin_menu" + expect(page).not_to have_css "#moderation_menu" expect(page).not_to have_content "You do not have permission to access this page" end @@ -84,20 +87,4 @@ describe "Valuation" do expect(page).not_to have_content "You do not have permission to access this page" end end - - scenario "Valuation dashboard" do - create(:valuator, user: user) - create(:budget) - - login_as(user) - visit root_path - - click_link "Menu" - click_link "Valuation" - - expect(page).to have_current_path(valuation_root_path) - expect(page).to have_css("#valuation_menu") - expect(page).not_to have_css("#admin_menu") - expect(page).not_to have_css("#moderation_menu") - end end From 1ff214483022eedfcffa0ca70440e95f57afe0bf Mon Sep 17 00:00:00 2001 From: decabeza Date: Tue, 25 Oct 2022 15:25:57 +0200 Subject: [PATCH 3/3] Show always poll officer menu to officers Since the change on commit cbbe188d6 we added a Poll.current.any? condition to show the officing link on admin menu to officers. That condition doesn't have much sense since Poll results only can be added after a poll has ended, and there may be only one active poll. --- app/components/layout/admin_login_items_component.rb | 2 +- spec/components/layout/admin_login_items_component_spec.rb | 2 +- spec/system/officing_spec.rb | 3 --- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/components/layout/admin_login_items_component.rb b/app/components/layout/admin_login_items_component.rb index 44dd2d429..2d7e85587 100644 --- a/app/components/layout/admin_login_items_component.rb +++ b/app/components/layout/admin_login_items_component.rb @@ -18,7 +18,7 @@ class Layout::AdminLoginItemsComponent < ApplicationComponent (moderation_link if user.administrator? || user.moderator?), (valuation_link if feature?(:budgets) && (user.administrator? || user.valuator?)), (management_link if user.administrator? || user.manager?), - (officing_link if user.poll_officer? && Poll.current.any?), + (officing_link if user.poll_officer?), (sdg_management_link if feature?(:sdg) && (user.administrator? || user.sdg_manager?)) ] end diff --git a/spec/components/layout/admin_login_items_component_spec.rb b/spec/components/layout/admin_login_items_component_spec.rb index def3ff068..8d1316ec1 100644 --- a/spec/components/layout/admin_login_items_component_spec.rb +++ b/spec/components/layout/admin_login_items_component_spec.rb @@ -102,7 +102,7 @@ describe Layout::AdminLoginItemsComponent do end it "shows the officing link to poll officers" do - user = create(:poll_officer, polls: [create(:poll)]).user + user = create(:poll_officer).user render_inline Layout::AdminLoginItemsComponent.new(user) diff --git a/spec/system/officing_spec.rb b/spec/system/officing_spec.rb index f3c791309..c48bafb48 100644 --- a/spec/system/officing_spec.rb +++ b/spec/system/officing_spec.rb @@ -61,7 +61,6 @@ describe "Poll Officing" do scenario "Access as an administrator is not authorized" do create(:administrator, user: user) - create(:poll) login_as(user) visit officing_root_path @@ -74,7 +73,6 @@ describe "Poll Officing" do scenario "Access as an administrator with poll officer role is authorized" do create(:administrator, user: user) create(:poll_officer, user: user) - create(:poll) login_as(user) visit root_path @@ -87,7 +85,6 @@ describe "Poll Officing" do scenario "Access as a poll officer is authorized" do create(:poll_officer, user: user) - create(:poll) login_as(user) visit root_path