From 5a9963ffa5f0f56c448e5f8a01ccc2f610f6a5b7 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 19 Jun 2024 18:47:30 +0200 Subject: [PATCH 01/33] Move the controller into general space --- app/controllers/{work_packages => }/shares/bulk_controller.rb | 2 +- app/controllers/{work_packages => }/shares_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename app/controllers/{work_packages => }/shares/bulk_controller.rb (98%) rename app/controllers/{work_packages => }/shares_controller.rb (99%) diff --git a/app/controllers/work_packages/shares/bulk_controller.rb b/app/controllers/shares/bulk_controller.rb similarity index 98% rename from app/controllers/work_packages/shares/bulk_controller.rb rename to app/controllers/shares/bulk_controller.rb index 79db00e72854..0ec310ebbb4c 100644 --- a/app/controllers/work_packages/shares/bulk_controller.rb +++ b/app/controllers/shares/bulk_controller.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -class WorkPackages::Shares::BulkController < ApplicationController +class Shares::BulkController < ApplicationController include OpTurbo::ComponentStream include MemberHelper diff --git a/app/controllers/work_packages/shares_controller.rb b/app/controllers/shares_controller.rb similarity index 99% rename from app/controllers/work_packages/shares_controller.rb rename to app/controllers/shares_controller.rb index 930f2fc8dfc6..0d67150d709d 100644 --- a/app/controllers/work_packages/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -class WorkPackages::SharesController < ApplicationController +class SharesController < ApplicationController include OpTurbo::ComponentStream include MemberHelper From 7da9d81872ba29262f87ac16bf0ec00670f72d88 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 19 Jun 2024 18:47:46 +0200 Subject: [PATCH 02/33] Extract a route concern and make it work with the `/state` route --- config/routes.rb | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 739003785c74..f60b41d009e2 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -76,6 +76,20 @@ via: :all end + # Shared route concerns + # TODO: Add description how to configure controller to support shares + concern :shareable do + resources :members, path: :shares, controller: "shares", only: %i[index create update destroy] do + member do + post "resend_invite" => "shares#resend_invite" + end + + collection do + resource :bulk, controller: "shares/bulk", only: %i[update destroy], as: :shares_bulk + end + end + end + scope controller: "account" do get "/account/force_password_change", action: "force_password_change" post "/account/change_password", action: "change_password" @@ -521,7 +535,7 @@ get "/bulk" => "bulk#destroy" end - resources :work_packages, only: [:index] do + resources :work_packages, only: [:index], concerns: [:shareable] do # move bulk of wps get "move/new" => "work_packages/moves#new", on: :collection, as: "new_move" post "move" => "work_packages/moves#create", on: :collection, as: "move" @@ -531,16 +545,6 @@ # states managed by client-side routing on work_package#index get "details/*state" => "work_packages#index", on: :collection, as: :details - # Rails managed sharing route - resources :members, path: :shares, controller: "work_packages/shares", only: %i[index create update destroy] do - member do - post "resend_invite" => "work_packages/shares#resend_invite" - end - collection do - resource :bulk, controller: "work_packages/shares/bulk", only: %i[update destroy], as: :shares_bulk - end - end - resource :progress, only: %i[new edit update], controller: "work_packages/progress" collection do resource :progress, @@ -554,7 +558,7 @@ get "/create_new" => "work_packages#index", on: :collection, as: "new_split" get "/new" => "work_packages#index", on: :collection, as: "new", state: "new" # We do not want to match the work package export routes - get "(/*state)" => "work_packages#show", on: :member, as: "", constraints: { id: /\d+/ } + get "(/*state)" => "work_packages#show", on: :member, as: "", constraints: { id: /\d+/, state: /(?!shares)/ } get "/share_upsale" => "work_packages#index", on: :collection, as: "share_upsale" get "/edit" => "work_packages#show", on: :member, as: "edit" end From 31b2f8f2e8f6044cd90cdc73a9bfad224f28ae96 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 19 Jun 2024 18:48:10 +0200 Subject: [PATCH 03/33] Change permitted by the share permission --- config/initializers/permissions.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/initializers/permissions.rb b/config/initializers/permissions.rb index 4a71cd8380b1..9dd015f92837 100644 --- a/config/initializers/permissions.rb +++ b/config/initializers/permissions.rb @@ -331,8 +331,8 @@ map.permission :share_work_packages, { members: %i[destroy_by_principal], - "work_packages/shares": %i[index create destroy update resend_invite], - "work_packages/shares/bulk": %i[update destroy] + shares: %i[index create destroy update resend_invite], + "shares/bulk": %i[update destroy] }, permissible_on: :project, dependencies: %i[edit_work_packages view_shared_work_packages], @@ -340,7 +340,7 @@ map.permission :view_shared_work_packages, { - "work_packages/shares": %i[index] + shares: %i[index] }, permissible_on: :project, require: :member, From cf4c479580f91b5dd7ecfb893362ccc4217bd071 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 19 Jun 2024 19:21:59 +0200 Subject: [PATCH 04/33] Fix routing specs --- app/controllers/members_controller.rb | 4 +-- spec/routing/work_package/shares_spec.rb | 32 ++++++++++++------------ spec/routing/work_packages_spec.rb | 4 +-- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index 4b64724d3fe5..b18e0bbc2b65 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -144,8 +144,8 @@ def members_table_options(roles) available_roles: roles, authorize_update: authorize_for("members", :update), authorize_delete: authorize_for("members", :destroy), - authorize_work_package_shares_view: authorize_for("work_packages/shares", :update), - authorize_work_package_shares_delete: authorize_for("work_packages/shares/bulk", :destroy), + authorize_work_package_shares_view: authorize_for("shares", :update), + authorize_work_package_shares_delete: authorize_for("shares/bulk", :destroy), authorize_manage_user: current_user.allowed_globally?(:manage_user), is_filtered: Members::UserFilterComponent.filtered?(params), shared_role_name: diff --git a/spec/routing/work_package/shares_spec.rb b/spec/routing/work_package/shares_spec.rb index 61a7cd5d2f4f..48c6e25ec28a 100644 --- a/spec/routing/work_package/shares_spec.rb +++ b/spec/routing/work_package/shares_spec.rb @@ -29,57 +29,57 @@ require "spec_helper" RSpec.describe "work package share routes" do - it "connects GET /work_packages/:wp_id/shares to work_packages/shares#index" do - expect(get("/work_packages/1/shares")).to route_to(controller: "work_packages/shares", + it "connects GET /work_packages/:wp_id/shares to shares#index" do + expect(get("/work_packages/1/shares")).to route_to(controller: "shares", action: "index", work_package_id: "1") end - it "connects POST /work_packages/:wp_id/shares to work_packages/shares#create" do - expect(post("/work_packages/1/shares")).to route_to(controller: "work_packages/shares", + it "connects POST /work_packages/:wp_id/shares to shares#create" do + expect(post("/work_packages/1/shares")).to route_to(controller: "shares", action: "create", work_package_id: "1") end - it "connects DELETE /work_packages/:wp_id/shares/:id to work_packages/shares#delete" do - expect(delete("/work_packages/1/shares/5")).to route_to(controller: "work_packages/shares", + it "connects DELETE /work_packages/:wp_id/shares/:id to shares#delete" do + expect(delete("/work_packages/1/shares/5")).to route_to(controller: "shares", action: "destroy", id: "5", work_package_id: "1") end - it "connects PATCH /work_packages/:wp_id/shares/:id to work_packages/shares#update" do - expect(patch("/work_packages/1/shares/5")).to route_to(controller: "work_packages/shares", + it "connects PATCH /work_packages/:wp_id/shares/:id to shares#update" do + expect(patch("/work_packages/1/shares/5")).to route_to(controller: "shares", action: "update", id: "5", work_package_id: "1") end - it "connects PUT /work_packages/:wp_id/shares/:id to work_packages/shares#update" do - expect(put("/work_packages/1/shares/5")).to route_to(controller: "work_packages/shares", + it "connects PUT /work_packages/:wp_id/shares/:id to shares#update" do + expect(put("/work_packages/1/shares/5")).to route_to(controller: "shares", action: "update", id: "5", work_package_id: "1") end - it "connects POST /work_packages/:wp_id/shares/:id/resend_invite to work_packages/shares#resend_invite" do - expect(post("/work_packages/1/shares/5/resend_invite")).to route_to(controller: "work_packages/shares", + it "connects POST /work_packages/:wp_id/shares/:id/resend_invite to shares#resend_invite" do + expect(post("/work_packages/1/shares/5/resend_invite")).to route_to(controller: "shares", action: "resend_invite", id: "5", work_package_id: "1") end context "on bulk actions" do - it "routes DELETE /work_packages/:work_package_id/shares/bulk to work_packages/shares/bulk#destroy" do + it "routes DELETE /work_packages/:work_package_id/shares/bulk to shares/bulk#destroy" do expect(delete("/work_packages/1/shares/bulk")) - .to route_to(controller: "work_packages/shares/bulk", + .to route_to(controller: "shares/bulk", action: "destroy", work_package_id: "1") end - it "routes PATCH /work_packages/:work_package_id/shares/bulk to work_packages/shares/bulk#update" do + it "routes PATCH /work_packages/:work_package_id/shares/bulk to shares/bulk#update" do expect(patch("/work_packages/1/shares/bulk")) - .to route_to(controller: "work_packages/shares/bulk", + .to route_to(controller: "shares/bulk", action: "update", work_package_id: "1") end diff --git a/spec/routing/work_packages_spec.rb b/spec/routing/work_packages_spec.rb index d80f95e60b2b..2b0b070ac2af 100644 --- a/spec/routing/work_packages_spec.rb +++ b/spec/routing/work_packages_spec.rb @@ -92,13 +92,13 @@ end it "connects GET /work_packages/:id/share to work_packages/shares#index" do - expect(get("/work_packages/1/shares")).to route_to(controller: "work_packages/shares", + expect(get("/work_packages/1/shares")).to route_to(controller: "shares", action: "index", work_package_id: "1") end it "connects POST /work_packages/:id/share to work_packages/shares#create" do - expect(post("/work_packages/1/shares")).to route_to(controller: "work_packages/shares", + expect(post("/work_packages/1/shares")).to route_to(controller: "shares", action: "create", work_package_id: "1") end From aa04032acd1aac4b4680ce11a441921ea00f1655 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 19 Jun 2024 20:07:46 +0200 Subject: [PATCH 05/33] Fix regex --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index f60b41d009e2..230749ce49f7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -558,7 +558,7 @@ get "/create_new" => "work_packages#index", on: :collection, as: "new_split" get "/new" => "work_packages#index", on: :collection, as: "new", state: "new" # We do not want to match the work package export routes - get "(/*state)" => "work_packages#show", on: :member, as: "", constraints: { id: /\d+/, state: /(?!shares)/ } + get "(/*state)" => "work_packages#show", on: :member, as: "", constraints: { id: /\d+/, state: /(?!shares).+/ } get "/share_upsale" => "work_packages#index", on: :collection, as: "share_upsale" get "/edit" => "work_packages#show", on: :member, as: "edit" end From d8967dcfbe78b109f59fd34b3a12c8b792126823 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Thu, 20 Jun 2024 11:26:15 +0200 Subject: [PATCH 06/33] Properly define bulk routes without a resource --- config/routes.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 230749ce49f7..c8329727ee62 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -85,7 +85,9 @@ end collection do - resource :bulk, controller: "shares/bulk", only: %i[update destroy], as: :shares_bulk + patch :bulk, to: "shares/bulk#update" + put :bulk, to: "shares/bulk#update" + delete :bulk, to: "shares/bulk#destroy" end end end From cb9ccc92c91e054c5b0751454807c4195b898b07 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Thu, 20 Jun 2024 11:26:33 +0200 Subject: [PATCH 07/33] Reorganize sharing components outside of the WorkPackage scope --- app/components/_index.sass | 4 +- .../bulk_permission_button_component.html.erb | 0 .../bulk_permission_button_component.rb | 20 +- .../bulk_selection_counter_component.html.erb | 0 .../bulk_selection_counter_component.rb | 18 +- .../concerns/authorization.rb | 16 +- .../concerns/displayable_roles.rb | 32 ++-- .../counter_component.html.erb | 4 +- .../share => shares}/counter_component.rb | 36 ++-- .../invite_user_form_component.html.erb | 4 +- .../invite_user_form_component.rb | 28 ++- .../invite_user_form_component.sass | 0 .../modal_body_component.html.erb | 10 +- app/components/shares/modal_body_component.rb | 178 +++++++++++++++++ .../modal_body_component.sass | 0 .../modal_upsale_component.html.erb | 0 .../modal_upsale_component.rb | 16 +- .../permission_button_component.html.erb | 0 .../shares/permission_button_component.rb | 84 ++++++++ .../share_counter_component.html.erb | 0 .../share_counter_component.rb | 18 +- .../share_row_component.html.erb | 4 +- .../share => shares}/share_row_component.rb | 74 ++++--- .../user_details_component.html.erb | 0 .../shares/user_details_component.rb | 127 ++++++++++++ .../share/modal_body_component.rb | 180 ------------------ .../share/permission_button_component.rb | 86 --------- .../share/user_details_component.rb | 131 ------------- app/controllers/shares/bulk_controller.rb | 10 +- app/controllers/shares_controller.rb | 24 +-- .../share => shares}/invitee.rb | 2 +- .../user_details_component_spec.rb | 2 +- 32 files changed, 541 insertions(+), 567 deletions(-) rename app/components/{work_packages/share => shares}/bulk_permission_button_component.html.erb (100%) rename app/components/{work_packages/share => shares}/bulk_permission_button_component.rb (78%) rename app/components/{work_packages/share => shares}/bulk_selection_counter_component.html.erb (100%) rename app/components/{work_packages/share => shares}/bulk_selection_counter_component.rb (85%) rename app/components/{work_packages/share => shares}/concerns/authorization.rb (82%) rename app/components/{work_packages/share => shares}/concerns/displayable_roles.rb (61%) rename app/components/{work_packages/share => shares}/counter_component.html.erb (76%) rename app/components/{work_packages/share => shares}/counter_component.rb (67%) rename app/components/{work_packages/share => shares}/invite_user_form_component.html.erb (96%) rename app/components/{work_packages/share => shares}/invite_user_form_component.rb (69%) rename app/components/{work_packages/share => shares}/invite_user_form_component.sass (100%) rename app/components/{work_packages/share => shares}/modal_body_component.html.erb (91%) create mode 100644 app/components/shares/modal_body_component.rb rename app/components/{work_packages/share => shares}/modal_body_component.sass (100%) rename app/components/{work_packages/share => shares}/modal_upsale_component.html.erb (100%) rename app/components/{work_packages/share => shares}/modal_upsale_component.rb (82%) rename app/components/{work_packages/share => shares}/permission_button_component.html.erb (100%) create mode 100644 app/components/shares/permission_button_component.rb rename app/components/{work_packages/share => shares}/share_counter_component.html.erb (100%) rename app/components/{work_packages/share => shares}/share_counter_component.rb (86%) rename app/components/{work_packages/share => shares}/share_row_component.html.erb (91%) rename app/components/{work_packages/share => shares}/share_row_component.rb (53%) rename app/components/{work_packages/share => shares}/user_details_component.html.erb (100%) create mode 100644 app/components/shares/user_details_component.rb delete mode 100644 app/components/work_packages/share/modal_body_component.rb delete mode 100644 app/components/work_packages/share/permission_button_component.rb delete mode 100644 app/components/work_packages/share/user_details_component.rb rename app/forms/{work_packages/share => shares}/invitee.rb (98%) rename spec/components/{work_packages/share => shares}/user_details_component_spec.rb (99%) diff --git a/app/components/_index.sass b/app/components/_index.sass index fa7b42a43c7f..a05d3e6b1e6e 100644 --- a/app/components/_index.sass +++ b/app/components/_index.sass @@ -1,5 +1,5 @@ -@import "work_packages/share/modal_body_component" -@import "work_packages/share/invite_user_form_component" +@import "shares/modal_body_component" +@import "shares/invite_user_form_component" @import "work_packages/progress/modal_body_component" @import "open_project/common/attribute_component" @import "filter/filters_component" diff --git a/app/components/work_packages/share/bulk_permission_button_component.html.erb b/app/components/shares/bulk_permission_button_component.html.erb similarity index 100% rename from app/components/work_packages/share/bulk_permission_button_component.html.erb rename to app/components/shares/bulk_permission_button_component.html.erb diff --git a/app/components/work_packages/share/bulk_permission_button_component.rb b/app/components/shares/bulk_permission_button_component.rb similarity index 78% rename from app/components/work_packages/share/bulk_permission_button_component.rb rename to app/components/shares/bulk_permission_button_component.rb index 1f0c2bad284c..ce6679a72be6 100644 --- a/app/components/work_packages/share/bulk_permission_button_component.rb +++ b/app/components/shares/bulk_permission_button_component.rb @@ -28,20 +28,18 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackages - module Share - class BulkPermissionButtonComponent < ApplicationComponent - include WorkPackages::Share::Concerns::DisplayableRoles +module Shares + class BulkPermissionButtonComponent < ApplicationComponent + include Shares::Concerns::DisplayableRoles - def initialize(work_package:) - super + def initialize(work_package:) + super - @work_package = work_package - end + @work_package = work_package + end - def update_path - work_package_shares_bulk_path(@work_package) - end + def update_path + url_for([:bulk, @work_package, Member]) end end end diff --git a/app/components/work_packages/share/bulk_selection_counter_component.html.erb b/app/components/shares/bulk_selection_counter_component.html.erb similarity index 100% rename from app/components/work_packages/share/bulk_selection_counter_component.html.erb rename to app/components/shares/bulk_selection_counter_component.html.erb diff --git a/app/components/work_packages/share/bulk_selection_counter_component.rb b/app/components/shares/bulk_selection_counter_component.rb similarity index 85% rename from app/components/work_packages/share/bulk_selection_counter_component.rb rename to app/components/shares/bulk_selection_counter_component.rb index 6dc141ba6e84..2196cc6dec48 100644 --- a/app/components/work_packages/share/bulk_selection_counter_component.rb +++ b/app/components/shares/bulk_selection_counter_component.rb @@ -28,18 +28,16 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackages - module Share - class BulkSelectionCounterComponent < ApplicationComponent - def initialize(count:) - super +module Shares + class BulkSelectionCounterComponent < ApplicationComponent + def initialize(count:) + super - @count = count - end + @count = count + end - private + private - attr_reader :count - end + attr_reader :count end end diff --git a/app/components/work_packages/share/concerns/authorization.rb b/app/components/shares/concerns/authorization.rb similarity index 82% rename from app/components/work_packages/share/concerns/authorization.rb rename to app/components/shares/concerns/authorization.rb index 4a927c0b5983..862e07166a72 100644 --- a/app/components/work_packages/share/concerns/authorization.rb +++ b/app/components/shares/concerns/authorization.rb @@ -28,16 +28,14 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackages - module Share - module Concerns - module Authorization - extend ActiveSupport::Concern +module Shares + module Concerns + module Authorization + extend ActiveSupport::Concern - included do - def sharing_manageable? - User.current.allowed_in_project?(:share_work_packages, @work_package.project) - end + included do + def sharing_manageable? + User.current.allowed_in_project?(:share_work_packages, @work_package.project) end end end diff --git a/app/components/work_packages/share/concerns/displayable_roles.rb b/app/components/shares/concerns/displayable_roles.rb similarity index 61% rename from app/components/work_packages/share/concerns/displayable_roles.rb rename to app/components/shares/concerns/displayable_roles.rb index ff0faa338347..79b887a48b4c 100644 --- a/app/components/work_packages/share/concerns/displayable_roles.rb +++ b/app/components/shares/concerns/displayable_roles.rb @@ -28,23 +28,21 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackages - module Share - module Concerns - module DisplayableRoles - def options - [ - { label: I18n.t("work_package.sharing.permissions.edit"), - value: Role::BUILTIN_WORK_PACKAGE_EDITOR, - description: I18n.t("work_package.sharing.permissions.edit_description") }, - { label: I18n.t("work_package.sharing.permissions.comment"), - value: Role::BUILTIN_WORK_PACKAGE_COMMENTER, - description: I18n.t("work_package.sharing.permissions.comment_description") }, - { label: I18n.t("work_package.sharing.permissions.view"), - value: Role::BUILTIN_WORK_PACKAGE_VIEWER, - description: I18n.t("work_package.sharing.permissions.view_description") } - ] - end +module Shares + module Concerns + module DisplayableRoles + def options + [ + { label: I18n.t("work_package.sharing.permissions.edit"), + value: Role::BUILTIN_WORK_PACKAGE_EDITOR, + description: I18n.t("work_package.sharing.permissions.edit_description") }, + { label: I18n.t("work_package.sharing.permissions.comment"), + value: Role::BUILTIN_WORK_PACKAGE_COMMENTER, + description: I18n.t("work_package.sharing.permissions.comment_description") }, + { label: I18n.t("work_package.sharing.permissions.view"), + value: Role::BUILTIN_WORK_PACKAGE_VIEWER, + description: I18n.t("work_package.sharing.permissions.view_description") } + ] end end end diff --git a/app/components/work_packages/share/counter_component.html.erb b/app/components/shares/counter_component.html.erb similarity index 76% rename from app/components/work_packages/share/counter_component.html.erb rename to app/components/shares/counter_component.html.erb index 71144ef8f38a..c2c1bfec88ff 100644 --- a/app/components/work_packages/share/counter_component.html.erb +++ b/app/components/shares/counter_component.html.erb @@ -5,9 +5,9 @@ # I'm able to manage shares if the only user that the work package is # currently shared is myself, since I'm not able to manage my own share. if sharing_manageable? && shared_with_anyone_else_other_than_myself? - render(WorkPackages::Share::BulkSelectionCounterComponent.new(count:)) + render(Shares::BulkSelectionCounterComponent.new(count:)) else - render(WorkPackages::Share::ShareCounterComponent.new(count:)) + render(Shares::ShareCounterComponent.new(count:)) end end end diff --git a/app/components/work_packages/share/counter_component.rb b/app/components/shares/counter_component.rb similarity index 67% rename from app/components/work_packages/share/counter_component.rb rename to app/components/shares/counter_component.rb index ee2b0d786da5..8da6c7f9cc16 100644 --- a/app/components/work_packages/share/counter_component.rb +++ b/app/components/shares/counter_component.rb @@ -28,30 +28,28 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackages - module Share - class CounterComponent < ApplicationComponent - include ApplicationHelper - include OpTurbo::Streamable - include OpPrimer::ComponentHelpers - include WorkPackages::Share::Concerns::Authorization +module Shares + class CounterComponent < ApplicationComponent + include ApplicationHelper + include OpTurbo::Streamable + include OpPrimer::ComponentHelpers + include Shares::Concerns::Authorization - def initialize(work_package:, count:) - super + def initialize(work_package:, count:) + super - @work_package = work_package - @count = count - end + @work_package = work_package + @count = count + end - private + private - attr_reader :work_package, :count + attr_reader :work_package, :count - def shared_with_anyone_else_other_than_myself? - Member.of_work_package(@work_package) - .where.not(principal: User.current) - .any? - end + def shared_with_anyone_else_other_than_myself? + Member.of_work_package(@work_package) + .where.not(principal: User.current) + .any? end end end diff --git a/app/components/work_packages/share/invite_user_form_component.html.erb b/app/components/shares/invite_user_form_component.html.erb similarity index 96% rename from app/components/work_packages/share/invite_user_form_component.html.erb rename to app/components/shares/invite_user_form_component.html.erb index 2cff37583ab6..8a62c75fa4f6 100644 --- a/app/components/work_packages/share/invite_user_form_component.html.erb +++ b/app/components/shares/invite_user_form_component.html.erb @@ -13,11 +13,11 @@ grid_layout('invite-user-form', tag: :div) do |invite_form| invite_form.with_area('invitee') do - render(WorkPackages::Share::Invitee.new(form)) + render(Shares::Invitee.new(form)) end invite_form.with_area('permission') do - render(WorkPackages::Share::PermissionButtonComponent.new( + render(Shares::PermissionButtonComponent.new( share: new_share, form_arguments: { builder: form, name: "role_id" }, data: { 'test-selector': 'op-share-wp-invite-role' }) diff --git a/app/components/work_packages/share/invite_user_form_component.rb b/app/components/shares/invite_user_form_component.rb similarity index 69% rename from app/components/work_packages/share/invite_user_form_component.rb rename to app/components/shares/invite_user_form_component.rb index 945256030394..1189f8b57624 100644 --- a/app/components/work_packages/share/invite_user_form_component.rb +++ b/app/components/shares/invite_user_form_component.rb @@ -26,24 +26,22 @@ # See COPYRIGHT and LICENSE files for more details. #++ -module WorkPackages - module Share - class InviteUserFormComponent < ApplicationComponent - include ApplicationHelper - include OpTurbo::Streamable - include OpPrimer::ComponentHelpers - include WorkPackages::Share::Concerns::Authorization +module Shares + class InviteUserFormComponent < ApplicationComponent + include ApplicationHelper + include OpTurbo::Streamable + include OpPrimer::ComponentHelpers + include Shares::Concerns::Authorization - def initialize(work_package:, errors: nil) - super + def initialize(work_package:, errors: nil) + super - @work_package = work_package - @errors = errors - end + @work_package = work_package + @errors = errors + end - def new_share - @new_share ||= Member.new(entity: @work_package, roles: [Role.new(builtin: Role::BUILTIN_WORK_PACKAGE_VIEWER)]) - end + def new_share + @new_share ||= Member.new(entity: @work_package, roles: [Role.new(builtin: Role::BUILTIN_WORK_PACKAGE_VIEWER)]) end end end diff --git a/app/components/work_packages/share/invite_user_form_component.sass b/app/components/shares/invite_user_form_component.sass similarity index 100% rename from app/components/work_packages/share/invite_user_form_component.sass rename to app/components/shares/invite_user_form_component.sass diff --git a/app/components/work_packages/share/modal_body_component.html.erb b/app/components/shares/modal_body_component.html.erb similarity index 91% rename from app/components/work_packages/share/modal_body_component.html.erb rename to app/components/shares/modal_body_component.html.erb index fc5bc15cce01..156ca7b249a3 100644 --- a/app/components/work_packages/share/modal_body_component.html.erb +++ b/app/components/shares/modal_body_component.html.erb @@ -2,7 +2,7 @@ component_wrapper(tag: 'turbo-frame') do flex_layout(data: { turbo: true }) do |modal_content| modal_content.with_row do - render(WorkPackages::Share::InviteUserFormComponent.new(work_package: @work_package, errors: @errors)) + render(Shares::InviteUserFormComponent.new(work_package: @work_package, errors: @errors)) end modal_content.with_row(mt: 3, @@ -13,7 +13,7 @@ border_box.with_header(color: :muted, data: { 'test-selector': 'op-share-wp-header' }) do grid_layout('op-share-wp-modal-body--header', tag: :div, align_items: :center) do |header_grid| header_grid.with_area(:counter, tag: :div) do - render(WorkPackages::Share::CounterComponent.new(work_package: @work_package, count: @shares.size)) + render(Shares::CounterComponent.new(work_package: @work_package, count: @shares.size)) end header_grid.with_area(:actions, @@ -72,11 +72,11 @@ data: { 'work-packages--share--bulk-selection-target': 'bulkActions' }) do if sharing_manageable? concat( - render(WorkPackages::Share::BulkPermissionButtonComponent.new(work_package: @work_package)) + render(Shares::BulkPermissionButtonComponent.new(work_package: @work_package)) ) concat( - form_with(url: work_package_shares_bulk_path(@work_package), + form_with(url: url_for([:bulk, @work_package, Member]), method: :delete, data: { 'work-packages--share--bulk-selection-target': 'bulkForm' }) do render(Primer::Beta::IconButton.new(icon: "trash", @@ -107,7 +107,7 @@ end else @shares.each do |share| - render(WorkPackages::Share::ShareRowComponent.new(share: share, container: border_box)) + render(Shares::ShareRowComponent.new(share: share, container: border_box)) end end end diff --git a/app/components/shares/modal_body_component.rb b/app/components/shares/modal_body_component.rb new file mode 100644 index 000000000000..b9b918a0a525 --- /dev/null +++ b/app/components/shares/modal_body_component.rb @@ -0,0 +1,178 @@ +#-- 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. +#++ + +module Shares + class ModalBodyComponent < ApplicationComponent + include ApplicationHelper + include MemberHelper + include OpTurbo::Streamable + include OpPrimer::ComponentHelpers + include Shares::Concerns::Authorization + include Shares::Concerns::DisplayableRoles + + def initialize(work_package:, shares:, errors: nil) + super + + @work_package = work_package + @shares = shares + @errors = errors + end + + def self.wrapper_key + "work_package_share_list" + end + + private + + def insert_target_modified? + true + end + + def insert_target_modifier_id + "op-share-wp-active-shares" + end + + def blankslate_config + @blankslate_config ||= {}.tap do |config| + if params[:filters].blank? + config[:icon] = :people + config[:heading_text] = I18n.t("work_package.sharing.text_empty_state_header") + config[:description_text] = I18n.t("work_package.sharing.text_empty_state_description") + else + config[:icon] = :search + config[:heading_text] = I18n.t("work_package.sharing.text_empty_search_header") + config[:description_text] = I18n.t("work_package.sharing.text_empty_search_description") + end + end + end + + def type_filter_options + [ + { label: I18n.t("work_package.sharing.filter.project_member"), + value: { principal_type: "User", project_member: true } }, + { label: I18n.t("work_package.sharing.filter.not_project_member"), + value: { principal_type: "User", project_member: false } }, + { label: I18n.t("work_package.sharing.filter.project_group"), + value: { principal_type: "Group", project_member: true } }, + { label: I18n.t("work_package.sharing.filter.not_project_group"), + value: { principal_type: "Group", project_member: false } } + ] + end + + def type_filter_option_active?(_option) + principal_type_filter_value = current_filter_value(params[:filters], "principal_type") + project_member_filter_value = current_filter_value(params[:filters], "also_project_member") + + return false if principal_type_filter_value.nil? || project_member_filter_value.nil? + + principal_type_checked = + _option[:value][:principal_type] == principal_type_filter_value + membership_selected = + _option[:value][:project_member] == ActiveRecord::Type::Boolean.new.cast(project_member_filter_value) + + principal_type_checked && membership_selected + end + + def role_filter_option_active?(_option) + role_filter_value = current_filter_value(params[:filters], "role_id") + + return false if role_filter_value.nil? + + find_role_ids(_option[:value]).first == role_filter_value.to_i + end + + def filter_url(type_option: nil, role_option: nil) + return url_for([@work_package, Member]) if type_option.nil? && role_option.nil? + + args = {} + filter = [] + + filter += apply_role_filter(role_option) + filter += apply_type_filter(type_option) + + args[:filters] = filter.to_json unless filter.empty? + + url_for([@work_package, Member, **args]) + end + + def apply_role_filter(_option) + current_role_filter_value = current_filter_value(params[:filters], "role_id") + filter = [] + + if _option.nil? && current_role_filter_value.present? + # When there is already a role filter set and no new value passed, we want to keep that filter + filter = role_filter_for({ value: current_role_filter_value }, builtin_role: false) + elsif _option.present? && !role_filter_option_active?(_option) + # Only when the passed filter option is not the currently selected one, we apply the filter + filter = role_filter_for(_option) + end + + filter + end + + def role_filter_for(_option, builtin_role: true) + [{ role_id: { operator: "=", values: builtin_role ? find_role_ids(_option[:value]) : [_option[:value]] } }] + end + + def apply_type_filter(_option) + current_type_filter_value = current_filter_value(params[:filters], "principal_type") + current_member_filter_value = current_filter_value(params[:filters], "also_project_member") + filter = [] + + if _option.nil? && current_type_filter_value.present? && current_member_filter_value.present? + # When there is already a type filter set and no new value passed, we want to keep that filter + value = { value: { principal_type: current_type_filter_value, project_member: current_member_filter_value } } + filter = type_filter_for(value) + elsif _option.present? && !type_filter_option_active?(_option) + # Only when the passed filter option is not the currently selected one, we apply the filter + filter = type_filter_for(_option) + end + + filter + end + + def type_filter_for(_option) + filter = [] + if ActiveRecord::Type::Boolean.new.cast(_option[:value][:project_member]) + filter.push({ also_project_member: { operator: "=", values: [OpenProject::Database::DB_VALUE_TRUE] } }) + else + filter.push({ also_project_member: { operator: "=", values: [OpenProject::Database::DB_VALUE_FALSE] } }) + end + + filter.push({ principal_type: { operator: "=", values: [_option[:value][:principal_type]] } }) + filter + end + + def current_filter_value(filters, filter_key) + return nil if filters.nil? + + given_filters = JSON.parse(filters).find { |key| key.key?(filter_key) } + given_filters ? given_filters[filter_key]["values"].first : nil + end + end +end diff --git a/app/components/work_packages/share/modal_body_component.sass b/app/components/shares/modal_body_component.sass similarity index 100% rename from app/components/work_packages/share/modal_body_component.sass rename to app/components/shares/modal_body_component.sass diff --git a/app/components/work_packages/share/modal_upsale_component.html.erb b/app/components/shares/modal_upsale_component.html.erb similarity index 100% rename from app/components/work_packages/share/modal_upsale_component.html.erb rename to app/components/shares/modal_upsale_component.html.erb diff --git a/app/components/work_packages/share/modal_upsale_component.rb b/app/components/shares/modal_upsale_component.rb similarity index 82% rename from app/components/work_packages/share/modal_upsale_component.rb rename to app/components/shares/modal_upsale_component.rb index 4a387bec3904..e2610156c88c 100644 --- a/app/components/work_packages/share/modal_upsale_component.rb +++ b/app/components/shares/modal_upsale_component.rb @@ -26,16 +26,14 @@ # See COPYRIGHT and LICENSE files for more details. #++ -module WorkPackages - module Share - class ModalUpsaleComponent < ApplicationComponent - include ApplicationHelper - include OpTurbo::Streamable - include OpPrimer::ComponentHelpers +module Shares + class ModalUpsaleComponent < ApplicationComponent + include ApplicationHelper + include OpTurbo::Streamable + include OpPrimer::ComponentHelpers - def self.wrapper_key - "work_package_share_list" - end + def self.wrapper_key + "work_package_share_list" end end end diff --git a/app/components/work_packages/share/permission_button_component.html.erb b/app/components/shares/permission_button_component.html.erb similarity index 100% rename from app/components/work_packages/share/permission_button_component.html.erb rename to app/components/shares/permission_button_component.html.erb diff --git a/app/components/shares/permission_button_component.rb b/app/components/shares/permission_button_component.rb new file mode 100644 index 000000000000..6de62dfb5538 --- /dev/null +++ b/app/components/shares/permission_button_component.rb @@ -0,0 +1,84 @@ +#-- 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. +#++ + +module Shares + class PermissionButtonComponent < ApplicationComponent + include ApplicationHelper + include OpPrimer::ComponentHelpers + include OpTurbo::Streamable + include Shares::Concerns::DisplayableRoles + + def initialize(share:, **system_arguments) + super + + @share = share + @system_arguments = system_arguments + end + + # Switches the component to either update the share directly (by sending a PATCH to the share path) + # or be passive and work like a select inside a form. + def update_path + if share.persisted? + url_for([share.entity, share]) + end + end + + def option_active?(option) + option[:value] == active_role.builtin + end + + def wrapper_uniq_by + share.id || @system_arguments.dig(:data, :"test-selector") + end + + private + + attr_reader :share + + def active_role + if share.persisted? + share.roles + .merge(MemberRole.only_non_inherited) + .first + else + share.roles.first + end + end + + def permission_name(value) + options.find { |option| option[:value] == value }[:label] + end + + def form_inputs(role_id) + [].tap do |inputs| + inputs << { name: "role_ids[]", value: role_id } + inputs << { name: "filters", value: params[:filters] } if params[:filters] + end + end + end +end diff --git a/app/components/work_packages/share/share_counter_component.html.erb b/app/components/shares/share_counter_component.html.erb similarity index 100% rename from app/components/work_packages/share/share_counter_component.html.erb rename to app/components/shares/share_counter_component.html.erb diff --git a/app/components/work_packages/share/share_counter_component.rb b/app/components/shares/share_counter_component.rb similarity index 86% rename from app/components/work_packages/share/share_counter_component.rb rename to app/components/shares/share_counter_component.rb index 0b882c56d5dc..f30bbf15bc6a 100644 --- a/app/components/work_packages/share/share_counter_component.rb +++ b/app/components/shares/share_counter_component.rb @@ -28,18 +28,16 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackages - module Share - class ShareCounterComponent < ApplicationComponent - def initialize(count:) - super +module Shares + class ShareCounterComponent < ApplicationComponent + def initialize(count:) + super - @count = count - end + @count = count + end - private + private - attr_reader :count - end + attr_reader :count end end diff --git a/app/components/work_packages/share/share_row_component.html.erb b/app/components/shares/share_row_component.html.erb similarity index 91% rename from app/components/work_packages/share/share_row_component.html.erb rename to app/components/shares/share_row_component.html.erb index 539819842104..02737d532476 100644 --- a/app/components/work_packages/share/share_row_component.html.erb +++ b/app/components/shares/share_row_component.html.erb @@ -17,12 +17,12 @@ end user_row_grid.with_area(:user_details, tag: :div, classes: 'ellipsis') do - render(WorkPackages::Share::UserDetailsComponent.new(share:, manager_mode: share_editable?)) + render(Shares::UserDetailsComponent.new(share:, manager_mode: share_editable?)) end if share_editable? user_row_grid.with_area(:button, tag: :div, color: :subtle) do - render(WorkPackages::Share::PermissionButtonComponent.new(share:, + render(Shares::PermissionButtonComponent.new(share:, data: { 'test-selector': 'op-share-wp-update-role' })) end diff --git a/app/components/work_packages/share/share_row_component.rb b/app/components/shares/share_row_component.rb similarity index 53% rename from app/components/work_packages/share/share_row_component.rb rename to app/components/shares/share_row_component.rb index 16d82a331a09..a5b376b8d0c9 100644 --- a/app/components/work_packages/share/share_row_component.rb +++ b/app/components/shares/share_row_component.rb @@ -28,53 +28,51 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackages - module Share - class ShareRowComponent < ApplicationComponent - include ApplicationHelper - include OpTurbo::Streamable - include OpPrimer::ComponentHelpers - include WorkPackages::Share::Concerns::Authorization +module Shares + class ShareRowComponent < ApplicationComponent + include ApplicationHelper + include OpTurbo::Streamable + include OpPrimer::ComponentHelpers + include Shares::Concerns::Authorization - def initialize(share:, - container: nil) - super + def initialize(share:, + container: nil) + super - @share = share - @work_package = share.entity - @principal = share.principal - @container = container - end + @share = share + @work_package = share.entity + @principal = share.principal + @container = container + end - def wrapper_uniq_by - share.id - end + def wrapper_uniq_by + share.id + end - private + private - attr_reader :share, :work_package, :principal, :container + attr_reader :share, :work_package, :principal, :container - def share_editable? - @share_editable ||= User.current != share.principal && sharing_manageable? - end + def share_editable? + @share_editable ||= User.current != share.principal && sharing_manageable? + end - def grid_css_classes - if sharing_manageable? - "op-share-wp-modal-body--user-row_manageable" - else - "op-share-wp-modal-body--user-row" - end + def grid_css_classes + if sharing_manageable? + "op-share-wp-modal-body--user-row_manageable" + else + "op-share-wp-modal-body--user-row" end + end - def select_share_checkbox_options - { - name: "share_ids", - value: share.id, - scheme: :array, - label: principal.name, - visually_hide_label: true - } - end + def select_share_checkbox_options + { + name: "share_ids", + value: share.id, + scheme: :array, + label: principal.name, + visually_hide_label: true + } end end end diff --git a/app/components/work_packages/share/user_details_component.html.erb b/app/components/shares/user_details_component.html.erb similarity index 100% rename from app/components/work_packages/share/user_details_component.html.erb rename to app/components/shares/user_details_component.html.erb diff --git a/app/components/shares/user_details_component.rb b/app/components/shares/user_details_component.rb new file mode 100644 index 000000000000..436d859e8c49 --- /dev/null +++ b/app/components/shares/user_details_component.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2023 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. +# ++ + +module Shares + class UserDetailsComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent + include OpTurbo::Streamable + include OpPrimer::ComponentHelpers + include Shares::Concerns::DisplayableRoles + + def initialize(share:, + manager_mode: User.current.allowed_in_project?(:share_work_packages, share.project), + invite_resent: false) + super + + @share = share + @user = share.principal + @manager_mode = manager_mode + @invite_resent = invite_resent + end + + private + + attr_reader :user, :share + + def manager_mode? = @manager_mode + + def invite_resent? = @invite_resent + + def wrapper_uniq_by + share.id + end + + def authoritative_work_package_role_name + @authoritative_work_package_role_name = options.find do |option| + option[:value] == share.roles.first.builtin + end[:label] + end + + def principal_show_path + case user + when User + user_path(user) + when Group + show_group_path(user) + else + placeholder_user_path(user) + end + end + + def resend_invite_path + url_for([:resend_invite, share.entity, share]) + end + + def user_is_a_group? + @user_is_a_group ||= user.is_a?(Group) + end + + def user_in_non_active_status? + user.locked? || user.invited? + end + + # Is a user member of a project no matter whether inherited or directly assigned + def project_member? + Member.exists?(project: share.project, + principal: user, + entity: nil) + end + + # Explicitly check whether the project membership was inherited by a group + def inherited_project_member? + Member.includes(:roles) + .references(:member_roles) + .where(project: share.project, principal: user, entity: nil) # membership in the project + .merge(MemberRole.only_inherited) # that was inherited + .any? + end + + def project_group? + user_is_a_group? && project_member? + end + + def part_of_a_shared_group? + share.member_roles.where.not(inherited_from: nil).any? + end + + def part_of_a_group? + GroupUser.where(user_id: user.id).any? + end + + def project_role_name + Member.where(project: share.project, + principal: user, + entity: nil) + .first + .roles + .first + .name + end + end +end diff --git a/app/components/work_packages/share/modal_body_component.rb b/app/components/work_packages/share/modal_body_component.rb deleted file mode 100644 index d3e59c2b11c2..000000000000 --- a/app/components/work_packages/share/modal_body_component.rb +++ /dev/null @@ -1,180 +0,0 @@ -#-- 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. -#++ - -module WorkPackages - module Share - class ModalBodyComponent < ApplicationComponent - include ApplicationHelper - include MemberHelper - include OpTurbo::Streamable - include OpPrimer::ComponentHelpers - include WorkPackages::Share::Concerns::Authorization - include WorkPackages::Share::Concerns::DisplayableRoles - - def initialize(work_package:, shares:, errors: nil) - super - - @work_package = work_package - @shares = shares - @errors = errors - end - - def self.wrapper_key - "work_package_share_list" - end - - private - - def insert_target_modified? - true - end - - def insert_target_modifier_id - "op-share-wp-active-shares" - end - - def blankslate_config - @blankslate_config ||= {}.tap do |config| - if params[:filters].blank? - config[:icon] = :people - config[:heading_text] = I18n.t("work_package.sharing.text_empty_state_header") - config[:description_text] = I18n.t("work_package.sharing.text_empty_state_description") - else - config[:icon] = :search - config[:heading_text] = I18n.t("work_package.sharing.text_empty_search_header") - config[:description_text] = I18n.t("work_package.sharing.text_empty_search_description") - end - end - end - - def type_filter_options - [ - { label: I18n.t("work_package.sharing.filter.project_member"), - value: { principal_type: "User", project_member: true } }, - { label: I18n.t("work_package.sharing.filter.not_project_member"), - value: { principal_type: "User", project_member: false } }, - { label: I18n.t("work_package.sharing.filter.project_group"), - value: { principal_type: "Group", project_member: true } }, - { label: I18n.t("work_package.sharing.filter.not_project_group"), - value: { principal_type: "Group", project_member: false } } - ] - end - - def type_filter_option_active?(_option) - principal_type_filter_value = current_filter_value(params[:filters], "principal_type") - project_member_filter_value = current_filter_value(params[:filters], "also_project_member") - - return false if principal_type_filter_value.nil? || project_member_filter_value.nil? - - principal_type_checked = - _option[:value][:principal_type] == principal_type_filter_value - membership_selected = - _option[:value][:project_member] == ActiveRecord::Type::Boolean.new.cast(project_member_filter_value) - - principal_type_checked && membership_selected - end - - def role_filter_option_active?(_option) - role_filter_value = current_filter_value(params[:filters], "role_id") - - return false if role_filter_value.nil? - - find_role_ids(_option[:value]).first == role_filter_value.to_i - end - - def filter_url(type_option: nil, role_option: nil) - return url_for([@work_package, Member]) if type_option.nil? && role_option.nil? - - args = {} - filter = [] - - filter += apply_role_filter(role_option) - filter += apply_type_filter(type_option) - - args[:filters] = filter.to_json unless filter.empty? - - url_for([@work_package, Member, **args]) - end - - def apply_role_filter(_option) - current_role_filter_value = current_filter_value(params[:filters], "role_id") - filter = [] - - if _option.nil? && current_role_filter_value.present? - # When there is already a role filter set and no new value passed, we want to keep that filter - filter = role_filter_for({ value: current_role_filter_value }, builtin_role: false) - elsif _option.present? && !role_filter_option_active?(_option) - # Only when the passed filter option is not the currently selected one, we apply the filter - filter = role_filter_for(_option) - end - - filter - end - - def role_filter_for(_option, builtin_role: true) - [{ role_id: { operator: "=", values: builtin_role ? find_role_ids(_option[:value]) : [_option[:value]] } }] - end - - def apply_type_filter(_option) - current_type_filter_value = current_filter_value(params[:filters], "principal_type") - current_member_filter_value = current_filter_value(params[:filters], "also_project_member") - filter = [] - - if _option.nil? && current_type_filter_value.present? && current_member_filter_value.present? - # When there is already a type filter set and no new value passed, we want to keep that filter - value = { value: { principal_type: current_type_filter_value, project_member: current_member_filter_value } } - filter = type_filter_for(value) - elsif _option.present? && !type_filter_option_active?(_option) - # Only when the passed filter option is not the currently selected one, we apply the filter - filter = type_filter_for(_option) - end - - filter - end - - def type_filter_for(_option) - filter = [] - if ActiveRecord::Type::Boolean.new.cast(_option[:value][:project_member]) - filter.push({ also_project_member: { operator: "=", values: [OpenProject::Database::DB_VALUE_TRUE] } }) - else - filter.push({ also_project_member: { operator: "=", values: [OpenProject::Database::DB_VALUE_FALSE] } }) - end - - filter.push({ principal_type: { operator: "=", values: [_option[:value][:principal_type]] } }) - filter - end - - def current_filter_value(filters, filter_key) - return nil if filters.nil? - - given_filters = JSON.parse(filters).find { |key| key.key?(filter_key) } - given_filters ? given_filters[filter_key]["values"].first : nil - end - end - end -end diff --git a/app/components/work_packages/share/permission_button_component.rb b/app/components/work_packages/share/permission_button_component.rb deleted file mode 100644 index 07c8f242f8e1..000000000000 --- a/app/components/work_packages/share/permission_button_component.rb +++ /dev/null @@ -1,86 +0,0 @@ -#-- 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. -#++ - -module WorkPackages - module Share - class PermissionButtonComponent < ApplicationComponent - include ApplicationHelper - include OpPrimer::ComponentHelpers - include OpTurbo::Streamable - include WorkPackages::Share::Concerns::DisplayableRoles - - def initialize(share:, **system_arguments) - super - - @share = share - @system_arguments = system_arguments - end - - # Switches the component to either update the share directly (by sending a PATCH to the share path) - # or be passive and work like a select inside a form. - def update_path - if share.persisted? - url_for([share.entity, share]) - end - end - - def option_active?(option) - option[:value] == active_role.builtin - end - - def wrapper_uniq_by - share.id || @system_arguments.dig(:data, :"test-selector") - end - - private - - attr_reader :share - - def active_role - if share.persisted? - share.roles - .merge(MemberRole.only_non_inherited) - .first - else - share.roles.first - end - end - - def permission_name(value) - options.find { |option| option[:value] == value }[:label] - end - - def form_inputs(role_id) - [].tap do |inputs| - inputs << { name: "role_ids[]", value: role_id } - inputs << { name: "filters", value: params[:filters] } if params[:filters] - end - end - end - end -end diff --git a/app/components/work_packages/share/user_details_component.rb b/app/components/work_packages/share/user_details_component.rb deleted file mode 100644 index 11c9b3fce510..000000000000 --- a/app/components/work_packages/share/user_details_component.rb +++ /dev/null @@ -1,131 +0,0 @@ -# frozen_string_literal: true - -# -- copyright -# OpenProject is an open source project management software. -# Copyright (C) 2023 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. -# ++ - -module WorkPackages - module Share - # rubocop:disable OpenProject/AddPreviewForViewComponent - class UserDetailsComponent < ApplicationComponent - # rubocop:enable OpenProject/AddPreviewForViewComponent - include OpTurbo::Streamable - include OpPrimer::ComponentHelpers - include WorkPackages::Share::Concerns::DisplayableRoles - - def initialize(share:, - manager_mode: User.current.allowed_in_project?(:share_work_packages, share.project), - invite_resent: false) - super - - @share = share - @user = share.principal - @manager_mode = manager_mode - @invite_resent = invite_resent - end - - private - - attr_reader :user, :share - - def manager_mode? = @manager_mode - - def invite_resent? = @invite_resent - - def wrapper_uniq_by - share.id - end - - def authoritative_work_package_role_name - @authoritative_work_package_role_name = options.find do |option| - option[:value] == share.roles.first.builtin - end[:label] - end - - def principal_show_path - case user - when User - user_path(user) - when Group - show_group_path(user) - else - placeholder_user_path(user) - end - end - - def resend_invite_path - url_for([:resend_invite, share.entity, share]) - end - - def user_is_a_group? - @user_is_a_group ||= user.is_a?(Group) - end - - def user_in_non_active_status? - user.locked? || user.invited? - end - - # Is a user member of a project no matter whether inherited or directly assigned - def project_member? - Member.exists?(project: share.project, - principal: user, - entity: nil) - end - - # Explicitly check whether the project membership was inherited by a group - def inherited_project_member? - Member.includes(:roles) - .references(:member_roles) - .where(project: share.project, principal: user, entity: nil) # membership in the project - .merge(MemberRole.only_inherited) # that was inherited - .any? - end - - def project_group? - user_is_a_group? && project_member? - end - - def part_of_a_shared_group? - share.member_roles.where.not(inherited_from: nil).any? - end - - def part_of_a_group? - GroupUser.where(user_id: user.id).any? - end - - def project_role_name - Member.where(project: share.project, - principal: user, - entity: nil) - .first - .roles - .first - .name - end - end - end -end diff --git a/app/controllers/shares/bulk_controller.rb b/app/controllers/shares/bulk_controller.rb index 0ec310ebbb4c..9dbf7fe6c4e4 100644 --- a/app/controllers/shares/bulk_controller.rb +++ b/app/controllers/shares/bulk_controller.rb @@ -69,8 +69,8 @@ def destroy def respond_with_update_permission_buttons @selected_shares.each do |share| replace_via_turbo_stream( - component: WorkPackages::Share::PermissionButtonComponent.new(share:, - data: { "test-selector": "op-share-wp-update-role" }) + component: Shares::PermissionButtonComponent.new(share:, + data: { "test-selector": "op-share-wp-update-role" }) ) end @@ -79,7 +79,7 @@ def respond_with_update_permission_buttons def respond_with_replace_modal replace_via_turbo_stream( - component: WorkPackages::Share::ModalBodyComponent.new(work_package: @work_package, shares: find_shares) + component: Shares::ModalBodyComponent.new(work_package: @work_package, shares: find_shares) ) respond_with_turbo_streams @@ -88,12 +88,12 @@ def respond_with_replace_modal def respond_with_remove_shares @selected_shares.each do |share| remove_via_turbo_stream( - component: WorkPackages::Share::ShareRowComponent.new(share:) + component: Shares::ShareRowComponent.new(share:) ) end update_via_turbo_stream( - component: WorkPackages::Share::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) + component: Shares::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) ) respond_with_turbo_streams diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 0d67150d709d..ddfcb285d653 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -45,7 +45,7 @@ def index @shares = load_shares query - render WorkPackages::Share::ModalBodyComponent.new(work_package: @work_package, shares: @shares, errors: @errors), layout: nil + render Shares::ModalBodyComponent.new(work_package: @work_package, shares: @shares, errors: @errors), layout: nil end def create @@ -123,12 +123,12 @@ def resend_invite def enterprise_check return if EnterpriseToken.allows_to?(:work_package_sharing) - render WorkPackages::Share::ModalUpsaleComponent.new + render Shares::ModalUpsaleComponent.new end def respond_with_replace_modal replace_via_turbo_stream( - component: WorkPackages::Share::ModalBodyComponent.new(work_package: @work_package, + component: Shares::ModalBodyComponent.new(work_package: @work_package, shares: @new_shares || find_shares, errors: @errors) ) @@ -138,17 +138,17 @@ def respond_with_replace_modal def respond_with_prepend_shares replace_via_turbo_stream( - component: WorkPackages::Share::InviteUserFormComponent.new(work_package: @work_package, errors: @errors) + component: Shares::InviteUserFormComponent.new(work_package: @work_package, errors: @errors) ) update_via_turbo_stream( - component: WorkPackages::Share::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) + component: Shares::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) ) @new_shares.each do |share| prepend_via_turbo_stream( - component: WorkPackages::Share::ShareRowComponent.new(share:), - target_component: WorkPackages::Share::ModalBodyComponent.new(work_package: @work_package, + component: Shares::ShareRowComponent.new(share:), + target_component: Shares::ModalBodyComponent.new(work_package: @work_package, shares: find_shares, errors: @errors) ) @@ -159,7 +159,7 @@ def respond_with_prepend_shares def respond_with_new_invite_form replace_via_turbo_stream( - component: WorkPackages::Share::InviteUserFormComponent.new(work_package: @work_package, errors: @errors) + component: Shares::InviteUserFormComponent.new(work_package: @work_package, errors: @errors) ) respond_with_turbo_streams @@ -167,7 +167,7 @@ def respond_with_new_invite_form def respond_with_update_permission_button replace_via_turbo_stream( - component: WorkPackages::Share::PermissionButtonComponent.new(share: @share, + component: Shares::PermissionButtonComponent.new(share: @share, data: { "test-selector": "op-share-wp-update-role" }) ) @@ -176,11 +176,11 @@ def respond_with_update_permission_button def respond_with_remove_share remove_via_turbo_stream( - component: WorkPackages::Share::ShareRowComponent.new(share: @share) + component: Shares::ShareRowComponent.new(share: @share) ) update_via_turbo_stream( - component: WorkPackages::Share::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) + component: Shares::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) ) respond_with_turbo_streams @@ -188,7 +188,7 @@ def respond_with_remove_share def respond_with_update_user_details update_via_turbo_stream( - component: WorkPackages::Share::UserDetailsComponent.new(share: @share, + component: Shares::UserDetailsComponent.new(share: @share, invite_resent: true) ) diff --git a/app/forms/work_packages/share/invitee.rb b/app/forms/shares/invitee.rb similarity index 98% rename from app/forms/work_packages/share/invitee.rb rename to app/forms/shares/invitee.rb index de2d34f73691..4dd542a470c9 100644 --- a/app/forms/work_packages/share/invitee.rb +++ b/app/forms/shares/invitee.rb @@ -25,7 +25,7 @@ # # See COPYRIGHT and LICENSE files for more details. #++ -module WorkPackages::Share +module Shares class Invitee < ApplicationForm form do |user_invite_form| user_invite_form.autocompleter( diff --git a/spec/components/work_packages/share/user_details_component_spec.rb b/spec/components/shares/user_details_component_spec.rb similarity index 99% rename from spec/components/work_packages/share/user_details_component_spec.rb rename to spec/components/shares/user_details_component_spec.rb index aeb385aa6ca1..5eefdb309e62 100644 --- a/spec/components/work_packages/share/user_details_component_spec.rb +++ b/spec/components/shares/user_details_component_spec.rb @@ -29,7 +29,7 @@ # ++ require "spec_helper" -RSpec.describe WorkPackages::Share::UserDetailsComponent, type: :component do +RSpec.describe Shares::UserDetailsComponent, type: :component do subject { render_inline(described_class.new(share:, manager_mode:, invite_resent:)) } shared_let(:project) { create(:project) } From abcbf57657112e6b1ab88a12fb386d32fdb709e5 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Thu, 20 Jun 2024 12:53:11 +0200 Subject: [PATCH 08/33] `WorkPackageMemberQuery` -> `EntityMemberQuery` --- app/controllers/shares_controller.rb | 2 +- app/models/queries/members.rb | 2 +- .../{work_package_member_query.rb => entity_member_query.rb} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename app/models/queries/members/{work_package_member_query.rb => entity_member_query.rb} (94%) diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index ddfcb285d653..9fb8cf22d349 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -218,7 +218,7 @@ def current_visible_member_count def load_query @query = ParamsToQueryService.new(Member, current_user, - query_class: Queries::Members::WorkPackageMemberQuery) + query_class: Queries::Members::EntityMemberQuery) .call(params) # Set default filter on the entity diff --git a/app/models/queries/members.rb b/app/models/queries/members.rb index 8f55951d4dc3..98228d61265e 100644 --- a/app/models/queries/members.rb +++ b/app/models/queries/members.rb @@ -50,7 +50,7 @@ module Queries::Members order Orders::StatusOrder end - ::Queries::Register.register(WorkPackageMemberQuery) do + ::Queries::Register.register(EntityMemberQuery) do filter Filters::NameFilter filter Filters::AnyNameAttributeFilter filter Filters::ProjectFilter diff --git a/app/models/queries/members/work_package_member_query.rb b/app/models/queries/members/entity_member_query.rb similarity index 94% rename from app/models/queries/members/work_package_member_query.rb rename to app/models/queries/members/entity_member_query.rb index 36436a38a6f3..27714076f0e1 100644 --- a/app/models/queries/members/work_package_member_query.rb +++ b/app/models/queries/members/entity_member_query.rb @@ -28,7 +28,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -class Queries::Members::WorkPackageMemberQuery < Queries::Members::MemberQuery +class Queries::Members::EntityMemberQuery < Queries::Members::MemberQuery def default_scope Member.joins(:member_roles).merge(MemberRole.only_non_inherited) end From 95910211c920b34a097bf9bdca970becb09aa3be Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Thu, 20 Jun 2024 13:08:29 +0200 Subject: [PATCH 09/33] Remove work package specific loading in SharesController and use entity --- app/controllers/shares_controller.rb | 105 +++++++++++++-------------- 1 file changed, 49 insertions(+), 56 deletions(-) diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 9fb8cf22d349..5459d374bdda 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -30,22 +30,18 @@ class SharesController < ApplicationController include OpTurbo::ComponentStream include MemberHelper - before_action :find_work_package, only: %i[index create destroy update resend_invite] - before_action :find_share, only: %i[destroy update resend_invite] - before_action :find_project + before_action :load_entity + before_action :load_shares, only: %i[index] + before_action :load_share, only: %i[destroy update resend_invite] before_action :authorize before_action :enterprise_check, only: %i[index] def index - query = load_query - - unless query.valid? + unless @query.valid? flash.now[:error] = query.errors.full_messages end - @shares = load_shares query - - render Shares::ModalBodyComponent.new(work_package: @work_package, shares: @shares, errors: @errors), layout: nil + render Shares::ModalBodyComponent.new(work_package: @entity, shares: @shares, errors: @errors), layout: nil end def create @@ -59,7 +55,7 @@ def create else service_call = WorkPackageMembers::CreateOrUpdateService .new(user: current_user) - .call(entity: @work_package, + .call(entity: @entity, user_id: member_params[:user_id], role_ids: find_role_ids(params[:member][:role_id])) @@ -87,7 +83,7 @@ def update .new(user: current_user, model: @share) .call(role_ids: find_role_ids(params[:role_ids])) - find_shares + load_shares if @shares.empty? respond_with_replace_modal @@ -128,9 +124,9 @@ def enterprise_check def respond_with_replace_modal replace_via_turbo_stream( - component: Shares::ModalBodyComponent.new(work_package: @work_package, - shares: @new_shares || find_shares, - errors: @errors) + component: Shares::ModalBodyComponent.new(work_package: @entity, + shares: @new_shares || load_shares, + errors: @errors) ) respond_with_turbo_streams @@ -138,19 +134,19 @@ def respond_with_replace_modal def respond_with_prepend_shares replace_via_turbo_stream( - component: Shares::InviteUserFormComponent.new(work_package: @work_package, errors: @errors) + component: Shares::InviteUserFormComponent.new(work_package: @entity, errors: @errors) ) update_via_turbo_stream( - component: Shares::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) + component: Shares::CounterComponent.new(work_package: @entity, count: current_visible_member_count) ) @new_shares.each do |share| prepend_via_turbo_stream( component: Shares::ShareRowComponent.new(share:), - target_component: Shares::ModalBodyComponent.new(work_package: @work_package, - shares: find_shares, - errors: @errors) + target_component: Shares::ModalBodyComponent.new(work_package: @entity, + shares: load_shares, + errors: @errors) ) end @@ -158,81 +154,78 @@ def respond_with_prepend_shares end def respond_with_new_invite_form - replace_via_turbo_stream( - component: Shares::InviteUserFormComponent.new(work_package: @work_package, errors: @errors) - ) + replace_via_turbo_stream(component: Shares::InviteUserFormComponent.new(work_package: @entity, errors: @errors)) respond_with_turbo_streams end def respond_with_update_permission_button - replace_via_turbo_stream( - component: Shares::PermissionButtonComponent.new(share: @share, - data: { "test-selector": "op-share-wp-update-role" }) - ) + replace_via_turbo_stream(component: Shares::PermissionButtonComponent.new(share: @share, + data: { "test-selector": "op-share-wp-update-role" })) respond_with_turbo_streams end def respond_with_remove_share - remove_via_turbo_stream( - component: Shares::ShareRowComponent.new(share: @share) - ) - - update_via_turbo_stream( - component: Shares::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) - ) + remove_via_turbo_stream(component: Shares::ShareRowComponent.new(share: @share)) + update_via_turbo_stream(component: Shares::CounterComponent.new(work_package: @entity, count: current_visible_member_count)) respond_with_turbo_streams end def respond_with_update_user_details - update_via_turbo_stream( - component: Shares::UserDetailsComponent.new(share: @share, - invite_resent: true) - ) + update_via_turbo_stream(component: Shares::UserDetailsComponent.new(share: @share, invite_resent: true)) respond_with_turbo_streams end - def find_work_package - @work_package = WorkPackage.find(params[:work_package_id]) - end - - def find_share - @share = @work_package.members.find(params[:id]) - end - - def find_shares - @shares = load_shares(load_query) + def load_entity + puts "*" * 100, "Loading Entity", params.to_unsafe_h, "*" * 100 + @entity = if params["work_package_id"] + WorkPackage.visible.find(params["work_package_id"]) + # TODO: Add support for other entities + else + raise ArgumentError, <<~ERROR + Nested the SharesController under an entity controller that is not yet configured to support sharing. + Edit the SharesController#load_entity method to load the entity from the correct parent. + ERROR + end + + puts "**** Entity: #{@entity} ****" + if @entity.respond_to?(:project) + @project = @entity.project + end end - def find_project - @project = @work_package.project + def load_share + @share = @entity.members.find(params[:id]) end def current_visible_member_count - @current_visible_member_count ||= load_shares(load_query).size + @current_visible_member_count ||= load_shares.size end def load_query + return @query if defined?(@query) + @query = ParamsToQueryService.new(Member, current_user, query_class: Queries::Members::EntityMemberQuery) .call(params) # Set default filter on the entity - @query.where("entity_id", "=", @work_package.id) - @query.where("entity_type", "=", WorkPackage.name) - @query.where("project_id", "=", @project.id) + @query.where("entity_id", "=", @entity.id) + @query.where("entity_type", "=", @entity.class.name) + if @project + @query.where("project_id", "=", @project.id) + end @query.order(name: :asc) unless params[:sortBy] @query end - def load_shares(query) - query - .results + def load_shares + @shares = load_query.results end end From cbb2fefb21af65723b70913be37c93f36e7b0aa6 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Thu, 20 Jun 2024 16:46:45 +0200 Subject: [PATCH 10/33] Introduce an `OfEntity` scope --- app/models/member.rb | 1 + app/models/members/scopes/of_entity.rb | 41 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 app/models/members/scopes/of_entity.rb diff --git a/app/models/member.rb b/app/models/member.rb index a651c46bc14a..0419e68cb3f4 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -56,6 +56,7 @@ class Member < ApplicationRecord :of_any_project, :of_work_package, :of_any_work_package, + :of_entity, :of_any_entity, :of_anything_in_project, :visible, diff --git a/app/models/members/scopes/of_entity.rb b/app/models/members/scopes/of_entity.rb new file mode 100644 index 000000000000..b9178a060458 --- /dev/null +++ b/app/models/members/scopes/of_entity.rb @@ -0,0 +1,41 @@ +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2010-2023 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. +# ++ + +module Members::Scopes + module OfEntity + extend ActiveSupport::Concern + + class_methods do + # Find all members of a specific Work Package + def of_entity(entity) + of_any_entity + .where(entity:) + end + end + end +end From 2c64cef0308f377acf4221f1eb863d6c1dc144c0 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Thu, 20 Jun 2024 16:48:20 +0200 Subject: [PATCH 11/33] Add `available_roles` and move everything to `entity` --- .../bulk_permission_button_component.html.erb | 12 ++-- .../bulk_permission_button_component.rb | 11 ++-- .../bulk_selection_counter_component.rb | 2 +- .../shares/concerns/authorization.rb | 3 +- .../shares/concerns/displayable_roles.rb | 49 --------------- app/components/shares/counter_component.rb | 8 +-- .../invite_user_form_component.html.erb | 3 +- .../shares/invite_user_form_component.rb | 15 +++-- .../shares/modal_body_component.html.erb | 18 +++--- app/components/shares/modal_body_component.rb | 62 ++++++++++++------- .../permission_button_component.html.erb | 20 +++--- .../shares/permission_button_component.rb | 14 ++--- .../shares/share_counter_component.rb | 2 +- .../shares/share_row_component.html.erb | 5 +- app/components/shares/share_row_component.rb | 8 ++- .../shares/user_details_component.rb | 3 +- app/controllers/concerns/member_helper.rb | 6 -- app/controllers/shares_controller.rb | 53 ++++++++++++---- 18 files changed, 150 insertions(+), 144 deletions(-) delete mode 100644 app/components/shares/concerns/displayable_roles.rb diff --git a/app/components/shares/bulk_permission_button_component.html.erb b/app/components/shares/bulk_permission_button_component.html.erb index b1b0a5173f4f..b0eacc5f312f 100644 --- a/app/components/shares/bulk_permission_button_component.html.erb +++ b/app/components/shares/bulk_permission_button_component.html.erb @@ -9,20 +9,20 @@ 'Placeholder' end - options.each do |option| - menu.with_item(label: option[:label], + @available_roles.each do |role_hash| + menu.with_item(label: role_hash[:label], href: update_path, method: :patch, active: false, form_arguments: { method: :patch, name: 'role_ids[]', - value: option[:value], + value: role_hash[:value], data: { 'work-packages--share--bulk-selection-target': 'bulkForm bulkUpdateRoleForm', - 'role-name': option[:label], - 'test-selector': "op-share-wp-bulk-update-role-permission-#{option[:label]}" } + 'role-name': role_hash[:label], + 'test-selector': "op-share-wp-bulk-update-role-permission-#{role_hash[:label]}" } }) do |item| - item.with_description.with_content(option[:description]) + item.with_description.with_content(role_hash[:description]) end end end diff --git a/app/components/shares/bulk_permission_button_component.rb b/app/components/shares/bulk_permission_button_component.rb index ce6679a72be6..f9a89e186737 100644 --- a/app/components/shares/bulk_permission_button_component.rb +++ b/app/components/shares/bulk_permission_button_component.rb @@ -29,17 +29,16 @@ # ++ module Shares - class BulkPermissionButtonComponent < ApplicationComponent - include Shares::Concerns::DisplayableRoles - - def initialize(work_package:) + class BulkPermissionButtonComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent + def initialize(entity:, available_roles:) super - @work_package = work_package + @entity = entity + @available_roles = available_roles end def update_path - url_for([:bulk, @work_package, Member]) + url_for([:bulk, @entity, Member]) end end end diff --git a/app/components/shares/bulk_selection_counter_component.rb b/app/components/shares/bulk_selection_counter_component.rb index 2196cc6dec48..fc56f453cdeb 100644 --- a/app/components/shares/bulk_selection_counter_component.rb +++ b/app/components/shares/bulk_selection_counter_component.rb @@ -29,7 +29,7 @@ # ++ module Shares - class BulkSelectionCounterComponent < ApplicationComponent + class BulkSelectionCounterComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent def initialize(count:) super diff --git a/app/components/shares/concerns/authorization.rb b/app/components/shares/concerns/authorization.rb index 862e07166a72..6c5041866a9d 100644 --- a/app/components/shares/concerns/authorization.rb +++ b/app/components/shares/concerns/authorization.rb @@ -35,7 +35,8 @@ module Authorization included do def sharing_manageable? - User.current.allowed_in_project?(:share_work_packages, @work_package.project) + # TODO: Fix this to check based on the entity + User.current.allowed_in_project?(:share_work_packages, @entity.project) end end end diff --git a/app/components/shares/concerns/displayable_roles.rb b/app/components/shares/concerns/displayable_roles.rb deleted file mode 100644 index 79b887a48b4c..000000000000 --- a/app/components/shares/concerns/displayable_roles.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -# -- copyright -# OpenProject is an open source project management software. -# Copyright (C) 2023 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. -# ++ - -module Shares - module Concerns - module DisplayableRoles - def options - [ - { label: I18n.t("work_package.sharing.permissions.edit"), - value: Role::BUILTIN_WORK_PACKAGE_EDITOR, - description: I18n.t("work_package.sharing.permissions.edit_description") }, - { label: I18n.t("work_package.sharing.permissions.comment"), - value: Role::BUILTIN_WORK_PACKAGE_COMMENTER, - description: I18n.t("work_package.sharing.permissions.comment_description") }, - { label: I18n.t("work_package.sharing.permissions.view"), - value: Role::BUILTIN_WORK_PACKAGE_VIEWER, - description: I18n.t("work_package.sharing.permissions.view_description") } - ] - end - end - end -end diff --git a/app/components/shares/counter_component.rb b/app/components/shares/counter_component.rb index 8da6c7f9cc16..5b22c7fa2b0f 100644 --- a/app/components/shares/counter_component.rb +++ b/app/components/shares/counter_component.rb @@ -35,19 +35,19 @@ class CounterComponent < ApplicationComponent include OpPrimer::ComponentHelpers include Shares::Concerns::Authorization - def initialize(work_package:, count:) + def initialize(entity:, count:) super - @work_package = work_package + @entity = entity @count = count end private - attr_reader :work_package, :count + attr_reader :entity, :count def shared_with_anyone_else_other_than_myself? - Member.of_work_package(@work_package) + Member.of_entity(@entity) .where.not(principal: User.current) .any? end diff --git a/app/components/shares/invite_user_form_component.html.erb b/app/components/shares/invite_user_form_component.html.erb index 8a62c75fa4f6..963065e9b2b7 100644 --- a/app/components/shares/invite_user_form_component.html.erb +++ b/app/components/shares/invite_user_form_component.html.erb @@ -3,7 +3,7 @@ if sharing_manageable? primer_form_with( model: new_share, - url: url_for([@work_package, Member]), + url: url_for([@entity, Member]), data: { controller: 'user-limit ' \ 'work-packages--share--user-selected', 'application-target': 'dynamic', @@ -19,6 +19,7 @@ invite_form.with_area('permission') do render(Shares::PermissionButtonComponent.new( share: new_share, + available_roles: @available_roles, form_arguments: { builder: form, name: "role_id" }, data: { 'test-selector': 'op-share-wp-invite-role' }) ) diff --git a/app/components/shares/invite_user_form_component.rb b/app/components/shares/invite_user_form_component.rb index 1189f8b57624..abaca7db0689 100644 --- a/app/components/shares/invite_user_form_component.rb +++ b/app/components/shares/invite_user_form_component.rb @@ -27,21 +27,28 @@ #++ module Shares - class InviteUserFormComponent < ApplicationComponent + class InviteUserFormComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include ApplicationHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers include Shares::Concerns::Authorization - def initialize(work_package:, errors: nil) + def initialize(entity:, available_roles:, errors: nil) super - @work_package = work_package + @entity = entity @errors = errors + @available_roles = available_roles end def new_share - @new_share ||= Member.new(entity: @work_package, roles: [Role.new(builtin: Role::BUILTIN_WORK_PACKAGE_VIEWER)]) + @new_share ||= Member.new(entity: @entity, roles: [Role.new(id: default_role[:value])]) + end + + private + + def default_role + @available_roles.find { |role_hash| role_hash[:default] } || @available_roles.first end end end diff --git a/app/components/shares/modal_body_component.html.erb b/app/components/shares/modal_body_component.html.erb index 156ca7b249a3..3e8d3ad558c9 100644 --- a/app/components/shares/modal_body_component.html.erb +++ b/app/components/shares/modal_body_component.html.erb @@ -2,7 +2,7 @@ component_wrapper(tag: 'turbo-frame') do flex_layout(data: { turbo: true }) do |modal_content| modal_content.with_row do - render(Shares::InviteUserFormComponent.new(work_package: @work_package, errors: @errors)) + render(Shares::InviteUserFormComponent.new(entity: @entity, available_roles: @available_roles, errors: @errors)) end modal_content.with_row(mt: 3, @@ -13,7 +13,7 @@ border_box.with_header(color: :muted, data: { 'test-selector': 'op-share-wp-header' }) do grid_layout('op-share-wp-modal-body--header', tag: :div, align_items: :center) do |header_grid| header_grid.with_area(:counter, tag: :div) do - render(Shares::CounterComponent.new(work_package: @work_package, count: @shares.size)) + render(Shares::CounterComponent.new(entity: @entity, count: @shares.size)) end header_grid.with_area(:actions, @@ -53,12 +53,12 @@ button.with_trailing_action_icon(icon: "triangle-down") I18n.t('work_package.sharing.filter.role') end - options.each do |option| - menu.with_item(label: option[:label], - href: filter_url(role_option: option), + @available_roles.each do |role_hash| + menu.with_item(label: role_hash[:label], + href: filter_url(role_option: role_hash), method: :get, tag: :a, - active: role_filter_option_active?(option), + active: role_filter_option_active?(role_hash), role: "menuitem") end end @@ -72,11 +72,11 @@ data: { 'work-packages--share--bulk-selection-target': 'bulkActions' }) do if sharing_manageable? concat( - render(Shares::BulkPermissionButtonComponent.new(work_package: @work_package)) + render(Shares::BulkPermissionButtonComponent.new(entity: @entity, available_roles: @available_roles)) ) concat( - form_with(url: url_for([:bulk, @work_package, Member]), + form_with(url: url_for([:bulk, @entity, Member]), method: :delete, data: { 'work-packages--share--bulk-selection-target': 'bulkForm' }) do render(Primer::Beta::IconButton.new(icon: "trash", @@ -107,7 +107,7 @@ end else @shares.each do |share| - render(Shares::ShareRowComponent.new(share: share, container: border_box)) + render(Shares::ShareRowComponent.new(share: share, available_roles: @available_roles, container: border_box)) end end end diff --git a/app/components/shares/modal_body_component.rb b/app/components/shares/modal_body_component.rb index b9b918a0a525..0df017788b0e 100644 --- a/app/components/shares/modal_body_component.rb +++ b/app/components/shares/modal_body_component.rb @@ -27,20 +27,22 @@ #++ module Shares - class ModalBodyComponent < ApplicationComponent + class ModalBodyComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include ApplicationHelper include MemberHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers include Shares::Concerns::Authorization - include Shares::Concerns::DisplayableRoles - def initialize(work_package:, shares:, errors: nil) + attr_reader :entity, :shares, :errors, :available_roles + + def initialize(entity:, shares:, available_roles:, errors: nil) super - @work_package = work_package + @entity = entity @shares = shares @errors = errors + @available_roles = available_roles end def self.wrapper_key @@ -49,6 +51,10 @@ def self.wrapper_key private + def project_scoped_entity? + entity.respond_to?(:project) + end + def insert_target_modified? true end @@ -72,16 +78,24 @@ def blankslate_config end def type_filter_options - [ - { label: I18n.t("work_package.sharing.filter.project_member"), - value: { principal_type: "User", project_member: true } }, - { label: I18n.t("work_package.sharing.filter.not_project_member"), - value: { principal_type: "User", project_member: false } }, - { label: I18n.t("work_package.sharing.filter.project_group"), - value: { principal_type: "Group", project_member: true } }, - { label: I18n.t("work_package.sharing.filter.not_project_group"), - value: { principal_type: "Group", project_member: false } } - ] + if project_scoped_entity? + [ + { label: I18n.t("work_package.sharing.filter.project_member"), + value: { principal_type: "User", project_member: true } }, + { label: I18n.t("work_package.sharing.filter.not_project_member"), + value: { principal_type: "User", project_member: false } }, + { label: I18n.t("work_package.sharing.filter.project_group"), + value: { principal_type: "Group", project_member: true } }, + { label: I18n.t("work_package.sharing.filter.not_project_group"), + value: { principal_type: "Group", project_member: false } } + ] + else + [ + { label: "User", value: { principal_type: "User" } }, + { label: "Group", value: { principal_type: "Group" } } + ] + + end end def type_filter_option_active?(_option) @@ -107,7 +121,7 @@ def role_filter_option_active?(_option) end def filter_url(type_option: nil, role_option: nil) - return url_for([@work_package, Member]) if type_option.nil? && role_option.nil? + return url_for([@entity, Member]) if type_option.nil? && role_option.nil? args = {} filter = [] @@ -117,26 +131,28 @@ def filter_url(type_option: nil, role_option: nil) args[:filters] = filter.to_json unless filter.empty? - url_for([@work_package, Member, **args]) + url_for([@entity, Member, args]) end - def apply_role_filter(_option) + def apply_role_filter(option) current_role_filter_value = current_filter_value(params[:filters], "role_id") filter = [] - if _option.nil? && current_role_filter_value.present? + if option.nil? && current_role_filter_value.present? # When there is already a role filter set and no new value passed, we want to keep that filter - filter = role_filter_for({ value: current_role_filter_value }, builtin_role: false) - elsif _option.present? && !role_filter_option_active?(_option) + filter = role_filter_for({ value: current_role_filter_value }) + elsif option.present? && !role_filter_option_active?(option) # Only when the passed filter option is not the currently selected one, we apply the filter - filter = role_filter_for(_option) + filter = role_filter_for(option) end filter end - def role_filter_for(_option, builtin_role: true) - [{ role_id: { operator: "=", values: builtin_role ? find_role_ids(_option[:value]) : [_option[:value]] } }] + def role_filter_for(option) + [ + { role_id: { operator: "=", values: [option[:value]] } } + ] end def apply_type_filter(_option) diff --git a/app/components/shares/permission_button_component.html.erb b/app/components/shares/permission_button_component.html.erb index 076976c73e1c..f26107091665 100644 --- a/app/components/shares/permission_button_component.html.erb +++ b/app/components/shares/permission_button_component.html.erb @@ -6,21 +6,25 @@ color: :subtle }.deep_merge(@system_arguments))) do |menu| menu.with_show_button(data: { 'work-packages--share--bulk-selection-target': 'userRowRole', 'share-id': share.id, - 'active-role-name': permission_name(active_role.builtin)}) do |button| + 'active-role-name': permission_name(active_role.id)}) do |button| button.with_trailing_action_icon(icon: :"triangle-down") - permission_name(active_role.builtin) + permission_name(active_role.id) end - options.each do |option| - menu.with_item(label: option[:label], + + puts "*"*100, @available_roles, "*"*100 + + @available_roles.each do |role_hash| + puts "### role_hash: #{role_hash} ###" + menu.with_item(label: role_hash[:label], href: update_path, method: :patch, - active: option_active?(option), - data: { value: option[:value] }, + active: role_active?(role_hash), + data: { value: role_hash[:value] }, form_arguments: { method: :patch, - inputs: form_inputs(option[:value]) + inputs: form_inputs(role_hash[:value]) }) do |item| - item.with_description.with_content(option[:description]) + item.with_description.with_content(role_hash[:description]) end end end diff --git a/app/components/shares/permission_button_component.rb b/app/components/shares/permission_button_component.rb index 6de62dfb5538..2746241a1570 100644 --- a/app/components/shares/permission_button_component.rb +++ b/app/components/shares/permission_button_component.rb @@ -27,15 +27,15 @@ #++ module Shares - class PermissionButtonComponent < ApplicationComponent + class PermissionButtonComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include ApplicationHelper include OpPrimer::ComponentHelpers include OpTurbo::Streamable - include Shares::Concerns::DisplayableRoles - def initialize(share:, **system_arguments) + def initialize(share:, available_roles:, **system_arguments) super + @available_roles = available_roles @share = share @system_arguments = system_arguments end @@ -48,8 +48,8 @@ def update_path end end - def option_active?(option) - option[:value] == active_role.builtin + def role_active?(role_hash) + role_hash[:value] == active_role.id end def wrapper_uniq_by @@ -58,7 +58,7 @@ def wrapper_uniq_by private - attr_reader :share + attr_reader :share, :available_roles def active_role if share.persisted? @@ -71,7 +71,7 @@ def active_role end def permission_name(value) - options.find { |option| option[:value] == value }[:label] + available_roles.find { |role_hash| role_hash[:value] == value }[:label] end def form_inputs(role_id) diff --git a/app/components/shares/share_counter_component.rb b/app/components/shares/share_counter_component.rb index f30bbf15bc6a..094b0c74dc12 100644 --- a/app/components/shares/share_counter_component.rb +++ b/app/components/shares/share_counter_component.rb @@ -29,7 +29,7 @@ # ++ module Shares - class ShareCounterComponent < ApplicationComponent + class ShareCounterComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent def initialize(count:) super diff --git a/app/components/shares/share_row_component.html.erb b/app/components/shares/share_row_component.html.erb index 02737d532476..cb3eaba15cbe 100644 --- a/app/components/shares/share_row_component.html.erb +++ b/app/components/shares/share_row_component.html.erb @@ -23,11 +23,12 @@ if share_editable? user_row_grid.with_area(:button, tag: :div, color: :subtle) do render(Shares::PermissionButtonComponent.new(share:, - data: { 'test-selector': 'op-share-wp-update-role' })) + available_roles: @available_roles, + data: { 'test-selector': 'op-share-wp-update-role' })) end user_row_grid.with_area(:remove, tag: :div) do - form_with url: url_for([work_package, share]), method: :delete do + form_with url: url_for([entity, share]), method: :delete do render(Primer::Beta::IconButton.new(icon: "trash", type: :submit, scheme: :danger, diff --git a/app/components/shares/share_row_component.rb b/app/components/shares/share_row_component.rb index a5b376b8d0c9..bb981e146ef4 100644 --- a/app/components/shares/share_row_component.rb +++ b/app/components/shares/share_row_component.rb @@ -29,19 +29,21 @@ # ++ module Shares - class ShareRowComponent < ApplicationComponent + class ShareRowComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include ApplicationHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers include Shares::Concerns::Authorization def initialize(share:, + available_roles:, container: nil) super @share = share - @work_package = share.entity + @entity = share.entity @principal = share.principal + @available_roles = available_roles @container = container end @@ -51,7 +53,7 @@ def wrapper_uniq_by private - attr_reader :share, :work_package, :principal, :container + attr_reader :share, :entity, :principal, :container, :available_roles def share_editable? @share_editable ||= User.current != share.principal && sharing_manageable? diff --git a/app/components/shares/user_details_component.rb b/app/components/shares/user_details_component.rb index 436d859e8c49..0003900998a1 100644 --- a/app/components/shares/user_details_component.rb +++ b/app/components/shares/user_details_component.rb @@ -32,7 +32,6 @@ module Shares class UserDetailsComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include OpTurbo::Streamable include OpPrimer::ComponentHelpers - include Shares::Concerns::DisplayableRoles def initialize(share:, manager_mode: User.current.allowed_in_project?(:share_work_packages, share.project), @@ -59,7 +58,7 @@ def wrapper_uniq_by def authoritative_work_package_role_name @authoritative_work_package_role_name = options.find do |option| - option[:value] == share.roles.first.builtin + option[:value] == share.roles.first.id end[:label] end diff --git a/app/controllers/concerns/member_helper.rb b/app/controllers/concerns/member_helper.rb index 70c7a09a5561..8eb65831fd92 100644 --- a/app/controllers/concerns/member_helper.rb +++ b/app/controllers/concerns/member_helper.rb @@ -29,12 +29,6 @@ module MemberHelper module_function - def find_role_ids(builtin_value) - # Role has a left join on permissions included leading to multiple ids being returned which - # is why we unscope. - WorkPackageRole.unscoped.where(builtin: builtin_value).pluck(:id) - end - def find_or_create_users(send_notification: true) @send_notification = send_notification diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 5459d374bdda..6464a16798f4 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -41,7 +41,7 @@ def index flash.now[:error] = query.errors.full_messages end - render Shares::ModalBodyComponent.new(work_package: @entity, shares: @shares, errors: @errors), layout: nil + render Shares::ModalBodyComponent.new(entity: @entity, shares: @shares, errors: @errors, available_roles:), layout: nil end def create @@ -57,7 +57,7 @@ def create .new(user: current_user) .call(entity: @entity, user_id: member_params[:user_id], - role_ids: find_role_ids(params[:member][:role_id])) + role_ids: [params[:member][:role_id]]) overall_result.push(service_call) end @@ -81,7 +81,7 @@ def create def update WorkPackageMembers::UpdateService .new(user: current_user, model: @share) - .call(role_ids: find_role_ids(params[:role_ids])) + .call(role_ids: params[:role_ids]) load_shares @@ -124,7 +124,8 @@ def enterprise_check def respond_with_replace_modal replace_via_turbo_stream( - component: Shares::ModalBodyComponent.new(work_package: @entity, + component: Shares::ModalBodyComponent.new(entity: @entity, + available_roles:, shares: @new_shares || load_shares, errors: @errors) ) @@ -134,17 +135,18 @@ def respond_with_replace_modal def respond_with_prepend_shares replace_via_turbo_stream( - component: Shares::InviteUserFormComponent.new(work_package: @entity, errors: @errors) + component: Shares::InviteUserFormComponent.new(entity: @entity, available_roles:, errors: @errors) ) update_via_turbo_stream( - component: Shares::CounterComponent.new(work_package: @entity, count: current_visible_member_count) + component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count) ) @new_shares.each do |share| prepend_via_turbo_stream( - component: Shares::ShareRowComponent.new(share:), - target_component: Shares::ModalBodyComponent.new(work_package: @entity, + component: Shares::ShareRowComponent.new(share:, available_roles:), + target_component: Shares::ModalBodyComponent.new(entity: @entity, + available_roles:, shares: load_shares, errors: @errors) ) @@ -154,21 +156,24 @@ def respond_with_prepend_shares end def respond_with_new_invite_form - replace_via_turbo_stream(component: Shares::InviteUserFormComponent.new(work_package: @entity, errors: @errors)) + replace_via_turbo_stream(component: Shares::InviteUserFormComponent.new(entity: @entity, + available_roles:, + errors: @errors)) respond_with_turbo_streams end def respond_with_update_permission_button replace_via_turbo_stream(component: Shares::PermissionButtonComponent.new(share: @share, + available_roles:, data: { "test-selector": "op-share-wp-update-role" })) respond_with_turbo_streams end def respond_with_remove_share - remove_via_turbo_stream(component: Shares::ShareRowComponent.new(share: @share)) - update_via_turbo_stream(component: Shares::CounterComponent.new(work_package: @entity, count: current_visible_member_count)) + remove_via_turbo_stream(component: Shares::ShareRowComponent.new(share: @share, available_roles:)) + update_via_turbo_stream(component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count)) respond_with_turbo_streams end @@ -228,4 +233,30 @@ def load_query def load_shares @shares = load_query.results end + + def load_selected_shares + @selected_shares = Member.includes(:principal) + .of_entity(@entity) + .where(id: params[:share_ids]) + end + + def available_roles + # TODO: Optimize loading of roles + if @entity.is_a?(WorkPackage) + [ + { label: I18n.t("work_package.sharing.permissions.edit"), + value: WorkPackageRole.find_by(builtin: Role::BUILTIN_WORK_PACKAGE_EDITOR).id, + description: I18n.t("work_package.sharing.permissions.edit_description") }, + { label: I18n.t("work_package.sharing.permissions.comment"), + value: WorkPackageRole.find_by(builtin: Role::BUILTIN_WORK_PACKAGE_COMMENTER).id, + description: I18n.t("work_package.sharing.permissions.comment_description") }, + { label: I18n.t("work_package.sharing.permissions.view"), + value: WorkPackageRole.find_by(builtin: Role::BUILTIN_WORK_PACKAGE_VIEWER).id, + description: I18n.t("work_package.sharing.permissions.view_description"), + default: true } + ] + else + [] + end + end end From 5727f1bec6034819ada54c61cb7d30f3be82e2b0 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Thu, 20 Jun 2024 16:50:37 +0200 Subject: [PATCH 12/33] Put bulk behavior into the SharesController --- app/controllers/shares/bulk_controller.rb | 134 ---------------------- app/controllers/shares_controller.rb | 55 ++++++++- config/initializers/permissions.rb | 3 +- config/routes.rb | 6 +- 4 files changed, 57 insertions(+), 141 deletions(-) delete mode 100644 app/controllers/shares/bulk_controller.rb diff --git a/app/controllers/shares/bulk_controller.rb b/app/controllers/shares/bulk_controller.rb deleted file mode 100644 index 9dbf7fe6c4e4..000000000000 --- a/app/controllers/shares/bulk_controller.rb +++ /dev/null @@ -1,134 +0,0 @@ -# frozen_string_literal: true - -# -- copyright -# OpenProject is an open source project management software. -# Copyright (C) 2010-2023 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. -# ++ - -class Shares::BulkController < ApplicationController - include OpTurbo::ComponentStream - include MemberHelper - - before_action :find_work_package - before_action :find_selected_shares - before_action :find_role_ids_from_params, only: :update - before_action :find_project - before_action :authorize - - def update - @selected_shares.each do |share| - WorkPackageMembers::CreateOrUpdateService - .new(user: current_user) - .call(entity: @work_package, - user_id: share.principal.id, - role_ids: @role_ids).result - end - - respond_with_update_permission_buttons - end - - def destroy - @selected_shares.each do |share| - WorkPackageMembers::DeleteService - .new(user: current_user, model: share) - .call - end - - if current_visible_member_count.zero? - respond_with_replace_modal - else - respond_with_remove_shares - end - end - - private - - def respond_with_update_permission_buttons - @selected_shares.each do |share| - replace_via_turbo_stream( - component: Shares::PermissionButtonComponent.new(share:, - data: { "test-selector": "op-share-wp-update-role" }) - ) - end - - respond_with_turbo_streams - end - - def respond_with_replace_modal - replace_via_turbo_stream( - component: Shares::ModalBodyComponent.new(work_package: @work_package, shares: find_shares) - ) - - respond_with_turbo_streams - end - - def respond_with_remove_shares - @selected_shares.each do |share| - remove_via_turbo_stream( - component: Shares::ShareRowComponent.new(share:) - ) - end - - update_via_turbo_stream( - component: Shares::CounterComponent.new(work_package: @work_package, count: current_visible_member_count) - ) - - respond_with_turbo_streams - end - - def find_work_package - @work_package = WorkPackage.find(params[:work_package_id]) - end - - def find_project - @project = @work_package.project - end - - def find_shares - @shares = Member.includes(:principal, :member_roles) - .references(:member_roles) - .of_work_package(@work_package) - .merge(MemberRole.only_non_inherited) - end - - def find_selected_shares - @selected_shares = Member.includes(:principal) - .of_work_package(@work_package) - .where(id: params[:share_ids]) - end - - def find_role_ids_from_params - @role_ids = find_role_ids(params[:role_ids]) - end - - def current_visible_member_count - @current_visible_member_count ||= Member - .joins(:member_roles) - .of_work_package(@work_package) - .merge(MemberRole.only_non_inherited) - .size - end -end diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 6464a16798f4..9b50ea5b77be 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -32,6 +32,7 @@ class SharesController < ApplicationController before_action :load_entity before_action :load_shares, only: %i[index] + before_action :load_selected_shares, only: %i[bulk_update bulk_destroy] before_action :load_share, only: %i[destroy update resend_invite] before_action :authorize before_action :enterprise_check, only: %i[index] @@ -114,6 +115,32 @@ def resend_invite respond_with_update_user_details end + def bulk_update + @selected_shares.each do |share| + WorkPackageMembers::CreateOrUpdateService + .new(user: current_user) + .call(entity: @entity, + user_id: share.principal.id, + role_ids: params[:role_ids]).result + end + + respond_with_bulk_updated_permission_buttons + end + + def bulk_destroy + @selected_shares.each do |share| + WorkPackageMembers::DeleteService + .new(user: current_user, model: share) + .call + end + + if current_visible_member_count.zero? + respond_with_replace_modal + else + respond_with_bulk_removed_shares + end + end + private def enterprise_check @@ -184,8 +211,33 @@ def respond_with_update_user_details respond_with_turbo_streams end + def respond_with_bulk_updated_permission_buttons + @selected_shares.each do |share| + replace_via_turbo_stream( + component: Shares::PermissionButtonComponent.new(share:, + available_roles:, + data: { "test-selector": "op-share-wp-update-role" }) + ) + end + + respond_with_turbo_streams + end + + def respond_with_bulk_removed_shares + @selected_shares.each do |share| + remove_via_turbo_stream( + component: Shares::ShareRowComponent.new(share:, available_roles:) + ) + end + + update_via_turbo_stream( + component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count) + ) + + respond_with_turbo_streams + end + def load_entity - puts "*" * 100, "Loading Entity", params.to_unsafe_h, "*" * 100 @entity = if params["work_package_id"] WorkPackage.visible.find(params["work_package_id"]) # TODO: Add support for other entities @@ -196,7 +248,6 @@ def load_entity ERROR end - puts "**** Entity: #{@entity} ****" if @entity.respond_to?(:project) @project = @entity.project end diff --git a/config/initializers/permissions.rb b/config/initializers/permissions.rb index 9dd015f92837..8b4ab0de9719 100644 --- a/config/initializers/permissions.rb +++ b/config/initializers/permissions.rb @@ -331,8 +331,7 @@ map.permission :share_work_packages, { members: %i[destroy_by_principal], - shares: %i[index create destroy update resend_invite], - "shares/bulk": %i[update destroy] + shares: %i[index create destroy update resend_invite bulk_update bulk_destroy] }, permissible_on: :project, dependencies: %i[edit_work_packages view_shared_work_packages], diff --git a/config/routes.rb b/config/routes.rb index c8329727ee62..10d63e1decb6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -85,9 +85,9 @@ end collection do - patch :bulk, to: "shares/bulk#update" - put :bulk, to: "shares/bulk#update" - delete :bulk, to: "shares/bulk#destroy" + patch :bulk, to: "shares#bulk_update" + put :bulk, to: "shares#bulk_update" + delete :bulk, to: "shares#bulk_destroy" end end end From 594162b1f2036876803f33a5b9958c08ac816e45 Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Thu, 20 Jun 2024 16:24:45 -0500 Subject: [PATCH 13/33] Fix routing specs --- spec/routing/work_package/shares_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/routing/work_package/shares_spec.rb b/spec/routing/work_package/shares_spec.rb index 48c6e25ec28a..d87be5f471c9 100644 --- a/spec/routing/work_package/shares_spec.rb +++ b/spec/routing/work_package/shares_spec.rb @@ -72,15 +72,15 @@ context "on bulk actions" do it "routes DELETE /work_packages/:work_package_id/shares/bulk to shares/bulk#destroy" do expect(delete("/work_packages/1/shares/bulk")) - .to route_to(controller: "shares/bulk", - action: "destroy", + .to route_to(controller: "shares", + action: "bulk_destroy", work_package_id: "1") end it "routes PATCH /work_packages/:work_package_id/shares/bulk to shares/bulk#update" do expect(patch("/work_packages/1/shares/bulk")) - .to route_to(controller: "shares/bulk", - action: "update", + .to route_to(controller: "shares", + action: "bulk_update", work_package_id: "1") end end From 7a71b145f53814308cae8a47620c725a2ca694f3 Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Thu, 20 Jun 2024 16:40:03 -0500 Subject: [PATCH 14/33] Fix controller specs --- app/controllers/members_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/members_controller.rb b/app/controllers/members_controller.rb index b18e0bbc2b65..0a0a3179c56f 100644 --- a/app/controllers/members_controller.rb +++ b/app/controllers/members_controller.rb @@ -86,8 +86,8 @@ def destroy_by_principal principal = Principal.find(params[:principal_id]) service_call = Members::DeleteByPrincipalService - .new(user: current_user, project: @project, principal:) - .call(params.permit(:project, :work_package_shares_role_id)) + .new(user: current_user, project: @project, principal:) + .call(params.permit(:project, :work_package_shares_role_id)) if service_call.success? flash[:notice] = I18n.t(:notice_member_removed, user: principal.name) @@ -145,7 +145,7 @@ def members_table_options(roles) authorize_update: authorize_for("members", :update), authorize_delete: authorize_for("members", :destroy), authorize_work_package_shares_view: authorize_for("shares", :update), - authorize_work_package_shares_delete: authorize_for("shares/bulk", :destroy), + authorize_work_package_shares_delete: authorize_for("shares", :bulk_destroy), authorize_manage_user: current_user.allowed_globally?(:manage_user), is_filtered: Members::UserFilterComponent.filtered?(params), shared_role_name: From bc44485e1117c883d91a23ed0309cb0b9eded6a1 Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Fri, 21 Jun 2024 10:49:27 -0500 Subject: [PATCH 15/33] Fix feature specs --- .../work_packages/share/share_account_activation_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/features/work_packages/share/share_account_activation_spec.rb b/spec/features/work_packages/share/share_account_activation_spec.rb index abbadf5e2041..be19c842df6d 100644 --- a/spec/features/work_packages/share/share_account_activation_spec.rb +++ b/spec/features/work_packages/share/share_account_activation_spec.rb @@ -33,6 +33,8 @@ RSpec.describe "Work package sharing invited users", :js, :with_cuprite, with_ee: %i[work_package_sharing] do + shared_let(:edit_work_package_role) { create(:edit_work_package_role) } + shared_let(:comment_work_package_role) { create(:comment_work_package_role) } shared_let(:view_work_package_role) { create(:view_work_package_role) } shared_let(:editor) { create(:admin, firstname: "Mr.", lastname: "Sharer") } From 17e3dc56bb4fabde0af82116c2fe53d98bda5ce1 Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Fri, 21 Jun 2024 11:05:53 -0500 Subject: [PATCH 16/33] Fix active role filter method --- app/components/shares/modal_body_component.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/components/shares/modal_body_component.rb b/app/components/shares/modal_body_component.rb index 0df017788b0e..72cfc8503515 100644 --- a/app/components/shares/modal_body_component.rb +++ b/app/components/shares/modal_body_component.rb @@ -117,7 +117,9 @@ def role_filter_option_active?(_option) return false if role_filter_value.nil? - find_role_ids(_option[:value]).first == role_filter_value.to_i + selected_role = @available_roles.find { _1[:value] == _option[:value] } + + selected_role[:value] == role_filter_value.to_i end def filter_url(type_option: nil, role_option: nil) From 39cf34655ec3b6b9896dd2cdbb4a845fcabe0dc7 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Mon, 24 Jun 2024 12:22:10 +0200 Subject: [PATCH 17/33] Generalize contracts and services for sharing --- .../base_contract.rb | 18 +++---- .../create_contract.rb | 2 +- .../delete_contract.rb | 4 +- .../update_contract.rb | 2 +- .../shares/work_packages/base_extension.rb | 54 +++++++++++++++++++ .../shares/work_packages/create_contract.rb | 35 ++++++++++++ .../shares/work_packages/delete_contract.rb | 37 +++++++++++++ .../shares/work_packages/update_contract.rb | 35 ++++++++++++ app/controllers/shares_controller.rb | 51 +++++++++--------- .../members/delete_by_principal_service.rb | 8 +-- .../concerns/role_assignment.rb | 6 +-- .../create_or_update_service.rb | 18 +++---- .../create_service.rb | 25 +++++---- .../delete_role_service.rb | 2 +- .../delete_service.rb | 12 ++--- .../set_attributes_service.rb | 8 +-- .../update_service.rb | 16 +++--- .../work_packages}/create_contract_spec.rb | 2 +- .../work_packages}/delete_contract_spec.rb | 2 +- .../shared_contract_examples.rb | 0 .../work_packages}/update_contract_spec.rb | 2 +- .../delete_by_principal_service_spec.rb | 22 ++++---- .../create_or_update_service_spec.rb | 2 +- .../create_service_spec.rb | 2 +- .../delete_role_service_spec.rb | 2 +- .../delete_service_spec.rb | 2 +- .../set_attributes_service_spec.rb | 8 +-- .../update_service_spec.rb | 2 +- 28 files changed, 266 insertions(+), 113 deletions(-) rename app/contracts/{work_package_members => shares}/base_contract.rb (89%) rename app/contracts/{work_package_members => shares}/create_contract.rb (98%) rename app/contracts/{work_package_members => shares}/delete_contract.rb (95%) rename app/contracts/{work_package_members => shares}/update_contract.rb (98%) create mode 100644 app/contracts/shares/work_packages/base_extension.rb create mode 100644 app/contracts/shares/work_packages/create_contract.rb create mode 100644 app/contracts/shares/work_packages/delete_contract.rb create mode 100644 app/contracts/shares/work_packages/update_contract.rb rename app/services/{work_package_members => shares}/concerns/role_assignment.rb (90%) rename app/services/{work_package_members => shares}/create_or_update_service.rb (70%) rename app/services/{work_package_members => shares}/create_service.rb (71%) rename app/services/{work_package_members => shares}/delete_role_service.rb (94%) rename app/services/{work_package_members => shares}/delete_service.rb (82%) rename app/services/{work_package_members => shares}/set_attributes_service.rb (89%) rename app/services/{work_package_members => shares}/update_service.rb (75%) rename spec/contracts/{work_package_members => shares/work_packages}/create_contract_spec.rb (98%) rename spec/contracts/{work_package_members => shares/work_packages}/delete_contract_spec.rb (98%) rename spec/contracts/{work_package_members => shares/work_packages}/shared_contract_examples.rb (100%) rename spec/contracts/{work_package_members => shares/work_packages}/update_contract_spec.rb (97%) rename spec/services/{work_package_members => shares}/create_or_update_service_spec.rb (98%) rename spec/services/{work_package_members => shares}/create_service_spec.rb (98%) rename spec/services/{work_package_members => shares}/delete_role_service_spec.rb (98%) rename spec/services/{work_package_members => shares}/delete_service_spec.rb (98%) rename spec/services/{work_package_members => shares}/set_attributes_service_spec.rb (96%) rename spec/services/{work_package_members => shares}/update_service_spec.rb (98%) diff --git a/app/contracts/work_package_members/base_contract.rb b/app/contracts/shares/base_contract.rb similarity index 89% rename from app/contracts/work_package_members/base_contract.rb rename to app/contracts/shares/base_contract.rb index 792865969400..020973c6dd03 100644 --- a/app/contracts/work_package_members/base_contract.rb +++ b/app/contracts/shares/base_contract.rb @@ -26,17 +26,13 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackageMembers +module Shares class BaseContract < ::ModelContract - delegate :project, - to: :model - attribute :roles validate :user_allowed_to_manage validate :role_grantable validate :single_non_inherited_role - validate :project_set validate :entity_set attribute_alias(:user_id, :principal) @@ -50,7 +46,7 @@ def user_allowed_to_manage end def user_allowed_to_manage? - user.allowed_in_project?(:share_work_packages, model.project) + raise NotImplementedError, "Must be overridden by subclass" end def single_non_inherited_role @@ -58,11 +54,7 @@ def single_non_inherited_role end def role_grantable - errors.add(:roles, :ungrantable) unless active_roles.all? { _1.is_a?(WorkPackageRole) } - end - - def project_set - errors.add(:project, :blank) if project.nil? + errors.add(:roles, :ungrantable) unless active_roles.all? { _1.is_a?(assignable_role_class) } end def active_roles @@ -80,5 +72,9 @@ def active_member_roles def entity_set errors.add(:entity, :blank) if entity_id.nil? end + + def assignable_role_class + raise NotImplementedError, "Must be overridden by subclass" + end end end diff --git a/app/contracts/work_package_members/create_contract.rb b/app/contracts/shares/create_contract.rb similarity index 98% rename from app/contracts/work_package_members/create_contract.rb rename to app/contracts/shares/create_contract.rb index c21ce60ef1a2..190af5d6ea5e 100644 --- a/app/contracts/work_package_members/create_contract.rb +++ b/app/contracts/shares/create_contract.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackageMembers +module Shares class CreateContract < BaseContract attribute :principal attribute :entity_id diff --git a/app/contracts/work_package_members/delete_contract.rb b/app/contracts/shares/delete_contract.rb similarity index 95% rename from app/contracts/work_package_members/delete_contract.rb rename to app/contracts/shares/delete_contract.rb index 41726467fa50..165815bb1fb7 100644 --- a/app/contracts/work_package_members/delete_contract.rb +++ b/app/contracts/shares/delete_contract.rb @@ -26,10 +26,8 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackageMembers +module Shares class DeleteContract < ::DeleteContract - delete_permission :share_work_packages - validate :member_is_deletable private diff --git a/app/contracts/work_package_members/update_contract.rb b/app/contracts/shares/update_contract.rb similarity index 98% rename from app/contracts/work_package_members/update_contract.rb rename to app/contracts/shares/update_contract.rb index 785ec1030ff4..355a739d558c 100644 --- a/app/contracts/work_package_members/update_contract.rb +++ b/app/contracts/shares/update_contract.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackageMembers +module Shares class UpdateContract < BaseContract attribute :principal, writable: false diff --git a/app/contracts/shares/work_packages/base_extension.rb b/app/contracts/shares/work_packages/base_extension.rb new file mode 100644 index 000000000000..6fa99c6a5300 --- /dev/null +++ b/app/contracts/shares/work_packages/base_extension.rb @@ -0,0 +1,54 @@ +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2010-2023 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. +# ++ + +module Shares + module WorkPackages + module BaseExtension + extend ActiveSupport::Concern + + included do + delegate :project, to: :model + validate :project_set + end + + private + + def user_allowed_to_manage? + user.allowed_in_project?(:share_work_packages, project) + end + + def assignable_role_class + WorkPackageRole + end + + def project_set + errors.add(:project, :blank) if project.nil? + end + end + end +end diff --git a/app/contracts/shares/work_packages/create_contract.rb b/app/contracts/shares/work_packages/create_contract.rb new file mode 100644 index 000000000000..eced07e71743 --- /dev/null +++ b/app/contracts/shares/work_packages/create_contract.rb @@ -0,0 +1,35 @@ +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2010-2023 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. +# ++ + +module Shares + module WorkPackages + class CreateContract < Shares::CreateContract + include Shares::WorkPackages::BaseExtension + end + end +end diff --git a/app/contracts/shares/work_packages/delete_contract.rb b/app/contracts/shares/work_packages/delete_contract.rb new file mode 100644 index 000000000000..a2ac75399add --- /dev/null +++ b/app/contracts/shares/work_packages/delete_contract.rb @@ -0,0 +1,37 @@ +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2010-2023 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. +# ++ + +module Shares + module WorkPackages + class DeleteContract < Shares::DeleteContract + # DeleteContract has its own permission check and does not care about the role class, + # so we do not need to include the BaseExtension here. + delete_permission :share_work_packages + end + end +end diff --git a/app/contracts/shares/work_packages/update_contract.rb b/app/contracts/shares/work_packages/update_contract.rb new file mode 100644 index 000000000000..f38d818cda01 --- /dev/null +++ b/app/contracts/shares/work_packages/update_contract.rb @@ -0,0 +1,35 @@ +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2010-2023 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. +# ++ + +module Shares + module WorkPackages + class UpdateContract < Shares::UpdateContract + include Shares::WorkPackages::BaseExtension + end + end +end diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 9b50ea5b77be..78f6167c8bb6 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -54,12 +54,7 @@ def create if user.present? && user.locked? @errors.add(:base, I18n.t("work_package.sharing.warning_locked_user", user: user.name)) else - service_call = WorkPackageMembers::CreateOrUpdateService - .new(user: current_user) - .call(entity: @entity, - user_id: member_params[:user_id], - role_ids: [params[:member][:role_id]]) - + service_call = create_or_update_share(member_params[:user_id], [params[:member][:role_id]]) overall_result.push(service_call) end end @@ -80,9 +75,7 @@ def create end def update - WorkPackageMembers::UpdateService - .new(user: current_user, model: @share) - .call(role_ids: params[:role_ids]) + create_or_update_share(@share.principal.id, params[:role_ids]) load_shares @@ -96,9 +89,7 @@ def update end def destroy - WorkPackageMembers::DeleteService - .new(user: current_user, model: @share) - .call + destroy_share(@share) if current_visible_member_count.zero? respond_with_replace_modal @@ -116,23 +107,13 @@ def resend_invite end def bulk_update - @selected_shares.each do |share| - WorkPackageMembers::CreateOrUpdateService - .new(user: current_user) - .call(entity: @entity, - user_id: share.principal.id, - role_ids: params[:role_ids]).result - end + @selected_shares.each { |share| create_or_update_share(share.principal.id, params[:role_ids]) } respond_with_bulk_updated_permission_buttons end def bulk_destroy - @selected_shares.each do |share| - WorkPackageMembers::DeleteService - .new(user: current_user, model: share) - .call - end + @selected_shares.each { |share| destroy_share(share) } if current_visible_member_count.zero? respond_with_replace_modal @@ -149,6 +130,22 @@ def enterprise_check render Shares::ModalUpsaleComponent.new end + def destroy_share(share) + Shares::DeleteService + .new(user: current_user, model: share, contract_class: sharing_contract_scope::DeleteContract) + .call + end + + def create_or_update_share(user_id, role_ids) + Shares::CreateOrUpdateService + .new( + user: current_user, + create_contract_class: sharing_contract_scope::CreateContract, + update_contract_class: sharing_contract_scope::UpdateContract + ) + .call(entity: @entity, user_id:, role_ids:) + end + def respond_with_replace_modal replace_via_turbo_stream( component: Shares::ModalBodyComponent.new(entity: @entity, @@ -310,4 +307,10 @@ def available_roles [] end end + + def sharing_contract_scope + if @entity.is_a?(WorkPackage) + Shares::WorkPackages + end + end end diff --git a/app/services/members/delete_by_principal_service.rb b/app/services/members/delete_by_principal_service.rb index 620ce72035df..87ac0c59e970 100644 --- a/app/services/members/delete_by_principal_service.rb +++ b/app/services/members/delete_by_principal_service.rb @@ -67,14 +67,14 @@ def delete_project_member end def delete_work_package_share(model) - WorkPackageMembers::DeleteService - .new(user:, model:) + Shares::WorkPackages::DeleteService + .new(user:, model:, contract_class: Shares::WorkPackages::DeleteContract) .call end def delete_work_package_share_with_role_id(model, role_id) - WorkPackageMembers::DeleteRoleService - .new(user:, model:) + Shares::WorkPackages::DeleteRoleService + .new(user:, model:, contract_class: Shares::WorkPackages::DeleteContract) .call(role_id:) end diff --git a/app/services/work_package_members/concerns/role_assignment.rb b/app/services/shares/concerns/role_assignment.rb similarity index 90% rename from app/services/work_package_members/concerns/role_assignment.rb rename to app/services/shares/concerns/role_assignment.rb index 31bdb1194332..0df795fdd0c3 100644 --- a/app/services/work_package_members/concerns/role_assignment.rb +++ b/app/services/shares/concerns/role_assignment.rb @@ -28,11 +28,11 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackageMembers::Concerns::RoleAssignment +module Shares::Concerns::RoleAssignment include Members::Concerns::RoleAssignment - # Work package memberships have a unique distinction from - # project memberships. A User should be able to be granted + # Memberships via shares have a unique distinction from + # regular project memberships. A User should be able to be granted # "Role X" independently and via a group. Meaning that for role assignment # as compared to Project memberships, the existing roles we want to take # into account are those that have not been inherited. diff --git a/app/services/work_package_members/create_or_update_service.rb b/app/services/shares/create_or_update_service.rb similarity index 70% rename from app/services/work_package_members/create_or_update_service.rb rename to app/services/shares/create_or_update_service.rb index 302f55b36a39..becdd8681719 100644 --- a/app/services/work_package_members/create_or_update_service.rb +++ b/app/services/shares/create_or_update_service.rb @@ -26,29 +26,27 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -class WorkPackageMembers::CreateOrUpdateService - def initialize(user:, contract_class: nil, contract_options: {}) +class Shares::CreateOrUpdateService + def initialize(user:, create_contract_class:, update_contract_class:, contract_options: {}) self.user = user - self.contract_class = contract_class + self.create_contract_class = create_contract_class + self.update_contract_class = update_contract_class self.contract_options = contract_options end def call(entity:, user_id:, **) - actual_service(entity, user_id) - .call(entity:, user_id:, **) + actual_service(entity, user_id).call(entity:, user_id:, **) end private - attr_accessor :user, :contract_class, :contract_options + attr_accessor :user, :create_contract_class, :update_contract_class, :contract_options def actual_service(entity, user_id) if (member = Member.find_by(entity:, principal: user_id)) - WorkPackageMembers::UpdateService - .new(user:, model: member, contract_class:, contract_options:) + Shares::UpdateService.new(user:, model: member, contract_class: update_contract_class, contract_options:) else - WorkPackageMembers::CreateService - .new(user:, contract_class:, contract_options:) + Shares::CreateService.new(user:, contract_class: create_contract_class, contract_options:) end end end diff --git a/app/services/work_package_members/create_service.rb b/app/services/shares/create_service.rb similarity index 71% rename from app/services/work_package_members/create_service.rb rename to app/services/shares/create_service.rb index ca0706235846..4f6b11f5e889 100644 --- a/app/services/work_package_members/create_service.rb +++ b/app/services/shares/create_service.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -class WorkPackageMembers::CreateService < BaseServices::Create +class Shares::CreateService < BaseServices::Create private def instance_class @@ -36,29 +36,28 @@ def instance_class def after_perform(service_call) return service_call unless service_call.success? - work_package_member = service_call.result + share = service_call.result - add_group_memberships(work_package_member) - send_notification(work_package_member) + add_group_memberships(share) + send_notification(share) service_call end - def add_group_memberships(work_package_member) - return unless work_package_member.principal.is_a?(Group) + def add_group_memberships(share) + return unless share.principal.is_a?(Group) Groups::CreateInheritedRolesService - .new(work_package_member.principal, - current_user: user, - contract_class: EmptyContract) - .call(user_ids: work_package_member.principal.user_ids, + .new(share.principal, current_user: user, contract_class: EmptyContract) + .call(user_ids: share.principal.user_ids, send_notifications: false, - project_ids: [work_package_member.project_id]) + project_ids: [share.project_id]) # TODO: Here we should add project_id and the entity id as well end - def send_notification(work_package_member) + def send_notification(share) + # TODO: We should select what sort of notification is sent out based on the shared entity OpenProject::Notifications.send(OpenProject::Events::WORK_PACKAGE_SHARED, - work_package_member:, + work_package_member: share, send_notifications: true) end end diff --git a/app/services/work_package_members/delete_role_service.rb b/app/services/shares/delete_role_service.rb similarity index 94% rename from app/services/work_package_members/delete_role_service.rb rename to app/services/shares/delete_role_service.rb index 54a3710a501d..edde8919863c 100644 --- a/app/services/work_package_members/delete_role_service.rb +++ b/app/services/shares/delete_role_service.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -class WorkPackageMembers::DeleteRoleService < WorkPackageMembers::DeleteService +class Shares::DeleteRoleService < Shares::DeleteService def destroy(object) if object.member_roles.where.not("inherited_from IS NULL AND role_id = ?", params[:role_id]).empty? super diff --git a/app/services/work_package_members/delete_service.rb b/app/services/shares/delete_service.rb similarity index 82% rename from app/services/work_package_members/delete_service.rb rename to app/services/shares/delete_service.rb index 1574b02a09a6..b935e618a04d 100644 --- a/app/services/work_package_members/delete_service.rb +++ b/app/services/shares/delete_service.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -class WorkPackageMembers::DeleteService < BaseServices::Delete +class Shares::DeleteService < BaseServices::Delete include Members::Concerns::CleanedUp def destroy(object) @@ -41,17 +41,17 @@ def destroy(object) def after_perform(service_call) super.tap do |call| - work_package_member = call.result + share = call.result - cleanup_for_group(work_package_member) + cleanup_for_group(share) end end - def cleanup_for_group(work_package_member) - return unless work_package_member.principal.is_a?(Group) + def cleanup_for_group(share) + return unless share.principal.is_a?(Group) Groups::CleanupInheritedRolesService - .new(work_package_member.principal, current_user: user, contract_class: EmptyContract) + .new(share.principal, current_user: user, contract_class: EmptyContract) .call end end diff --git a/app/services/work_package_members/set_attributes_service.rb b/app/services/shares/set_attributes_service.rb similarity index 89% rename from app/services/work_package_members/set_attributes_service.rb rename to app/services/shares/set_attributes_service.rb index 70bc10ec483d..1ef7f078e45f 100644 --- a/app/services/work_package_members/set_attributes_service.rb +++ b/app/services/shares/set_attributes_service.rb @@ -26,9 +26,9 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -module WorkPackageMembers +module Shares class SetAttributesService < ::BaseServices::SetAttributes - prepend WorkPackageMembers::Concerns::RoleAssignment + prepend Shares::Concerns::RoleAssignment private @@ -36,7 +36,9 @@ def set_attributes(params) super model.change_by_system do - model.project = model.entity&.project + if model.entity.respond_to?(:project) + model.project = model.entity&.project + end end end end diff --git a/app/services/work_package_members/update_service.rb b/app/services/shares/update_service.rb similarity index 75% rename from app/services/work_package_members/update_service.rb rename to app/services/shares/update_service.rb index 42dae489b44d..a065256d24d5 100644 --- a/app/services/work_package_members/update_service.rb +++ b/app/services/shares/update_service.rb @@ -26,7 +26,7 @@ # See COPYRIGHT and LICENSE files for more details. # ++ -class WorkPackageMembers::UpdateService < BaseServices::Update +class Shares::UpdateService < BaseServices::Update include Members::Concerns::CleanedUp protected @@ -34,20 +34,16 @@ class WorkPackageMembers::UpdateService < BaseServices::Update def after_perform(service_call) return service_call unless service_call.success? - work_package_member = service_call.result + share = service_call.result - update_group_roles(work_package_member) if work_package_member.principal.is_a?(Group) + update_group_roles(share) if share.principal.is_a?(Group) service_call end - def update_group_roles(work_package_member) + def update_group_roles(share) Groups::UpdateRolesService - .new(work_package_member.principal, - current_user: user, - contract_class: EmptyContract) - .call(member: work_package_member, - send_notifications: false, - message: nil) + .new(share.principal, current_user: user, contract_class: EmptyContract) + .call(member: share, send_notifications: false, message: nil) end end diff --git a/spec/contracts/work_package_members/create_contract_spec.rb b/spec/contracts/shares/work_packages/create_contract_spec.rb similarity index 98% rename from spec/contracts/work_package_members/create_contract_spec.rb rename to spec/contracts/shares/work_packages/create_contract_spec.rb index 291380cb657f..0af0bd377379 100644 --- a/spec/contracts/work_package_members/create_contract_spec.rb +++ b/spec/contracts/shares/work_packages/create_contract_spec.rb @@ -29,7 +29,7 @@ require "spec_helper" require_relative "shared_contract_examples" -RSpec.describe WorkPackageMembers::CreateContract do +RSpec.describe Shares::WorkPackages::CreateContract do it_behaves_like "work package member contract" do let(:member) do Member.new(roles: member_roles, diff --git a/spec/contracts/work_package_members/delete_contract_spec.rb b/spec/contracts/shares/work_packages/delete_contract_spec.rb similarity index 98% rename from spec/contracts/work_package_members/delete_contract_spec.rb rename to spec/contracts/shares/work_packages/delete_contract_spec.rb index c16d829481df..648466cd27d6 100644 --- a/spec/contracts/work_package_members/delete_contract_spec.rb +++ b/spec/contracts/shares/work_packages/delete_contract_spec.rb @@ -29,7 +29,7 @@ require "spec_helper" require "contracts/shared/model_contract_shared_context" -RSpec.describe WorkPackageMembers::DeleteContract do +RSpec.describe Shares::WorkPackages::DeleteContract do include_context "ModelContract shared context" let(:contract) { described_class.new(member, current_user) } diff --git a/spec/contracts/work_package_members/shared_contract_examples.rb b/spec/contracts/shares/work_packages/shared_contract_examples.rb similarity index 100% rename from spec/contracts/work_package_members/shared_contract_examples.rb rename to spec/contracts/shares/work_packages/shared_contract_examples.rb diff --git a/spec/contracts/work_package_members/update_contract_spec.rb b/spec/contracts/shares/work_packages/update_contract_spec.rb similarity index 97% rename from spec/contracts/work_package_members/update_contract_spec.rb rename to spec/contracts/shares/work_packages/update_contract_spec.rb index 707e3b021b11..ae5bad5ee273 100644 --- a/spec/contracts/work_package_members/update_contract_spec.rb +++ b/spec/contracts/shares/work_packages/update_contract_spec.rb @@ -29,7 +29,7 @@ require "spec_helper" require_relative "shared_contract_examples" -RSpec.describe WorkPackageMembers::UpdateContract do +RSpec.describe Shares::WorkPackages::UpdateContract do it_behaves_like "work package member contract" do let(:member) do build_stubbed(:work_package_member, diff --git a/spec/services/members/delete_by_principal_service_spec.rb b/spec/services/members/delete_by_principal_service_spec.rb index 694fddd17bbf..ca7ce07261ec 100644 --- a/spec/services/members/delete_by_principal_service_spec.rb +++ b/spec/services/members/delete_by_principal_service_spec.rb @@ -99,17 +99,17 @@ member_roles: [build(:member_role), build(:member_role, inherited_from: 123)], entity: work_package_c) end - let(:service_instance_a) { instance_double(WorkPackageMembers::DeleteService, call: service_result_a) } - let(:service_instance_c) { instance_double(WorkPackageMembers::DeleteService, call: service_result_c) } + let(:service_instance_a) { instance_double(Shares::DeleteService, call: service_result_a) } + let(:service_instance_c) { instance_double(Shares::DeleteService, call: service_result_c) } let(:service_result_a) { ServiceResult.success } let(:service_result_c) { ServiceResult.success } before do - allow(WorkPackageMembers::DeleteService) + allow(Shares::DeleteService) .to receive(:new) .with(user:, model: member_a) .and_return(service_instance_a) - allow(WorkPackageMembers::DeleteService) + allow(Shares::DeleteService) .to receive(:new) .with(user:, model: member_c) .and_return(service_instance_c) @@ -119,7 +119,7 @@ service_call expect(service_instance_a).to have_received(:call).with(no_args) - expect(WorkPackageMembers::DeleteService) + expect(Shares::DeleteService) .not_to have_received(:new) .with(user:, model: member_b) expect(service_instance_c).to have_received(:call).with(no_args) @@ -171,17 +171,17 @@ member_roles: [build(:member_role)], entity: work_package_d) end - let(:service_instance_a) { instance_double(WorkPackageMembers::DeleteRoleService, call: service_result_a) } - let(:service_instance_c) { instance_double(WorkPackageMembers::DeleteRoleService, call: service_result_c) } + let(:service_instance_a) { instance_double(Shares::DeleteRoleService, call: service_result_a) } + let(:service_instance_c) { instance_double(Shares::DeleteRoleService, call: service_result_c) } let(:service_result_a) { ServiceResult.success } let(:service_result_c) { ServiceResult.success } before do - allow(WorkPackageMembers::DeleteRoleService) + allow(Shares::DeleteRoleService) .to receive(:new) .with(user:, model: member_a) .and_return(service_instance_a) - allow(WorkPackageMembers::DeleteRoleService) + allow(Shares::DeleteRoleService) .to receive(:new) .with(user:, model: member_c) .and_return(service_instance_c) @@ -191,11 +191,11 @@ service_call expect(service_instance_a).to have_received(:call).with(role_id: role.id.to_s) - expect(WorkPackageMembers::DeleteRoleService) + expect(Shares::DeleteRoleService) .not_to have_received(:new) .with(user:, model: member_b) expect(service_instance_c).to have_received(:call).with(role_id: role.id.to_s) - expect(WorkPackageMembers::DeleteRoleService) + expect(Shares::DeleteRoleService) .not_to have_received(:new) .with(user:, model: member_d) end diff --git a/spec/services/work_package_members/create_or_update_service_spec.rb b/spec/services/shares/create_or_update_service_spec.rb similarity index 98% rename from spec/services/work_package_members/create_or_update_service_spec.rb rename to spec/services/shares/create_or_update_service_spec.rb index 5ea7609054d7..b2a9684f5779 100644 --- a/spec/services/work_package_members/create_or_update_service_spec.rb +++ b/spec/services/shares/create_or_update_service_spec.rb @@ -30,7 +30,7 @@ require "spec_helper" -RSpec.describe WorkPackageMembers::CreateOrUpdateService do +RSpec.describe Shares::CreateOrUpdateService do let(:user) { build_stubbed(:user) } let(:role) { build_stubbed(:view_work_package_role) } let(:work_package) { build_stubbed(:work_package) } diff --git a/spec/services/work_package_members/create_service_spec.rb b/spec/services/shares/create_service_spec.rb similarity index 98% rename from spec/services/work_package_members/create_service_spec.rb rename to spec/services/shares/create_service_spec.rb index a137bafbf06c..df44385f1c95 100644 --- a/spec/services/work_package_members/create_service_spec.rb +++ b/spec/services/shares/create_service_spec.rb @@ -31,7 +31,7 @@ require "spec_helper" require "services/base_services/behaves_like_create_service" -RSpec.describe WorkPackageMembers::CreateService, type: :model do +RSpec.describe Shares::CreateService, type: :model do subject(:service_call) { instance.call(call_attributes) } let(:instance) { described_class.new(user:) } diff --git a/spec/services/work_package_members/delete_role_service_spec.rb b/spec/services/shares/delete_role_service_spec.rb similarity index 98% rename from spec/services/work_package_members/delete_role_service_spec.rb rename to spec/services/shares/delete_role_service_spec.rb index 8625551c5dae..d1eb9bf02206 100644 --- a/spec/services/work_package_members/delete_role_service_spec.rb +++ b/spec/services/shares/delete_role_service_spec.rb @@ -31,7 +31,7 @@ require "spec_helper" require "services/base_services/behaves_like_delete_service" -RSpec.describe WorkPackageMembers::DeleteRoleService, type: :model do +RSpec.describe Shares::DeleteRoleService, type: :model do it_behaves_like "BaseServices delete service" do let(:model_class) { Member } let(:model_instance) { build_stubbed(:work_package_member, principal:) } diff --git a/spec/services/work_package_members/delete_service_spec.rb b/spec/services/shares/delete_service_spec.rb similarity index 98% rename from spec/services/work_package_members/delete_service_spec.rb rename to spec/services/shares/delete_service_spec.rb index ff30cc211b0a..6fc39360a696 100644 --- a/spec/services/work_package_members/delete_service_spec.rb +++ b/spec/services/shares/delete_service_spec.rb @@ -31,7 +31,7 @@ require "spec_helper" require "services/base_services/behaves_like_delete_service" -RSpec.describe WorkPackageMembers::DeleteService, type: :model do +RSpec.describe Shares::DeleteService, type: :model do it_behaves_like "BaseServices delete service" do let(:model_class) { Member } let(:model_instance) { build_stubbed(:work_package_member, principal:) } diff --git a/spec/services/work_package_members/set_attributes_service_spec.rb b/spec/services/shares/set_attributes_service_spec.rb similarity index 96% rename from spec/services/work_package_members/set_attributes_service_spec.rb rename to spec/services/shares/set_attributes_service_spec.rb index b7111e67fe8d..96112b9300d4 100644 --- a/spec/services/work_package_members/set_attributes_service_spec.rb +++ b/spec/services/shares/set_attributes_service_spec.rb @@ -28,7 +28,7 @@ require "spec_helper" -RSpec.describe WorkPackageMembers::SetAttributesService, type: :model do +RSpec.describe Shares::SetAttributesService, type: :model do let(:user) { build_stubbed(:user) } let(:work_package) { build_stubbed(:work_package) } let(:member) do @@ -38,15 +38,15 @@ let(:existing_member) { build_stubbed(:work_package_member) } let(:contract_class) do - allow(WorkPackageMembers::CreateContract) + allow(Shares::WorkPackages::CreateContract) .to receive(:new) .with(member, user, options: {}) .and_return(contract_instance) - WorkPackageMembers::CreateContract + Shares::WorkPackages end let(:contract_instance) do - instance_double(WorkPackageMembers::CreateContract, validate: contract_valid, errors: contract_errors) + instance_double(Shares::WorkPackages::CreateContract, validate: contract_valid, errors: contract_errors) end let(:contract_valid) { true } let(:contract_errors) do diff --git a/spec/services/work_package_members/update_service_spec.rb b/spec/services/shares/update_service_spec.rb similarity index 98% rename from spec/services/work_package_members/update_service_spec.rb rename to spec/services/shares/update_service_spec.rb index d2ffd4fd663a..cff496da09ba 100644 --- a/spec/services/work_package_members/update_service_spec.rb +++ b/spec/services/shares/update_service_spec.rb @@ -31,7 +31,7 @@ require "spec_helper" require "services/base_services/behaves_like_update_service" -RSpec.describe WorkPackageMembers::UpdateService do +RSpec.describe Shares::UpdateService do let!(:groups_update_roles_service) do instance_double(Groups::UpdateRolesService).tap do |service_double| allow(Groups::UpdateRolesService) From f261cc2ce1ecaee6c96e5208221d0990aa4bab09 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Mon, 24 Jun 2024 14:29:55 +0200 Subject: [PATCH 18/33] Fix specs --- .../members/delete_by_principal_service.rb | 12 ++------ .../delete_by_principal_service_spec.rb | 14 ++++----- .../shares/create_or_update_service_spec.rb | 29 +++++++++++++------ spec/services/shares/create_service_spec.rb | 1 + .../shares/delete_role_service_spec.rb | 1 + spec/services/shares/delete_service_spec.rb | 1 + .../shares/set_attributes_service_spec.rb | 2 +- 7 files changed, 34 insertions(+), 26 deletions(-) diff --git a/app/services/members/delete_by_principal_service.rb b/app/services/members/delete_by_principal_service.rb index 87ac0c59e970..f52fd5c89999 100644 --- a/app/services/members/delete_by_principal_service.rb +++ b/app/services/members/delete_by_principal_service.rb @@ -61,21 +61,15 @@ def call(params) def delete_project_member project_member = Member.of_project(project).find_by!(principal:) - Members::DeleteService - .new(user:, model: project_member) - .call + Members::DeleteService.new(user:, model: project_member).call end def delete_work_package_share(model) - Shares::WorkPackages::DeleteService - .new(user:, model:, contract_class: Shares::WorkPackages::DeleteContract) - .call + Shares::DeleteService.new(user:, model:, contract_class: Shares::WorkPackages::DeleteContract).call end def delete_work_package_share_with_role_id(model, role_id) - Shares::WorkPackages::DeleteRoleService - .new(user:, model:, contract_class: Shares::WorkPackages::DeleteContract) - .call(role_id:) + Shares::DeleteRoleService.new(user:, model:, contract_class: Shares::WorkPackages::DeleteContract).call(role_id:) end def work_package_shares_scope diff --git a/spec/services/members/delete_by_principal_service_spec.rb b/spec/services/members/delete_by_principal_service_spec.rb index ca7ce07261ec..af3e02d8ef75 100644 --- a/spec/services/members/delete_by_principal_service_spec.rb +++ b/spec/services/members/delete_by_principal_service_spec.rb @@ -107,11 +107,11 @@ before do allow(Shares::DeleteService) .to receive(:new) - .with(user:, model: member_a) + .with(user:, model: member_a, contract_class: Shares::WorkPackages::DeleteContract) .and_return(service_instance_a) allow(Shares::DeleteService) .to receive(:new) - .with(user:, model: member_c) + .with(user:, model: member_c, contract_class: Shares::WorkPackages::DeleteContract) .and_return(service_instance_c) end @@ -121,7 +121,7 @@ expect(service_instance_a).to have_received(:call).with(no_args) expect(Shares::DeleteService) .not_to have_received(:new) - .with(user:, model: member_b) + .with(user:, model: member_b, contract_class: Shares::WorkPackages::DeleteContract) expect(service_instance_c).to have_received(:call).with(no_args) end @@ -179,11 +179,11 @@ before do allow(Shares::DeleteRoleService) .to receive(:new) - .with(user:, model: member_a) + .with(user:, model: member_a, contract_class: Shares::WorkPackages::DeleteContract) .and_return(service_instance_a) allow(Shares::DeleteRoleService) .to receive(:new) - .with(user:, model: member_c) + .with(user:, model: member_c, contract_class: Shares::WorkPackages::DeleteContract) .and_return(service_instance_c) end @@ -193,11 +193,11 @@ expect(service_instance_a).to have_received(:call).with(role_id: role.id.to_s) expect(Shares::DeleteRoleService) .not_to have_received(:new) - .with(user:, model: member_b) + .with(user:, model: member_b, contract_class: Shares::WorkPackages::DeleteContract) expect(service_instance_c).to have_received(:call).with(role_id: role.id.to_s) expect(Shares::DeleteRoleService) .not_to have_received(:new) - .with(user:, model: member_d) + .with(user:, model: member_d, contract_class: Shares::WorkPackages::DeleteContract) end context "when all calls succeed" do diff --git a/spec/services/shares/create_or_update_service_spec.rb b/spec/services/shares/create_or_update_service_spec.rb index b2a9684f5779..6793f81e57d3 100644 --- a/spec/services/shares/create_or_update_service_spec.rb +++ b/spec/services/shares/create_or_update_service_spec.rb @@ -34,7 +34,9 @@ let(:user) { build_stubbed(:user) } let(:role) { build_stubbed(:view_work_package_role) } let(:work_package) { build_stubbed(:work_package) } - let(:instance) { described_class.new(user:) } + let(:create_contract_class) { class_double(Shares::WorkPackages::CreateContract) } + let(:update_contract_class) { class_double(Shares::WorkPackages::UpdateContract) } + let(:instance) { described_class.new(user:, create_contract_class:, update_contract_class:) } let(:params) { { user_id: user, roles: [role], entity: work_package } } let(:service_result) { instance_double(ServiceResult) } @@ -48,12 +50,16 @@ .and_return(existing_member) end - context "when the user has no work_package_member for that work package" do - let(:create_instance) { instance_double(WorkPackageMembers::CreateService) } + context "when the user is not a member of the shared entity" do + let(:create_instance) { instance_double(Shares::CreateService) } let(:existing_member) { nil } - it "calls the WorkPackageMembers::CreateService" do - allow(WorkPackageMembers::CreateService).to receive(:new).and_return(create_instance) + it "calls the Shares::CreateService" do + allow(Shares::CreateService).to receive(:new).with( + contract_class: create_contract_class, + contract_options: {}, + user: + ).and_return(create_instance) allow(create_instance).to receive(:call).and_return(service_result) service_call @@ -64,12 +70,17 @@ end end - context "when the user already has a work_package_member for that work package" do - let(:update_instance) { instance_double(WorkPackageMembers::UpdateService) } + context "when the user is already a member of the shared entity" do + let(:update_instance) { instance_double(Shares::UpdateService) } let(:existing_member) { build_stubbed(:work_package_member) } - it "calls the WorkPackageMembers::UpdateService" do - allow(WorkPackageMembers::UpdateService).to receive(:new).and_return(update_instance) + it "calls the Shares::UpdateService" do + allow(Shares::UpdateService).to receive(:new).with( + contract_class: update_contract_class, + contract_options: {}, + model: existing_member, + user: + ).and_return(update_instance) allow(update_instance).to receive(:call).and_return(service_result) service_call diff --git a/spec/services/shares/create_service_spec.rb b/spec/services/shares/create_service_spec.rb index df44385f1c95..058974522727 100644 --- a/spec/services/shares/create_service_spec.rb +++ b/spec/services/shares/create_service_spec.rb @@ -70,6 +70,7 @@ def stub_notifications before { stub_notifications } it_behaves_like "BaseServices create service" do + let(:factory) { :work_package_member } let(:model_class) { Member } let(:principal) { richard } let(:call_attributes) { { principal:, roles: [role], entity: work_package, project: work_package.project } } diff --git a/spec/services/shares/delete_role_service_spec.rb b/spec/services/shares/delete_role_service_spec.rb index d1eb9bf02206..b5ebce34b51c 100644 --- a/spec/services/shares/delete_role_service_spec.rb +++ b/spec/services/shares/delete_role_service_spec.rb @@ -33,6 +33,7 @@ RSpec.describe Shares::DeleteRoleService, type: :model do it_behaves_like "BaseServices delete service" do + let(:factory) { :work_package_member } let(:model_class) { Member } let(:model_instance) { build_stubbed(:work_package_member, principal:) } let(:principal) { user } diff --git a/spec/services/shares/delete_service_spec.rb b/spec/services/shares/delete_service_spec.rb index 6fc39360a696..0df47449fb8d 100644 --- a/spec/services/shares/delete_service_spec.rb +++ b/spec/services/shares/delete_service_spec.rb @@ -33,6 +33,7 @@ RSpec.describe Shares::DeleteService, type: :model do it_behaves_like "BaseServices delete service" do + let(:factory) { :work_package_member } let(:model_class) { Member } let(:model_instance) { build_stubbed(:work_package_member, principal:) } let(:principal) { user } diff --git a/spec/services/shares/set_attributes_service_spec.rb b/spec/services/shares/set_attributes_service_spec.rb index 96112b9300d4..874f97e5d2dd 100644 --- a/spec/services/shares/set_attributes_service_spec.rb +++ b/spec/services/shares/set_attributes_service_spec.rb @@ -43,7 +43,7 @@ .with(member, user, options: {}) .and_return(contract_instance) - Shares::WorkPackages + Shares::WorkPackages::CreateContract end let(:contract_instance) do instance_double(Shares::WorkPackages::CreateContract, validate: contract_valid, errors: contract_errors) From 21e03a2271f29696b357418c9ecab890c3844dc2 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Mon, 24 Jun 2024 14:41:55 +0200 Subject: [PATCH 19/33] Remove debug output --- app/components/shares/permission_button_component.html.erb | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/components/shares/permission_button_component.html.erb b/app/components/shares/permission_button_component.html.erb index f26107091665..08f13d687fb3 100644 --- a/app/components/shares/permission_button_component.html.erb +++ b/app/components/shares/permission_button_component.html.erb @@ -11,10 +11,7 @@ permission_name(active_role.id) end - puts "*"*100, @available_roles, "*"*100 - @available_roles.each do |role_hash| - puts "### role_hash: #{role_hash} ###" menu.with_item(label: role_hash[:label], href: update_path, method: :patch, From b318231c202f80d5cfb5064adbe488cb819c47ba Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Mon, 24 Jun 2024 11:38:34 -0500 Subject: [PATCH 20/33] Inject permission checks from controller --- app/components/shares/counter_component.rb | 8 ++- .../invite_user_form_component.html.erb | 4 +- .../shares/invite_user_form_component.rb | 9 ++- .../shares/modal_body_component.html.erb | 16 +++-- app/components/shares/modal_body_component.rb | 18 +++-- app/components/shares/share_row_component.rb | 5 +- .../shares/user_details_component.rb | 2 +- app/controllers/shares_controller.rb | 65 +++++++++++++++---- 8 files changed, 97 insertions(+), 30 deletions(-) diff --git a/app/components/shares/counter_component.rb b/app/components/shares/counter_component.rb index 5b22c7fa2b0f..9bd01472efbd 100644 --- a/app/components/shares/counter_component.rb +++ b/app/components/shares/counter_component.rb @@ -33,19 +33,23 @@ class CounterComponent < ApplicationComponent include ApplicationHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers - include Shares::Concerns::Authorization - def initialize(entity:, count:) + def initialize(entity:, + count:, + sharing_manageable:) super @entity = entity @count = count + @sharing_manageable = sharing_manageable end private attr_reader :entity, :count + def sharing_manageable? = @sharing_manageable + def shared_with_anyone_else_other_than_myself? Member.of_entity(@entity) .where.not(principal: User.current) diff --git a/app/components/shares/invite_user_form_component.html.erb b/app/components/shares/invite_user_form_component.html.erb index 963065e9b2b7..fa52ae08d800 100644 --- a/app/components/shares/invite_user_form_component.html.erb +++ b/app/components/shares/invite_user_form_component.html.erb @@ -1,11 +1,11 @@ <%= component_wrapper do - if sharing_manageable? + if @sharing_manageable primer_form_with( model: new_share, url: url_for([@entity, Member]), data: { controller: 'user-limit ' \ - 'work-packages--share--user-selected', + 'work-packages--share--user-selected', 'application-target': 'dynamic', 'user-limit-open-seats-value': OpenProject::Enterprise.open_seats_count, action: 'submit->work-packages--share--user-selected#ensureUsersSelected' } diff --git a/app/components/shares/invite_user_form_component.rb b/app/components/shares/invite_user_form_component.rb index abaca7db0689..48ae8e0715ed 100644 --- a/app/components/shares/invite_user_form_component.rb +++ b/app/components/shares/invite_user_form_component.rb @@ -31,14 +31,17 @@ class InviteUserFormComponent < ApplicationComponent # rubocop:disable OpenProje include ApplicationHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers - include Shares::Concerns::Authorization - def initialize(entity:, available_roles:, errors: nil) + def initialize(entity:, + available_roles:, + sharing_manageable:, + errors: nil) super @entity = entity - @errors = errors @available_roles = available_roles + @sharing_manageable = sharing_manageable + @errors = errors end def new_share diff --git a/app/components/shares/modal_body_component.html.erb b/app/components/shares/modal_body_component.html.erb index 3e8d3ad558c9..2ac2d17c80d1 100644 --- a/app/components/shares/modal_body_component.html.erb +++ b/app/components/shares/modal_body_component.html.erb @@ -2,7 +2,10 @@ component_wrapper(tag: 'turbo-frame') do flex_layout(data: { turbo: true }) do |modal_content| modal_content.with_row do - render(Shares::InviteUserFormComponent.new(entity: @entity, available_roles: @available_roles, errors: @errors)) + render(Shares::InviteUserFormComponent.new(entity: @entity, + available_roles: @available_roles, + sharing_manageable: @sharing_manageable, + errors: @errors)) end modal_content.with_row(mt: 3, @@ -13,7 +16,9 @@ border_box.with_header(color: :muted, data: { 'test-selector': 'op-share-wp-header' }) do grid_layout('op-share-wp-modal-body--header', tag: :div, align_items: :center) do |header_grid| header_grid.with_area(:counter, tag: :div) do - render(Shares::CounterComponent.new(entity: @entity, count: @shares.size)) + render(Shares::CounterComponent.new(entity: @entity, + count: @shares.size, + sharing_manageable: @sharing_manageable)) end header_grid.with_area(:actions, @@ -70,7 +75,7 @@ tag: :div, hidden: true, # Prevent flicker on initial render data: { 'work-packages--share--bulk-selection-target': 'bulkActions' }) do - if sharing_manageable? + if @sharing_manageable concat( render(Shares::BulkPermissionButtonComponent.new(entity: @entity, available_roles: @available_roles)) ) @@ -107,7 +112,10 @@ end else @shares.each do |share| - render(Shares::ShareRowComponent.new(share: share, available_roles: @available_roles, container: border_box)) + render(Shares::ShareRowComponent.new(share: share, + available_roles: @available_roles, + sharing_manageable: @sharing_manageable, + container: border_box)) end end end diff --git a/app/components/shares/modal_body_component.rb b/app/components/shares/modal_body_component.rb index 72cfc8503515..064cdf4efade 100644 --- a/app/components/shares/modal_body_component.rb +++ b/app/components/shares/modal_body_component.rb @@ -32,17 +32,25 @@ class ModalBodyComponent < ApplicationComponent # rubocop:disable OpenProject/Ad include MemberHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers - include Shares::Concerns::Authorization - attr_reader :entity, :shares, :errors, :available_roles - - def initialize(entity:, shares:, available_roles:, errors: nil) + attr_reader :entity, + :shares, + :available_roles, + :sharing_manageable, + :errors + + def initialize(entity:, + shares:, + available_roles:, + sharing_manageable:, + errors: nil) super @entity = entity @shares = shares - @errors = errors @available_roles = available_roles + @sharing_manageable = sharing_manageable + @errors = errors end def self.wrapper_key diff --git a/app/components/shares/share_row_component.rb b/app/components/shares/share_row_component.rb index bb981e146ef4..3c6b094698ef 100644 --- a/app/components/shares/share_row_component.rb +++ b/app/components/shares/share_row_component.rb @@ -33,10 +33,10 @@ class ShareRowComponent < ApplicationComponent # rubocop:disable OpenProject/Add include ApplicationHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers - include Shares::Concerns::Authorization def initialize(share:, available_roles:, + sharing_manageable:, container: nil) super @@ -44,6 +44,7 @@ def initialize(share:, @entity = share.entity @principal = share.principal @available_roles = available_roles + @sharing_manageable = sharing_manageable @container = container end @@ -59,6 +60,8 @@ def share_editable? @share_editable ||= User.current != share.principal && sharing_manageable? end + def sharing_manageable? = @sharing_manageable + def grid_css_classes if sharing_manageable? "op-share-wp-modal-body--user-row_manageable" diff --git a/app/components/shares/user_details_component.rb b/app/components/shares/user_details_component.rb index 0003900998a1..8f189f2b6dbf 100644 --- a/app/components/shares/user_details_component.rb +++ b/app/components/shares/user_details_component.rb @@ -34,7 +34,7 @@ class UserDetailsComponent < ApplicationComponent # rubocop:disable OpenProject/ include OpPrimer::ComponentHelpers def initialize(share:, - manager_mode: User.current.allowed_in_project?(:share_work_packages, share.project), + manager_mode:, invite_resent: false) super diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 78f6167c8bb6..f3a89fab62a3 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -42,7 +42,12 @@ def index flash.now[:error] = query.errors.full_messages end - render Shares::ModalBodyComponent.new(entity: @entity, shares: @shares, errors: @errors, available_roles:), layout: nil + render Shares::ModalBodyComponent.new(entity: @entity, + shares: @shares, + errors: @errors, + sharing_manageable: User.current.allowed_in_project?(:share_work_packages, + @entity.project), + available_roles:), layout: nil end def create @@ -132,8 +137,8 @@ def enterprise_check def destroy_share(share) Shares::DeleteService - .new(user: current_user, model: share, contract_class: sharing_contract_scope::DeleteContract) - .call + .new(user: current_user, model: share, contract_class: sharing_contract_scope::DeleteContract) + .call end def create_or_update_share(user_id, role_ids) @@ -151,6 +156,8 @@ def respond_with_replace_modal component: Shares::ModalBodyComponent.new(entity: @entity, available_roles:, shares: @new_shares || load_shares, + sharing_manageable: User.current.allowed_in_project?(:share_work_packages, + @project), errors: @errors) ) @@ -159,18 +166,31 @@ def respond_with_replace_modal def respond_with_prepend_shares replace_via_turbo_stream( - component: Shares::InviteUserFormComponent.new(entity: @entity, available_roles:, errors: @errors) + component: Shares::InviteUserFormComponent.new(entity: @entity, + available_roles:, + sharing_manageable: User.current.allowed_in_project?(:share_work_packages, + @project), + errors: @errors) ) update_via_turbo_stream( - component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count) + component: Shares::CounterComponent.new(entity: @entity, + count: current_visible_member_count, + sharing_manageable: User.current.allowed_in_project?(:share_work_packages, + @project)) ) @new_shares.each do |share| prepend_via_turbo_stream( - component: Shares::ShareRowComponent.new(share:, available_roles:), + component: Shares::ShareRowComponent.new(share:, + available_roles:, + sharing_manageable: User.current.allowed_in_project?(:share_work_packages, + @project)), target_component: Shares::ModalBodyComponent.new(entity: @entity, available_roles:, + sharing_manageable: User.current.allowed_in_project?( + :share_work_packages, @project + ), shares: load_shares, errors: @errors) ) @@ -182,6 +202,9 @@ def respond_with_prepend_shares def respond_with_new_invite_form replace_via_turbo_stream(component: Shares::InviteUserFormComponent.new(entity: @entity, available_roles:, + sharing_manageable: User.current.allowed_in_project?( + :share_work_packages, @project + ), errors: @errors)) respond_with_turbo_streams @@ -196,14 +219,26 @@ def respond_with_update_permission_button end def respond_with_remove_share - remove_via_turbo_stream(component: Shares::ShareRowComponent.new(share: @share, available_roles:)) - update_via_turbo_stream(component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count)) + remove_via_turbo_stream(component: Shares::ShareRowComponent.new(share: @share, + available_roles:, + sharing_manageable: User.current.allowed_in_project?( + :share_work_packages, @project + ))) + update_via_turbo_stream(component: Shares::CounterComponent.new(entity: @entity, + count: current_visible_member_count, + sharing_manageable: User.current.allowed_in_project?( + :share_work_packages, @project + ))) respond_with_turbo_streams end def respond_with_update_user_details - update_via_turbo_stream(component: Shares::UserDetailsComponent.new(share: @share, invite_resent: true)) + update_via_turbo_stream(component: Shares::UserDetailsComponent.new(share: @share, + manager_mode: User.current.allowed_in_project?( + :share_work_packages, @entity.project + ), + invite_resent: true)) respond_with_turbo_streams end @@ -223,12 +258,18 @@ def respond_with_bulk_updated_permission_buttons def respond_with_bulk_removed_shares @selected_shares.each do |share| remove_via_turbo_stream( - component: Shares::ShareRowComponent.new(share:, available_roles:) + component: Shares::ShareRowComponent.new(share:, + available_roles:, + sharing_manageable: User.current.allowed_in_project?(:share_work_packages, + @project)) ) end update_via_turbo_stream( - component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count) + component: Shares::CounterComponent.new(entity: @entity, + count: current_visible_member_count, + sharing_manageable: User.current.allowed_in_project?(:share_work_packages, + @project)) ) respond_with_turbo_streams @@ -237,7 +278,7 @@ def respond_with_bulk_removed_shares def load_entity @entity = if params["work_package_id"] WorkPackage.visible.find(params["work_package_id"]) - # TODO: Add support for other entities + # TODO: Add support for other entities else raise ArgumentError, <<~ERROR Nested the SharesController under an entity controller that is not yet configured to support sharing. From 36f6685a803c8f00e2a5604bf0db1512185d306f Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Mon, 24 Jun 2024 15:29:11 -0500 Subject: [PATCH 21/33] Extract permission check for share management to controller concern --- .../shares/work_packages}/authorization.rb | 14 +++++-- app/controllers/shares_controller.rb | 42 ++++++------------- 2 files changed, 24 insertions(+), 32 deletions(-) rename app/{components/shares/concerns => controllers/concerns/shares/work_packages}/authorization.rb (78%) diff --git a/app/components/shares/concerns/authorization.rb b/app/controllers/concerns/shares/work_packages/authorization.rb similarity index 78% rename from app/components/shares/concerns/authorization.rb rename to app/controllers/concerns/shares/work_packages/authorization.rb index 6c5041866a9d..32b4bf5f34e0 100644 --- a/app/components/shares/concerns/authorization.rb +++ b/app/controllers/concerns/shares/work_packages/authorization.rb @@ -2,7 +2,7 @@ # -- copyright # OpenProject is an open source project management software. -# Copyright (C) 2023 the OpenProject GmbH +# Copyright (C) 2010-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. @@ -29,14 +29,22 @@ # ++ module Shares - module Concerns + module WorkPackages module Authorization extend ActiveSupport::Concern included do def sharing_manageable? # TODO: Fix this to check based on the entity - User.current.allowed_in_project?(:share_work_packages, @entity.project) + case @entity + when WorkPackage + User.current.allowed_in_project?(:share_work_packages, @entity.project) + else + raise ArgumentError, <<~ERROR + Checking sharing capabilities for an unsupported entity: + - #{@entity.class} + ERROR + end end end end diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index f3a89fab62a3..efb79c77f520 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -28,6 +28,7 @@ class SharesController < ApplicationController include OpTurbo::ComponentStream + include Shares::WorkPackages::Authorization include MemberHelper before_action :load_entity @@ -45,8 +46,7 @@ def index render Shares::ModalBodyComponent.new(entity: @entity, shares: @shares, errors: @errors, - sharing_manageable: User.current.allowed_in_project?(:share_work_packages, - @entity.project), + sharing_manageable: sharing_manageable?, available_roles:), layout: nil end @@ -156,8 +156,7 @@ def respond_with_replace_modal component: Shares::ModalBodyComponent.new(entity: @entity, available_roles:, shares: @new_shares || load_shares, - sharing_manageable: User.current.allowed_in_project?(:share_work_packages, - @project), + sharing_manageable: sharing_manageable?, errors: @errors) ) @@ -168,29 +167,24 @@ def respond_with_prepend_shares replace_via_turbo_stream( component: Shares::InviteUserFormComponent.new(entity: @entity, available_roles:, - sharing_manageable: User.current.allowed_in_project?(:share_work_packages, - @project), + sharing_manageable: sharing_manageable?, errors: @errors) ) update_via_turbo_stream( component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count, - sharing_manageable: User.current.allowed_in_project?(:share_work_packages, - @project)) + sharing_manageable: sharing_manageable?) ) @new_shares.each do |share| prepend_via_turbo_stream( component: Shares::ShareRowComponent.new(share:, available_roles:, - sharing_manageable: User.current.allowed_in_project?(:share_work_packages, - @project)), + sharing_manageable: sharing_manageable?), target_component: Shares::ModalBodyComponent.new(entity: @entity, available_roles:, - sharing_manageable: User.current.allowed_in_project?( - :share_work_packages, @project - ), + sharing_manageable: sharing_manageable?, shares: load_shares, errors: @errors) ) @@ -202,9 +196,7 @@ def respond_with_prepend_shares def respond_with_new_invite_form replace_via_turbo_stream(component: Shares::InviteUserFormComponent.new(entity: @entity, available_roles:, - sharing_manageable: User.current.allowed_in_project?( - :share_work_packages, @project - ), + sharing_manageable: sharing_manageable?, errors: @errors)) respond_with_turbo_streams @@ -221,23 +213,17 @@ def respond_with_update_permission_button def respond_with_remove_share remove_via_turbo_stream(component: Shares::ShareRowComponent.new(share: @share, available_roles:, - sharing_manageable: User.current.allowed_in_project?( - :share_work_packages, @project - ))) + sharing_manageable: sharing_manageable?)) update_via_turbo_stream(component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count, - sharing_manageable: User.current.allowed_in_project?( - :share_work_packages, @project - ))) + sharing_manageable: sharing_manageable?)) respond_with_turbo_streams end def respond_with_update_user_details update_via_turbo_stream(component: Shares::UserDetailsComponent.new(share: @share, - manager_mode: User.current.allowed_in_project?( - :share_work_packages, @entity.project - ), + manager_mode: sharing_manageable?, invite_resent: true)) respond_with_turbo_streams @@ -260,16 +246,14 @@ def respond_with_bulk_removed_shares remove_via_turbo_stream( component: Shares::ShareRowComponent.new(share:, available_roles:, - sharing_manageable: User.current.allowed_in_project?(:share_work_packages, - @project)) + sharing_manageable: sharing_manageable?) ) end update_via_turbo_stream( component: Shares::CounterComponent.new(entity: @entity, count: current_visible_member_count, - sharing_manageable: User.current.allowed_in_project?(:share_work_packages, - @project)) + sharing_manageable: sharing_manageable?) ) respond_with_turbo_streams From fce40dc813dce9da9a993d2ac7ac3a482f129bc5 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 25 Jun 2024 08:36:20 +0200 Subject: [PATCH 22/33] Move stimulus controller out of the work packages namespace --- .../share => shares}/bulk-selection.controller.ts | 2 +- .../{work-packages/share => shares}/user-selected.controller.ts | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename frontend/src/stimulus/controllers/dynamic/{work-packages/share => shares}/bulk-selection.controller.ts (98%) rename frontend/src/stimulus/controllers/dynamic/{work-packages/share => shares}/user-selected.controller.ts (100%) diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/share/bulk-selection.controller.ts b/frontend/src/stimulus/controllers/dynamic/shares/bulk-selection.controller.ts similarity index 98% rename from frontend/src/stimulus/controllers/dynamic/work-packages/share/bulk-selection.controller.ts rename to frontend/src/stimulus/controllers/dynamic/shares/bulk-selection.controller.ts index f6d119154bc5..3e01bc147cf7 100644 --- a/frontend/src/stimulus/controllers/dynamic/work-packages/share/bulk-selection.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/shares/bulk-selection.controller.ts @@ -201,7 +201,7 @@ export default class BulkSelectionController extends Controller { hiddenInput.type = 'hidden'; hiddenInput.name = 'share_ids[]'; hiddenInput.value = checkbox.value; - hiddenInput.setAttribute('data-work-packages--share--bulk-selection-target', 'hiddenShare'); + hiddenInput.setAttribute('data-shares--bulk-selection-target', 'hiddenShare'); return hiddenInput; }); diff --git a/frontend/src/stimulus/controllers/dynamic/work-packages/share/user-selected.controller.ts b/frontend/src/stimulus/controllers/dynamic/shares/user-selected.controller.ts similarity index 100% rename from frontend/src/stimulus/controllers/dynamic/work-packages/share/user-selected.controller.ts rename to frontend/src/stimulus/controllers/dynamic/shares/user-selected.controller.ts From b0fcda8c36c57196e4ba45f3d4a4fc77f6fc1733 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 25 Jun 2024 08:36:45 +0200 Subject: [PATCH 23/33] `op-whare-wp` -> `op-share-dialog` and use renamed controller --- .../bulk_permission_button_component.html.erb | 8 ++-- .../bulk_selection_counter_component.html.erb | 8 ++-- .../shares/counter_component.html.erb | 2 +- .../invite_user_form_component.html.erb | 14 +++---- .../shares/modal_body_component.html.erb | 24 +++++------ app/components/shares/modal_body_component.rb | 2 +- .../shares/modal_body_component.sass | 2 +- .../permission_button_component.html.erb | 2 +- .../shares/share_row_component.html.erb | 10 ++--- app/components/shares/share_row_component.rb | 4 +- app/controllers/shares_controller.rb | 4 +- app/forms/shares/invitee.rb | 6 +-- .../wp-share-modal/wp-share.modal.html | 4 +- .../wp-share-modal/wp-share.modal.sass | 2 +- ...progress-popover-edit-field.component.html | 4 +- .../modal_preview/status_based.html.erb | 2 +- .../modal_preview/work_based.html.erb | 2 +- .../components/work_packages/share_modal.rb | 42 +++++++++---------- 18 files changed, 71 insertions(+), 71 deletions(-) diff --git a/app/components/shares/bulk_permission_button_component.html.erb b/app/components/shares/bulk_permission_button_component.html.erb index b0eacc5f312f..32acee800a79 100644 --- a/app/components/shares/bulk_permission_button_component.html.erb +++ b/app/components/shares/bulk_permission_button_component.html.erb @@ -3,8 +3,8 @@ dynamic_label: true, anchor_align: :end, color: :subtle, - data: { test_selector: 'op-share-wp-bulk-update-role'})) do |menu| - menu.with_show_button(scheme: :invisible, color: :subtle, data: { 'work-packages--share--bulk-selection-target': 'bulkUpdateRoleLabel' }) do |button| + data: { test_selector: 'op-share-dialog-bulk-update-role'})) do |menu| + menu.with_show_button(scheme: :invisible, color: :subtle, data: { 'shares--bulk-selection-target': 'bulkUpdateRoleLabel' }) do |button| button.with_trailing_action_icon(icon: "triangle-down") 'Placeholder' end @@ -18,9 +18,9 @@ method: :patch, name: 'role_ids[]', value: role_hash[:value], - data: { 'work-packages--share--bulk-selection-target': 'bulkForm bulkUpdateRoleForm', + data: { 'shares--bulk-selection-target': 'bulkForm bulkUpdateRoleForm', 'role-name': role_hash[:label], - 'test-selector': "op-share-wp-bulk-update-role-permission-#{role_hash[:label]}" } + 'test-selector': "op-share-dialog-bulk-update-role-permission-#{role_hash[:label]}" } }) do |item| item.with_description.with_content(role_hash[:description]) end diff --git a/app/components/shares/bulk_selection_counter_component.html.erb b/app/components/shares/bulk_selection_counter_component.html.erb index f3542f12e65c..7066f1e08f6e 100644 --- a/app/components/shares/bulk_selection_counter_component.html.erb +++ b/app/components/shares/bulk_selection_counter_component.html.erb @@ -4,18 +4,18 @@ value: nil, label: I18n.t('work_package.sharing.label_toggle_all'), visually_hide_label: true, - data: { 'work-packages--share--bulk-selection-target': 'toggleAll', - action: 'work-packages--share--bulk-selection#toggle' })) + data: { 'shares--bulk-selection-target': 'toggleAll', + action: 'shares--bulk-selection#toggle' })) ) concat( - render(Primer::Beta::Text.new(ml: 2, data: { 'work-packages--share--bulk-selection-target': 'sharedCounter' })) do + render(Primer::Beta::Text.new(ml: 2, data: { 'shares--bulk-selection-target': 'sharedCounter' })) do I18n.t('work_package.sharing.count', count:) end ) # Text contents managed by Stimulus controller concat( - render(Primer::Beta::Text.new(ml: 2, data: { 'work-packages--share--bulk-selection-target': 'selectedCounter' })) + render(Primer::Beta::Text.new(ml: 2, data: { 'shares--bulk-selection-target': 'selectedCounter' })) ) %> diff --git a/app/components/shares/counter_component.html.erb b/app/components/shares/counter_component.html.erb index c2c1bfec88ff..e9281848a582 100644 --- a/app/components/shares/counter_component.html.erb +++ b/app/components/shares/counter_component.html.erb @@ -1,5 +1,5 @@ <%= - component_wrapper(data: { test_selector: 'op-share-wp-active-count'}) do + component_wrapper(data: { test_selector: 'op-share-dialog-active-count'}) do render(Primer::Box.new(display: :flex, aligns_items: :center)) do # There's no point in rendering the BulkSelectionCounterComponent even if # I'm able to manage shares if the only user that the work package is diff --git a/app/components/shares/invite_user_form_component.html.erb b/app/components/shares/invite_user_form_component.html.erb index fa52ae08d800..605a51d8f0e6 100644 --- a/app/components/shares/invite_user_form_component.html.erb +++ b/app/components/shares/invite_user_form_component.html.erb @@ -5,10 +5,10 @@ model: new_share, url: url_for([@entity, Member]), data: { controller: 'user-limit ' \ - 'work-packages--share--user-selected', + 'shares--user-selected', 'application-target': 'dynamic', 'user-limit-open-seats-value': OpenProject::Enterprise.open_seats_count, - action: 'submit->work-packages--share--user-selected#ensureUsersSelected' } + action: 'submit->shares--user-selected#ensureUsersSelected' } ) do |form| grid_layout('invite-user-form', tag: :div) do |invite_form| @@ -21,7 +21,7 @@ share: new_share, available_roles: @available_roles, form_arguments: { builder: form, name: "role_id" }, - data: { 'test-selector': 'op-share-wp-invite-role' }) + data: { 'test-selector': 'op-share-dialog-invite-role' }) ) end @@ -34,7 +34,7 @@ if OpenProject::Enterprise.user_limit.present? invite_form.with_area('userLimitWarning', data: { 'user-limit-target': 'limitWarning', - 'test-selector': 'op-share-wp-user-limit' }, + 'test-selector': 'op-share-dialog-user-limit' }, display: :none) do flex_layout do |user_limit_row| user_limit_row.with_column(mr: 2) do @@ -54,8 +54,8 @@ end invite_form.with_area('userSelectedWarning', - data: { 'work-packages--share--user-selected-target': 'error', - 'test-selector': 'op-share-wp-no-user-selected' }, + data: { 'shares--user-selected-target': 'error', + 'test-selector': 'op-share-dialog-no-user-selected' }, display: :none) do flex_layout do |no_selected_user_row| no_selected_user_row.with_column(mr: 2) do @@ -72,7 +72,7 @@ if @errors.present? invite_form.with_area('errors', - data: { 'test-selector': 'op-share-wp-error-message' }) do + data: { 'test-selector': 'op-share-dialog-error-message' }) do flex_layout do |error_rows| @errors.full_messages.each do |error_message| error_rows.with_row do diff --git a/app/components/shares/modal_body_component.html.erb b/app/components/shares/modal_body_component.html.erb index 2ac2d17c80d1..63a2929c2227 100644 --- a/app/components/shares/modal_body_component.html.erb +++ b/app/components/shares/modal_body_component.html.erb @@ -9,12 +9,12 @@ end modal_content.with_row(mt: 3, - data: { 'test-selector': 'op-share-wp-active-list', - controller: 'work-packages--share--bulk-selection', + data: { 'test-selector': 'op-share-dialog-active-list', + controller: 'shares--bulk-selection', application_target: 'dynamic' }) do render(border_box_container(list_id: insert_target_modifier_id)) do |border_box| - border_box.with_header(color: :muted, data: { 'test-selector': 'op-share-wp-header' }) do - grid_layout('op-share-wp-modal-body--header', tag: :div, align_items: :center) do |header_grid| + border_box.with_header(color: :muted, data: { 'test-selector': 'op-share-dialog-header' }) do + grid_layout('op-share-dialog-modal-body--header', tag: :div, align_items: :center) do |header_grid| header_grid.with_area(:counter, tag: :div) do render(Shares::CounterComponent.new(entity: @entity, count: @shares.size, @@ -23,7 +23,7 @@ header_grid.with_area(:actions, tag: :div, - data: { 'work-packages--share--bulk-selection-target': 'defaultActions' }) do + data: { 'shares--bulk-selection-target': 'defaultActions' }) do flex_layout do |header_actions| header_actions.with_column(mr: 2) do render(Primer::Alpha::ActionMenu.new(anchor_align: :end, @@ -31,8 +31,8 @@ dynamic_label: true, dynamic_label_prefix: I18n.t('work_package.sharing.filter.type'), color: :muted, - data: { 'test-selector': 'op-share-wp-filter-type' })) do |menu| - menu.with_show_button(scheme: :invisible, color: :muted, data: { 'test-selector': 'op-share-wp-filter-type-button' }) do |button| + data: { 'test-selector': 'op-share-dialog-filter-type' })) do |menu| + menu.with_show_button(scheme: :invisible, color: :muted, data: { 'test-selector': 'op-share-dialog-filter-type-button' }) do |button| button.with_trailing_action_icon(icon: "triangle-down") I18n.t('work_package.sharing.filter.type') end @@ -53,8 +53,8 @@ dynamic_label: true, dynamic_label_prefix: I18n.t('work_package.sharing.filter.role'), color: :muted, - data: { 'test-selector': 'op-share-wp-filter-role' })) do |menu| - menu.with_show_button(scheme: :invisible, color: :muted, data: { 'test-selector': 'op-share-wp-filter-role-button' }) do |button| + data: { 'test-selector': 'op-share-dialog-filter-role' })) do |menu| + menu.with_show_button(scheme: :invisible, color: :muted, data: { 'test-selector': 'op-share-dialog-filter-role-button' }) do |button| button.with_trailing_action_icon(icon: "triangle-down") I18n.t('work_package.sharing.filter.role') end @@ -74,7 +74,7 @@ header_grid.with_area(:actions, tag: :div, hidden: true, # Prevent flicker on initial render - data: { 'work-packages--share--bulk-selection-target': 'bulkActions' }) do + data: { 'shares--bulk-selection-target': 'bulkActions' }) do if @sharing_manageable concat( render(Shares::BulkPermissionButtonComponent.new(entity: @entity, available_roles: @available_roles)) @@ -83,12 +83,12 @@ concat( form_with(url: url_for([:bulk, @entity, Member]), method: :delete, - data: { 'work-packages--share--bulk-selection-target': 'bulkForm' }) do + data: { 'shares--bulk-selection-target': 'bulkForm' }) do render(Primer::Beta::IconButton.new(icon: "trash", type: :submit, scheme: :danger, "aria-label": I18n.t('work_package.sharing.remove'), - test_selector: 'op-share-wp--bulk-remove')) + test_selector: 'op-share-dialog--bulk-remove')) end ) end diff --git a/app/components/shares/modal_body_component.rb b/app/components/shares/modal_body_component.rb index 064cdf4efade..e3ce85a99547 100644 --- a/app/components/shares/modal_body_component.rb +++ b/app/components/shares/modal_body_component.rb @@ -68,7 +68,7 @@ def insert_target_modified? end def insert_target_modifier_id - "op-share-wp-active-shares" + "op-share-dialog-active-shares" end def blankslate_config diff --git a/app/components/shares/modal_body_component.sass b/app/components/shares/modal_body_component.sass index 4ba5f5ee53c9..17f99bc4aab1 100644 --- a/app/components/shares/modal_body_component.sass +++ b/app/components/shares/modal_body_component.sass @@ -1,4 +1,4 @@ -.op-share-wp-modal-body +.op-share-dialog-modal-body &--user-row display: grid grid-template-columns: minmax(31px, auto) 1fr // 31px is the width needed to display a group avatar diff --git a/app/components/shares/permission_button_component.html.erb b/app/components/shares/permission_button_component.html.erb index 08f13d687fb3..6402891c82ad 100644 --- a/app/components/shares/permission_button_component.html.erb +++ b/app/components/shares/permission_button_component.html.erb @@ -4,7 +4,7 @@ dynamic_label: true, anchor_align: :end, color: :subtle }.deep_merge(@system_arguments))) do |menu| - menu.with_show_button(data: { 'work-packages--share--bulk-selection-target': 'userRowRole', + menu.with_show_button(data: { 'shares--bulk-selection-target': 'userRowRole', 'share-id': share.id, 'active-role-name': permission_name(active_role.id)}) do |button| button.with_trailing_action_icon(icon: :"triangle-down") diff --git a/app/components/shares/share_row_component.html.erb b/app/components/shares/share_row_component.html.erb index cb3eaba15cbe..c85ae11c0104 100644 --- a/app/components/shares/share_row_component.html.erb +++ b/app/components/shares/share_row_component.html.erb @@ -1,13 +1,13 @@ <%= - component_wrapper(:border_box_row, data: { 'test-selector': "op-share-wp-active-user-#{principal.id}" }) do + component_wrapper(:border_box_row, data: { 'test-selector': "op-share-dialog-active-user-#{principal.id}" }) do grid_layout(grid_css_classes, tag: :div, align_items: :center, classes: 'ellipsis') do |user_row_grid| user_row_grid.with_area(:selection, tag: :div) do if share_editable? render(Primer::Alpha::CheckBox.new(name: "share_ids", value: share.id, label: "#{principal.name}", visually_hide_label: true, scheme: :array, data: { - 'work-packages--share--bulk-selection-target': 'shareCheckbox', - action: 'work-packages--share--bulk-selection#refresh' + 'shares--bulk-selection-target': 'shareCheckbox', + action: 'shares--bulk-selection#refresh' })) end end @@ -24,7 +24,7 @@ user_row_grid.with_area(:button, tag: :div, color: :subtle) do render(Shares::PermissionButtonComponent.new(share:, available_roles: @available_roles, - data: { 'test-selector': 'op-share-wp-update-role' })) + data: { 'test-selector': 'op-share-dialog-update-role' })) end user_row_grid.with_area(:remove, tag: :div) do @@ -33,7 +33,7 @@ type: :submit, scheme: :danger, "aria-label": I18n.t('work_package.sharing.remove'), - test_selector: 'op-share-wp--remove')) + test_selector: 'op-share-dialog--remove')) end end end diff --git a/app/components/shares/share_row_component.rb b/app/components/shares/share_row_component.rb index 3c6b094698ef..8f9a1dd6e53f 100644 --- a/app/components/shares/share_row_component.rb +++ b/app/components/shares/share_row_component.rb @@ -64,9 +64,9 @@ def sharing_manageable? = @sharing_manageable def grid_css_classes if sharing_manageable? - "op-share-wp-modal-body--user-row_manageable" + "op-share-dialog-modal-body--user-row_manageable" else - "op-share-wp-modal-body--user-row" + "op-share-dialog-modal-body--user-row" end end diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index efb79c77f520..9f5f2fe07d22 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -205,7 +205,7 @@ def respond_with_new_invite_form def respond_with_update_permission_button replace_via_turbo_stream(component: Shares::PermissionButtonComponent.new(share: @share, available_roles:, - data: { "test-selector": "op-share-wp-update-role" })) + data: { "test-selector": "op-share-dialog-update-role" })) respond_with_turbo_streams end @@ -234,7 +234,7 @@ def respond_with_bulk_updated_permission_buttons replace_via_turbo_stream( component: Shares::PermissionButtonComponent.new(share:, available_roles:, - data: { "test-selector": "op-share-wp-update-role" }) + data: { "test-selector": "op-share-dialog-update-role" }) ) end diff --git a/app/forms/shares/invitee.rb b/app/forms/shares/invitee.rb index 4dd542a470c9..419c7f5d71b0 100644 --- a/app/forms/shares/invitee.rb +++ b/app/forms/shares/invitee.rb @@ -32,14 +32,14 @@ class Invitee < ApplicationForm name: :user_id, label: I18n.t("work_package.sharing.label_search"), visually_hide_label: true, - data: { "work-packages--share--user-limit-target": "autocompleter" }, + data: { "shares--user-limit-target": "autocompleter" }, autocomplete_options: { component: "opce-user-autocompleter", defaultData: false, - id: "op-share-wp-invite-autocomplete", + id: "op-share-dialog-invite-autocomplete", placeholder: I18n.t("work_package.sharing.label_search_placeholder"), data: { - "test-selector": "op-share-wp-invite-autocomplete" + "test-selector": "op-share-dialog-invite-autocomplete" }, url: ::API::V3::Utilities::PathHelper::ApiV3Path.principals, filters: [{ name: "type", operator: "=", values: %w[User Group] }, diff --git a/frontend/src/app/features/work-packages/components/wp-share-modal/wp-share.modal.html b/frontend/src/app/features/work-packages/components/wp-share-modal/wp-share.modal.html index 76cdc0ea4a02..f222871e6932 100644 --- a/frontend/src/app/features/work-packages/components/wp-share-modal/wp-share.modal.html +++ b/frontend/src/app/features/work-packages/components/wp-share-modal/wp-share.modal.html @@ -1,5 +1,5 @@
- + @@ -69,7 +69,7 @@ - + diff --git a/lookbook/previews/open_project/progress/modal_preview/status_based.html.erb b/lookbook/previews/open_project/progress/modal_preview/status_based.html.erb index 0964ac897048..1ffc6c8db93f 100644 --- a/lookbook/previews/open_project/progress/modal_preview/status_based.html.erb +++ b/lookbook/previews/open_project/progress/modal_preview/status_based.html.erb @@ -4,7 +4,7 @@
Date: Tue, 25 Jun 2024 14:07:22 +0200 Subject: [PATCH 25/33] Move I18n out of the work package scope and properly load permissions --- .../bulk_selection_counter_component.html.erb | 4 +- app/components/shares/counter_component.rb | 2 +- .../invite_user_form_component.html.erb | 11 ++- .../shares/modal_body_component.html.erb | 10 +- app/components/shares/modal_body_component.rb | 20 ++-- .../shares/modal_upsale_component.html.erb | 1 + .../shares/share_counter_component.html.erb | 2 +- .../shares/share_row_component.html.erb | 4 +- .../shares/user_details_component.html.erb | 26 ++--- app/controllers/shares_controller.rb | 17 ++-- config/locales/en.yml | 98 ++++++++++--------- 11 files changed, 100 insertions(+), 95 deletions(-) diff --git a/app/components/shares/bulk_selection_counter_component.html.erb b/app/components/shares/bulk_selection_counter_component.html.erb index 7066f1e08f6e..9c45cb13e06b 100644 --- a/app/components/shares/bulk_selection_counter_component.html.erb +++ b/app/components/shares/bulk_selection_counter_component.html.erb @@ -2,7 +2,7 @@ concat( render(Primer::Alpha::CheckBox.new(name: 'toggle_all', value: nil, - label: I18n.t('work_package.sharing.label_toggle_all'), + label: I18n.t('sharing.label_toggle_all'), visually_hide_label: true, data: { 'shares--bulk-selection-target': 'toggleAll', action: 'shares--bulk-selection#toggle' })) @@ -10,7 +10,7 @@ concat( render(Primer::Beta::Text.new(ml: 2, data: { 'shares--bulk-selection-target': 'sharedCounter' })) do - I18n.t('work_package.sharing.count', count:) + I18n.t('sharing.count', count:) end ) diff --git a/app/components/shares/counter_component.rb b/app/components/shares/counter_component.rb index 9bd01472efbd..145ee3b8c530 100644 --- a/app/components/shares/counter_component.rb +++ b/app/components/shares/counter_component.rb @@ -29,7 +29,7 @@ # ++ module Shares - class CounterComponent < ApplicationComponent + class CounterComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include ApplicationHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers diff --git a/app/components/shares/invite_user_form_component.html.erb b/app/components/shares/invite_user_form_component.html.erb index 605a51d8f0e6..917b9c8ce542 100644 --- a/app/components/shares/invite_user_form_component.html.erb +++ b/app/components/shares/invite_user_form_component.html.erb @@ -27,7 +27,7 @@ invite_form.with_area('submit') do render(Primer::Beta::Button.new(scheme: :primary, type: :submit)) do - I18n.t('work_package.sharing.share') + I18n.t('sharing.share') end end @@ -44,8 +44,9 @@ user_limit_row.with_column do render(Primer::Beta::Text.new(color: :danger)) do I18n.t( - "work_package.sharing.warning_user_limit_reached#{'_admin' if User.current.admin?}", - upgrade_url: OpenProject::Enterprise.upgrade_url + "sharing.warning_user_limit_reached#{'_admin' if User.current.admin?}", + upgrade_url: OpenProject::Enterprise.upgrade_url, + entity: @entity.model_name.human ).html_safe end end @@ -64,7 +65,7 @@ no_selected_user_row.with_column do render(Primer::Beta::Text.new(color: :danger)) do - I18n.t("work_package.sharing.warning_no_selected_user") + I18n.t("sharing.warning_no_selected_user", entity: @entity.model_name.human) end end end @@ -95,7 +96,7 @@ end end else - render(Primer::Alpha::Banner.new(icon: :info)) { I18n.t('work_package.sharing.permissions.denied') } + render(Primer::Alpha::Banner.new(icon: :info)) { I18n.t('sharing.denied', entity: @entity.model_name.human(count: 2)) } end end %> diff --git a/app/components/shares/modal_body_component.html.erb b/app/components/shares/modal_body_component.html.erb index 63a2929c2227..97de19aeae13 100644 --- a/app/components/shares/modal_body_component.html.erb +++ b/app/components/shares/modal_body_component.html.erb @@ -29,12 +29,12 @@ render(Primer::Alpha::ActionMenu.new(anchor_align: :end, select_variant: :single, dynamic_label: true, - dynamic_label_prefix: I18n.t('work_package.sharing.filter.type'), + dynamic_label_prefix: I18n.t('sharing.filter.type'), color: :muted, data: { 'test-selector': 'op-share-dialog-filter-type' })) do |menu| menu.with_show_button(scheme: :invisible, color: :muted, data: { 'test-selector': 'op-share-dialog-filter-type-button' }) do |button| button.with_trailing_action_icon(icon: "triangle-down") - I18n.t('work_package.sharing.filter.type') + I18n.t('sharing.filter.type') end type_filter_options.each do |option| menu.with_item(label: option[:label], @@ -51,12 +51,12 @@ render(Primer::Alpha::ActionMenu.new(anchor_align: :end, select_variant: :single, dynamic_label: true, - dynamic_label_prefix: I18n.t('work_package.sharing.filter.role'), + dynamic_label_prefix: I18n.t('sharing.filter.role'), color: :muted, data: { 'test-selector': 'op-share-dialog-filter-role' })) do |menu| menu.with_show_button(scheme: :invisible, color: :muted, data: { 'test-selector': 'op-share-dialog-filter-role-button' }) do |button| button.with_trailing_action_icon(icon: "triangle-down") - I18n.t('work_package.sharing.filter.role') + I18n.t('sharing.filter.role') end @available_roles.each do |role_hash| menu.with_item(label: role_hash[:label], @@ -87,7 +87,7 @@ render(Primer::Beta::IconButton.new(icon: "trash", type: :submit, scheme: :danger, - "aria-label": I18n.t('work_package.sharing.remove'), + "aria-label": I18n.t('sharing.remove'), test_selector: 'op-share-dialog--bulk-remove')) end ) diff --git a/app/components/shares/modal_body_component.rb b/app/components/shares/modal_body_component.rb index 2bd0b31a578a..741c2fd5eba2 100644 --- a/app/components/shares/modal_body_component.rb +++ b/app/components/shares/modal_body_component.rb @@ -75,12 +75,12 @@ def blankslate_config @blankslate_config ||= {}.tap do |config| if params[:filters].blank? config[:icon] = :people - config[:heading_text] = I18n.t("work_package.sharing.text_empty_state_header") - config[:description_text] = I18n.t("work_package.sharing.text_empty_state_description") + config[:heading_text] = I18n.t("sharing.text_empty_state_header") + config[:description_text] = I18n.t("sharing.text_empty_state_description", entity: @entity.class.model_name.human) else config[:icon] = :search - config[:heading_text] = I18n.t("work_package.sharing.text_empty_search_header") - config[:description_text] = I18n.t("work_package.sharing.text_empty_search_description") + config[:heading_text] = I18n.t("sharing.text_empty_search_header") + config[:description_text] = I18n.t("sharing.text_empty_search_description") end end end @@ -88,19 +88,19 @@ def blankslate_config def type_filter_options if project_scoped_entity? [ - { label: I18n.t("work_package.sharing.filter.project_member"), + { label: I18n.t("sharing.filter.project_member"), value: { principal_type: "User", project_member: true } }, - { label: I18n.t("work_package.sharing.filter.not_project_member"), + { label: I18n.t("sharing.filter.not_project_member"), value: { principal_type: "User", project_member: false } }, - { label: I18n.t("work_package.sharing.filter.project_group"), + { label: I18n.t("sharing.filter.project_group"), value: { principal_type: "Group", project_member: true } }, - { label: I18n.t("work_package.sharing.filter.not_project_group"), + { label: I18n.t("sharing.filter.not_project_group"), value: { principal_type: "Group", project_member: false } } ] else [ - { label: "User", value: { principal_type: "User" } }, - { label: "Group", value: { principal_type: "Group" } } + { label: I18n.t("sharing.filter.user"), value: { principal_type: "User" } }, + { label: I18n.t("sharing.filter.group"), value: { principal_type: "Group" } } ] end diff --git a/app/components/shares/modal_upsale_component.html.erb b/app/components/shares/modal_upsale_component.html.erb index 2d5f3269a0d7..9f6cc4ae3e20 100644 --- a/app/components/shares/modal_upsale_component.html.erb +++ b/app/components/shares/modal_upsale_component.html.erb @@ -3,6 +3,7 @@ render Primer::Beta::Blankslate.new(border: true) do |component| component.with_visual_icon(icon: :'op-enterprise-addons', classes: 'upsale-colored') component.with_heading(tag: :h2, classes: 'upsale-colored').with_content(I18n.t(:label_enterprise_addon)) + # TODO: Generalize this text component.with_description { I18n.t('mail.sharing.work_packages.enterprise_text') } href = "#{OpenProject::Static::Links.links[:upsale][:href]}/?utm_source=unknown&utm_medium=community-edition&utm_campaign=work-package-sharing-modal" diff --git a/app/components/shares/share_counter_component.html.erb b/app/components/shares/share_counter_component.html.erb index 855f37330382..04f3ac8250c5 100644 --- a/app/components/shares/share_counter_component.html.erb +++ b/app/components/shares/share_counter_component.html.erb @@ -1,3 +1,3 @@ <% - concat(render(Primer::Beta::Text.new) { I18n.t('work_package.sharing.count', count:) }) + concat(render(Primer::Beta::Text.new) { I18n.t('sharing.count', count:) }) %> diff --git a/app/components/shares/share_row_component.html.erb b/app/components/shares/share_row_component.html.erb index c85ae11c0104..f633f52876b4 100644 --- a/app/components/shares/share_row_component.html.erb +++ b/app/components/shares/share_row_component.html.erb @@ -3,7 +3,7 @@ grid_layout(grid_css_classes, tag: :div, align_items: :center, classes: 'ellipsis') do |user_row_grid| user_row_grid.with_area(:selection, tag: :div) do if share_editable? - render(Primer::Alpha::CheckBox.new(name: "share_ids", value: share.id, label: "#{principal.name}", + render(Primer::Alpha::CheckBox.new(name: "share_ids", value: share.id, label: principal.name, visually_hide_label: true, scheme: :array, data: { 'shares--bulk-selection-target': 'shareCheckbox', @@ -32,7 +32,7 @@ render(Primer::Beta::IconButton.new(icon: "trash", type: :submit, scheme: :danger, - "aria-label": I18n.t('work_package.sharing.remove'), + "aria-label": I18n.t('sharing.remove'), test_selector: 'op-share-dialog--remove')) end end diff --git a/app/components/shares/user_details_component.html.erb b/app/components/shares/user_details_component.html.erb index 6ad99a059151..f9c64eaacb7d 100644 --- a/app/components/shares/user_details_component.html.erb +++ b/app/components/shares/user_details_component.html.erb @@ -9,20 +9,20 @@ if manager_mode? if user_is_a_group? if project_group? - render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.project_group")} + render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.project_group")} else - render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.not_project_group")} + render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.not_project_group")} end else if user_in_non_active_status? if user.locked? concat(render(Primer::Beta::Octicon.new(icon: :lock, color: :muted, mr: 1))) - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.locked") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.locked") }) elsif user.invited? if invite_resent? - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.invite_resent") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.invite_resent") }) else - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t('work_package.sharing.user_details.invited') }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t('sharing.user_details.invited') }) concat( form_with(url: resend_invite_path, method: :post) do render(Primer::Beta::Button.new(type: :submit, px: 0, scheme: :link)) { I18n.t('work_package.sharing.user_details.resend_invite') } @@ -34,31 +34,31 @@ if part_of_a_group? if part_of_a_shared_group? if project_member? - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.additional_privileges_project_or_group") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.additional_privileges_project_or_group") }) else - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.additional_privileges_group") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.additional_privileges_group") }) end else if inherited_project_member? - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.additional_privileges_project_or_group") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.additional_privileges_project_or_group") }) elsif project_member? - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.additional_privileges_project") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.additional_privileges_project") }) else - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.not_project_member") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.not_project_member") }) end end else if project_member? - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.additional_privileges_project") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.additional_privileges_project") }) else - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.not_project_member") }) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.not_project_member") }) end end end end else if user.invited? - concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("work_package.sharing.user_details.invited")}) + concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t("sharing.user_details.invited")}) end end end diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 9f5f2fe07d22..34fd4094c550 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -314,17 +314,18 @@ def load_selected_shares end def available_roles - # TODO: Optimize loading of roles - if @entity.is_a?(WorkPackage) + @available_roles ||= if @entity.is_a?(WorkPackage) + role_mapping = WorkPackageRole.unscoped.pluck(:builtin, :id).to_h + [ - { label: I18n.t("work_package.sharing.permissions.edit"), - value: WorkPackageRole.find_by(builtin: Role::BUILTIN_WORK_PACKAGE_EDITOR).id, + { label: I18n.t("work_package.permissions.edit"), + value: role_mapping[Role::BUILTIN_WORK_PACKAGE_EDITOR], description: I18n.t("work_package.sharing.permissions.edit_description") }, - { label: I18n.t("work_package.sharing.permissions.comment"), - value: WorkPackageRole.find_by(builtin: Role::BUILTIN_WORK_PACKAGE_COMMENTER).id, - description: I18n.t("work_package.sharing.permissions.comment_description") }, + { label: I18n.t("work_package.permissions.comment"), + value: role_mapping[Role::BUILTIN_WORK_PACKAGE_COMMENTER], + description: I18n.t("work_package.permissions.comment_description") }, { label: I18n.t("work_package.sharing.permissions.view"), - value: WorkPackageRole.find_by(builtin: Role::BUILTIN_WORK_PACKAGE_VIEWER).id, + value: role_mapping[Role::BUILTIN_WORK_PACKAGE_VIEWER], description: I18n.t("work_package.sharing.permissions.view_description"), default: true } ] diff --git a/config/locales/en.yml b/config/locales/en.yml index 80e31ee4641c..913f69703039 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3617,54 +3617,56 @@ Project attributes and sections are defined in the upgrade your plan to be able to add more users.' - warning_user_limit_reached: > - Adding additional users will exceed the current limit. Please contact an administrator to increase the user limit to ensure external users are able to access this work package. - warning_user_limit_reached_admin: > - Adding additional users will exceed the current limit. Please upgrade your plan to be able to ensure external users are able to access this work package. - warning_no_selected_user: "Please select users to share this work package with" - warning_locked_user: "The user %{user} is locked and cannot be shared with" - user_details: - locked: "Locked user" - invited: "Invite sent. " - resend_invite: "Resend." - invite_resent: "Invite has been resent" - not_project_member: "Not a project member" - project_group: "Group members might have additional privileges (as project members)" - not_project_group: "Group (shared with all members)" - additional_privileges_project: "Might have additional privileges (as project member)" - additional_privileges_group: "Might have additional privileges (as group member)" - additional_privileges_project_or_group: "Might have additional privileges (as project or group member)" + permissions: + comment: "Comment" + comment_description: "Can view and comment this work package." + edit: "Edit" + edit_description: "Can view, comment and edit this work package." + view: "View" + view_description: "Can view this work package." + sharing: + count: + zero: "0 users" + one: "1 user" + other: "%{count} users" + filter: + project_member: "Project member" + not_project_member: "Not project member" + project_group: "Project group" + not_project_group: "Not project group" + user: "User" + group: "Group" + role: "Role" + type: "Type" + denied: "You don't have permissions to share %{entities}." + label_search: "Search for users to invite" + label_search_placeholder: "Search by user or email address" + label_toggle_all: "Toggle all shares" + remove: "Remove" + share: "Share" + text_empty_search_description: "There are no users with the current filter criteria." + text_empty_search_header: "We couldn't find any matching results." + text_empty_state_description: "The %{entity} has not been shared with anyone yet." + text_empty_state_header: "Not shared" + text_user_limit_reached: "Adding additional users will exceed the current limit. Please contact an administrator to increase the user limit to ensure external users are able to access this %{entity}." + text_user_limit_reached_admins: 'Adding additional users will exceed the current limit. Please upgrade your plan to be able to add more users.' + warning_user_limit_reached: > + Adding additional users will exceed the current limit. Please contact an administrator to increase the user limit to ensure external users are able to access this %{entity}. + warning_user_limit_reached_admin: > + Adding additional users will exceed the current limit. Please upgrade your plan to be able to ensure external users are able to access this %{entity}. + warning_no_selected_user: "Please select users to share this %{entity} with" + warning_locked_user: "The user %{user} is locked and cannot be shared with" + user_details: + locked: "Locked user" + invited: "Invite sent. " + resend_invite: "Resend." + invite_resent: "Invite has been resent" + not_project_member: "Not a project member" + project_group: "Group members might have additional privileges (as project members)" + not_project_group: "Group (shared with all members)" + additional_privileges_project: "Might have additional privileges (as project member)" + additional_privileges_group: "Might have additional privileges (as group member)" + additional_privileges_project_or_group: "Might have additional privileges (as project or group member)" working_days: info: > From 72809b1b7da2e17fab59572e27502efc1c3b88db Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 25 Jun 2024 14:21:58 +0200 Subject: [PATCH 26/33] More i18n string changes --- .../members/user_filter_component.rb | 6 ++-- .../shares/user_details_component.html.erb | 2 +- app/controllers/shares_controller.rb | 8 ++--- app/forms/shares/invitee.rb | 4 +-- app/mailers/sharing_mailer.rb | 18 +++++------ .../work_packages/share/filter_spec.rb | 32 +++++++++---------- .../work_packages/share/share_spec.rb | 2 +- .../components/work_packages/share_modal.rb | 20 ++++++------ 8 files changed, 46 insertions(+), 46 deletions(-) diff --git a/app/components/members/user_filter_component.rb b/app/components/members/user_filter_component.rb index 43180102d97a..0ef24a8de7fa 100644 --- a/app/components/members/user_filter_component.rb +++ b/app/components/members/user_filter_component.rb @@ -103,11 +103,11 @@ def builtin_share_roles def mapped_shared_role_name(role) case role.builtin when Role::BUILTIN_WORK_PACKAGE_VIEWER - I18n.t("work_package.sharing.permissions.view") + I18n.t("work_package.permissions.view") when Role::BUILTIN_WORK_PACKAGE_COMMENTER - I18n.t("work_package.sharing.permissions.comment") + I18n.t("work_package.permissions.comment") when Role::BUILTIN_WORK_PACKAGE_EDITOR - I18n.t("work_package.sharing.permissions.edit") + I18n.t("work_package.permissions.edit") else role.name end diff --git a/app/components/shares/user_details_component.html.erb b/app/components/shares/user_details_component.html.erb index f9c64eaacb7d..2af0f5a1929b 100644 --- a/app/components/shares/user_details_component.html.erb +++ b/app/components/shares/user_details_component.html.erb @@ -25,7 +25,7 @@ concat(render(Primer::Beta::Text.new(color: :subtle)) { I18n.t('sharing.user_details.invited') }) concat( form_with(url: resend_invite_path, method: :post) do - render(Primer::Beta::Button.new(type: :submit, px: 0, scheme: :link)) { I18n.t('work_package.sharing.user_details.resend_invite') } + render(Primer::Beta::Button.new(type: :submit, px: 0, scheme: :link)) { I18n.t('sharing.user_details.resend_invite') } end ) end diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 34fd4094c550..7a15f180e956 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -57,7 +57,7 @@ def create find_or_create_users(send_notification: false) do |member_params| user = User.find_by(id: member_params[:user_id]) if user.present? && user.locked? - @errors.add(:base, I18n.t("work_package.sharing.warning_locked_user", user: user.name)) + @errors.add(:base, I18n.t("sharing.warning_locked_user", user: user.name)) else service_call = create_or_update_share(member_params[:user_id], [params[:member][:role_id]]) overall_result.push(service_call) @@ -320,13 +320,13 @@ def available_roles [ { label: I18n.t("work_package.permissions.edit"), value: role_mapping[Role::BUILTIN_WORK_PACKAGE_EDITOR], - description: I18n.t("work_package.sharing.permissions.edit_description") }, + description: I18n.t("work_package.permissions.edit_description") }, { label: I18n.t("work_package.permissions.comment"), value: role_mapping[Role::BUILTIN_WORK_PACKAGE_COMMENTER], description: I18n.t("work_package.permissions.comment_description") }, - { label: I18n.t("work_package.sharing.permissions.view"), + { label: I18n.t("work_package.permissions.view"), value: role_mapping[Role::BUILTIN_WORK_PACKAGE_VIEWER], - description: I18n.t("work_package.sharing.permissions.view_description"), + description: I18n.t("work_package.permissions.view_description"), default: true } ] else diff --git a/app/forms/shares/invitee.rb b/app/forms/shares/invitee.rb index 419c7f5d71b0..e6857c141cd0 100644 --- a/app/forms/shares/invitee.rb +++ b/app/forms/shares/invitee.rb @@ -30,14 +30,14 @@ class Invitee < ApplicationForm form do |user_invite_form| user_invite_form.autocompleter( name: :user_id, - label: I18n.t("work_package.sharing.label_search"), + label: I18n.t("sharing.label_search"), visually_hide_label: true, data: { "shares--user-limit-target": "autocompleter" }, autocomplete_options: { component: "opce-user-autocompleter", defaultData: false, id: "op-share-dialog-invite-autocomplete", - placeholder: I18n.t("work_package.sharing.label_search_placeholder"), + placeholder: I18n.t("sharing.label_search_placeholder"), data: { "test-selector": "op-share-dialog-invite-autocomplete" }, diff --git a/app/mailers/sharing_mailer.rb b/app/mailers/sharing_mailer.rb index d01efe5a9169..db6e63fa2978 100644 --- a/app/mailers/sharing_mailer.rb +++ b/app/mailers/sharing_mailer.rb @@ -39,11 +39,11 @@ def optionally_activated_url(back_url, invitation_token) def derive_role_rights(role) case role.builtin when Role::BUILTIN_WORK_PACKAGE_EDITOR - I18n.t("work_package.sharing.permissions.edit") + I18n.t("work_package.permissions.edit") when Role::BUILTIN_WORK_PACKAGE_COMMENTER - I18n.t("work_package.sharing.permissions.comment") + I18n.t("work_package.permissions.comment") when Role::BUILTIN_WORK_PACKAGE_VIEWER - I18n.t("work_package.sharing.permissions.view") + I18n.t("work_package.permissions.view") end end @@ -51,14 +51,14 @@ def derive_allowed_work_package_actions(role) allowed_actions = case role.builtin when Role::BUILTIN_WORK_PACKAGE_EDITOR - [I18n.t("work_package.sharing.permissions.view"), - I18n.t("work_package.sharing.permissions.comment"), - I18n.t("work_package.sharing.permissions.edit")] + [I18n.t("work_package.permissions.view"), + I18n.t("work_package.permissions.comment"), + I18n.t("work_package.permissions.edit")] when Role::BUILTIN_WORK_PACKAGE_COMMENTER - [I18n.t("work_package.sharing.permissions.view"), - I18n.t("work_package.sharing.permissions.comment")] + [I18n.t("work_package.permissions.view"), + I18n.t("work_package.permissions.comment")] when Role::BUILTIN_WORK_PACKAGE_VIEWER - [I18n.t("work_package.sharing.permissions.view")] + [I18n.t("work_package.permissions.view")] end allowed_actions.map(&:downcase) diff --git a/spec/features/work_packages/share/filter_spec.rb b/spec/features/work_packages/share/filter_spec.rb index c6e7e81e6a12..993db90e1c37 100644 --- a/spec/features/work_packages/share/filter_spec.rb +++ b/spec/features/work_packages/share/filter_spec.rb @@ -86,7 +86,7 @@ share_modal.expect_shared_count_of(6) # Filter for: project members (users only) - share_modal.filter("type", I18n.t("work_package.sharing.filter.project_member")) + share_modal.filter("type", I18n.t("sharing.filter.project_member")) share_modal.expect_shared_count_of(3) share_modal.expect_shared_with(project_user, "View") @@ -98,7 +98,7 @@ share_modal.expect_not_shared_with(shared_non_project_group) # Filter for: non-project members (users only) - share_modal.filter("type", I18n.t("work_package.sharing.filter.not_project_member")) + share_modal.filter("type", I18n.t("sharing.filter.not_project_member")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(non_project_user, "Edit") @@ -109,7 +109,7 @@ share_modal.expect_not_shared_with(shared_non_project_group) # Filter for: project members (groups only) - share_modal.filter("type", I18n.t("work_package.sharing.filter.project_group")) + share_modal.filter("type", I18n.t("sharing.filter.project_group")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(shared_project_group, "Edit") @@ -120,7 +120,7 @@ share_modal.expect_not_shared_with(shared_non_project_group) # Filter for: non-project members (groups only) - share_modal.filter("type", I18n.t("work_package.sharing.filter.not_project_group")) + share_modal.filter("type", I18n.t("sharing.filter.not_project_group")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(shared_non_project_group, "View") @@ -131,7 +131,7 @@ share_modal.expect_not_shared_with(shared_project_group) # Clicking again on the filter will reset it - share_modal.filter("type", I18n.t("work_package.sharing.filter.not_project_group")) + share_modal.filter("type", I18n.t("sharing.filter.not_project_group")) share_modal.expect_shared_count_of(6) share_modal.expect_shared_with(project_user, "View") @@ -147,7 +147,7 @@ share_modal.expect_shared_count_of(6) # Filter for: all principals with Edit permission - share_modal.filter("role", I18n.t("work_package.sharing.permissions.edit")) + share_modal.filter("role", I18n.t("sharing.permissions.edit")) share_modal.expect_shared_count_of(3) share_modal.expect_shared_with(inherited_project_user, "Edit") @@ -158,7 +158,7 @@ share_modal.expect_not_shared_with(shared_non_project_group) # Filter for: all principals with View permission - share_modal.filter("role", I18n.t("work_package.sharing.permissions.view")) + share_modal.filter("role", I18n.t("sharing.permissions.view")) share_modal.expect_shared_count_of(2) share_modal.expect_shared_with(project_user, "View") @@ -169,7 +169,7 @@ share_modal.expect_not_shared_with(shared_project_group) # Filter for: all principals with Comment permission - share_modal.filter("role", I18n.t("work_package.sharing.permissions.comment")) + share_modal.filter("role", I18n.t("sharing.permissions.comment")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(project_user2, "Comment") @@ -180,7 +180,7 @@ share_modal.expect_not_shared_with(shared_non_project_group) # Clicking again on the filter will reset it - share_modal.filter("role", I18n.t("work_package.sharing.permissions.comment")) + share_modal.filter("role", I18n.t("sharing.permissions.comment")) share_modal.expect_shared_count_of(6) share_modal.expect_shared_with(project_user, "View") @@ -198,7 +198,7 @@ # Filter for: all principals with View permission # role: view # type: none - share_modal.filter("role", I18n.t("work_package.sharing.permissions.view")) + share_modal.filter("role", I18n.t("sharing.permissions.view")) share_modal.expect_shared_count_of(2) share_modal.expect_shared_with(project_user, "View") @@ -211,7 +211,7 @@ # Additional filter for: project members (users only) # role: view # type: project members (users only) - share_modal.filter("type", I18n.t("work_package.sharing.filter.project_member")) + share_modal.filter("type", I18n.t("sharing.filter.project_member")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(project_user, "View") @@ -224,7 +224,7 @@ # Change type filter to: project members (groups only) # role: view # type: non-project members (groups only) - share_modal.filter("type", I18n.t("work_package.sharing.filter.not_project_group")) + share_modal.filter("type", I18n.t("sharing.filter.not_project_group")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(shared_non_project_group, "View") @@ -237,7 +237,7 @@ # Reset role filter # role: none # type: non-project members (groups only) - share_modal.filter("role", I18n.t("work_package.sharing.permissions.view")) + share_modal.filter("role", I18n.t("sharing.permissions.view")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(shared_non_project_group, "View") @@ -250,7 +250,7 @@ # Reset type filter # role: none # type: none - share_modal.filter("type", I18n.t("work_package.sharing.filter.not_project_group")) + share_modal.filter("type", I18n.t("sharing.filter.not_project_group")) share_modal.expect_shared_count_of(6) share_modal.expect_shared_with(project_user, "View") @@ -264,8 +264,8 @@ context "and there are no matching results for my filter" do it 'does not check the "toggle all" checkbox' do share_modal.expect_open - share_modal.filter("type", I18n.t("work_package.sharing.filter.not_project_member")) - share_modal.filter("role", I18n.t("work_package.sharing.permissions.view")) + share_modal.filter("type", I18n.t("sharing.filter.not_project_member")) + share_modal.filter("role", I18n.t("sharing.permissions.view")) share_modal.expect_empty_search_blankslate share_modal.expect_shared_count_of(0) diff --git a/spec/features/work_packages/share/share_spec.rb b/spec/features/work_packages/share/share_spec.rb index 33d4fbc47b7b..2151b4fad217 100644 --- a/spec/features/work_packages/share/share_spec.rb +++ b/spec/features/work_packages/share/share_spec.rb @@ -450,7 +450,7 @@ def inherited_member_roles(group:) # The number of shared people has not changed, but an error message is shown share_modal.expect_shared_count_of(6) - share_modal.expect_error_message(I18n.t("work_package.sharing.warning_locked_user", user: locked_user.name)) + share_modal.expect_error_message(I18n.t("sharing.warning_locked_user", user: locked_user.name)) end end diff --git a/spec/support/components/work_packages/share_modal.rb b/spec/support/components/work_packages/share_modal.rb index 566d22724a34..023e237eca76 100644 --- a/spec/support/components/work_packages/share_modal.rb +++ b/spec/support/components/work_packages/share_modal.rb @@ -190,13 +190,13 @@ def unchecked_permission def expect_blankslate within_modal do - expect(page).to have_text(I18n.t("work_package.sharing.text_empty_state_description")) + expect(page).to have_text(I18n.t("sharing.text_empty_state_description")) end end def expect_empty_search_blankslate within_modal do - expect(page).to have_text(I18n.t("work_package.sharing.text_empty_search_description")) + expect(page).to have_text(I18n.t("sharing.text_empty_search_description")) end end @@ -318,26 +318,26 @@ def expect_not_shared_with(*principals) def expect_shared_count_of(count) expect(shares_header) - .to have_text(I18n.t("work_package.sharing.count", count:)) + .to have_text(I18n.t("sharing.count", count:)) end def expect_no_invite_option within_modal do expect(page) - .to have_text(I18n.t("work_package.sharing.permissions.denied")) + .to have_text(I18n.t("sharing.permissions.denied")) end end def resend_invite(user) within user_row(user) do - click_button I18n.t("work_package.sharing.user_details.resend_invite") + click_button I18n.t("sharing.user_details.resend_invite") end end def expect_invite_resent(user) within user_row(user) do expect(page) - .to have_text(I18n.t("work_package.sharing.user_details.invite_resent")) + .to have_text(I18n.t("sharing.user_details.invite_resent")) end end @@ -388,14 +388,14 @@ def expect_upsale_banner def expect_no_user_limit_warning within modal_element do expect(page) - .to have_no_text(I18n.t("work_package.sharing.warning_user_limit_reached"), wait: 0) + .to have_no_text(I18n.t("sharing.warning_user_limit_reached", entity: WorkPackage.model_name.human), wait: 0) end end def expect_user_limit_warning within modal_element do expect(page) - .to have_text(I18n.t("work_package.sharing.warning_user_limit_reached")) + .to have_text(I18n.t("sharing.warning_user_limit_reached", entity: WorkPackage.model_name.human)) end end @@ -410,14 +410,14 @@ def expect_error_message(text) def expect_select_a_user_hint within modal_element do expect(page) - .to have_text(I18n.t("work_package.sharing.warning_no_selected_user")) + .to have_text(I18n.t("sharing.warning_no_selected_user", entity: WorkPackage.model_name.human)) end end def expect_no_select_a_user_hint within modal_element do expect(page) - .to have_no_text(I18n.t("work_package.sharing.warning_no_selected_user"), wait: 0) + .to have_no_text(I18n.t("sharing.warning_no_selected_user", entity: WorkPackage.model_name.human), wait: 0) end end end From c4b36d0189d5ef8d912ed584ce0845abc3f9fc87 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 25 Jun 2024 14:42:45 +0200 Subject: [PATCH 27/33] Correct i18n --- spec/features/work_packages/share/filter_spec.rb | 14 +++++++------- .../components/work_packages/share_modal.rb | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/features/work_packages/share/filter_spec.rb b/spec/features/work_packages/share/filter_spec.rb index 993db90e1c37..741725934eea 100644 --- a/spec/features/work_packages/share/filter_spec.rb +++ b/spec/features/work_packages/share/filter_spec.rb @@ -147,7 +147,7 @@ share_modal.expect_shared_count_of(6) # Filter for: all principals with Edit permission - share_modal.filter("role", I18n.t("sharing.permissions.edit")) + share_modal.filter("role", I18n.t("work_package.permissions.edit")) share_modal.expect_shared_count_of(3) share_modal.expect_shared_with(inherited_project_user, "Edit") @@ -158,7 +158,7 @@ share_modal.expect_not_shared_with(shared_non_project_group) # Filter for: all principals with View permission - share_modal.filter("role", I18n.t("sharing.permissions.view")) + share_modal.filter("role", I18n.t("work_package.permissions.view")) share_modal.expect_shared_count_of(2) share_modal.expect_shared_with(project_user, "View") @@ -169,7 +169,7 @@ share_modal.expect_not_shared_with(shared_project_group) # Filter for: all principals with Comment permission - share_modal.filter("role", I18n.t("sharing.permissions.comment")) + share_modal.filter("role", I18n.t("work_package.permissions.comment")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(project_user2, "Comment") @@ -180,7 +180,7 @@ share_modal.expect_not_shared_with(shared_non_project_group) # Clicking again on the filter will reset it - share_modal.filter("role", I18n.t("sharing.permissions.comment")) + share_modal.filter("role", I18n.t("work_package.permissions.comment")) share_modal.expect_shared_count_of(6) share_modal.expect_shared_with(project_user, "View") @@ -198,7 +198,7 @@ # Filter for: all principals with View permission # role: view # type: none - share_modal.filter("role", I18n.t("sharing.permissions.view")) + share_modal.filter("role", I18n.t("work_package.permissions.view")) share_modal.expect_shared_count_of(2) share_modal.expect_shared_with(project_user, "View") @@ -237,7 +237,7 @@ # Reset role filter # role: none # type: non-project members (groups only) - share_modal.filter("role", I18n.t("sharing.permissions.view")) + share_modal.filter("role", I18n.t("work_package.permissions.view")) share_modal.expect_shared_count_of(1) share_modal.expect_shared_with(shared_non_project_group, "View") @@ -265,7 +265,7 @@ it 'does not check the "toggle all" checkbox' do share_modal.expect_open share_modal.filter("type", I18n.t("sharing.filter.not_project_member")) - share_modal.filter("role", I18n.t("sharing.permissions.view")) + share_modal.filter("role", I18n.t("work_package.permissions.view")) share_modal.expect_empty_search_blankslate share_modal.expect_shared_count_of(0) diff --git a/spec/support/components/work_packages/share_modal.rb b/spec/support/components/work_packages/share_modal.rb index 023e237eca76..f67d395db947 100644 --- a/spec/support/components/work_packages/share_modal.rb +++ b/spec/support/components/work_packages/share_modal.rb @@ -324,7 +324,7 @@ def expect_shared_count_of(count) def expect_no_invite_option within_modal do expect(page) - .to have_text(I18n.t("sharing.permissions.denied")) + .to have_text(I18n.t("work_package.permissions.denied")) end end From aa168597231fbbe30ac5c8d24db0ef9074e2b23d Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 25 Jun 2024 15:09:21 +0200 Subject: [PATCH 28/33] Fix interpolations --- app/components/shares/invite_user_form_component.html.erb | 2 +- spec/support/components/work_packages/share_modal.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/components/shares/invite_user_form_component.html.erb b/app/components/shares/invite_user_form_component.html.erb index 917b9c8ce542..5900c1a2ef47 100644 --- a/app/components/shares/invite_user_form_component.html.erb +++ b/app/components/shares/invite_user_form_component.html.erb @@ -96,7 +96,7 @@ end end else - render(Primer::Alpha::Banner.new(icon: :info)) { I18n.t('sharing.denied', entity: @entity.model_name.human(count: 2)) } + render(Primer::Alpha::Banner.new(icon: :info)) { I18n.t('sharing.denied', entities: @entity.model_name.human(count: 2)) } end end %> diff --git a/spec/support/components/work_packages/share_modal.rb b/spec/support/components/work_packages/share_modal.rb index f67d395db947..ef9b4f67fdb2 100644 --- a/spec/support/components/work_packages/share_modal.rb +++ b/spec/support/components/work_packages/share_modal.rb @@ -190,7 +190,7 @@ def unchecked_permission def expect_blankslate within_modal do - expect(page).to have_text(I18n.t("sharing.text_empty_state_description")) + expect(page).to have_text(I18n.t("sharing.text_empty_state_description", entity: WorkPackage.model_name.human)) end end @@ -324,7 +324,7 @@ def expect_shared_count_of(count) def expect_no_invite_option within_modal do expect(page) - .to have_text(I18n.t("work_package.permissions.denied")) + .to have_text(I18n.t("sharing.denied", entities: WorkPackage.model_name.human(count: 2))) end end From 1cde8f614a853420240685fc45a9b6a95fa9c7f3 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 25 Jun 2024 15:51:56 +0200 Subject: [PATCH 29/33] Fix Rubocop issues --- app/controllers/shares_controller.rb | 182 ++++++++++++++++----------- 1 file changed, 108 insertions(+), 74 deletions(-) diff --git a/app/controllers/shares_controller.rb b/app/controllers/shares_controller.rb index 7a15f180e956..0e8a174e3d31 100644 --- a/app/controllers/shares_controller.rb +++ b/app/controllers/shares_controller.rb @@ -50,7 +50,7 @@ def index available_roles:), layout: nil end - def create + def create # rubocop:disable Metrics/AbcSize,Metrics/PerceivedComplexity overall_result = [] @errors = ActiveModel::Errors.new(self) @@ -142,51 +142,60 @@ def destroy_share(share) end def create_or_update_share(user_id, role_ids) - Shares::CreateOrUpdateService - .new( - user: current_user, - create_contract_class: sharing_contract_scope::CreateContract, - update_contract_class: sharing_contract_scope::UpdateContract - ) + Shares::CreateOrUpdateService.new( + user: current_user, + create_contract_class: sharing_contract_scope::CreateContract, + update_contract_class: sharing_contract_scope::UpdateContract + ) .call(entity: @entity, user_id:, role_ids:) end def respond_with_replace_modal replace_via_turbo_stream( - component: Shares::ModalBodyComponent.new(entity: @entity, - available_roles:, - shares: @new_shares || load_shares, - sharing_manageable: sharing_manageable?, - errors: @errors) + component: Shares::ModalBodyComponent.new( + entity: @entity, + available_roles:, + shares: @new_shares || load_shares, + sharing_manageable: sharing_manageable?, + errors: @errors + ) ) respond_with_turbo_streams end - def respond_with_prepend_shares + def respond_with_prepend_shares # rubocop:disable Metrics/AbcSize replace_via_turbo_stream( - component: Shares::InviteUserFormComponent.new(entity: @entity, - available_roles:, - sharing_manageable: sharing_manageable?, - errors: @errors) + component: Shares::InviteUserFormComponent.new( + entity: @entity, + available_roles:, + sharing_manageable: sharing_manageable?, + errors: @errors + ) ) update_via_turbo_stream( - component: Shares::CounterComponent.new(entity: @entity, - count: current_visible_member_count, - sharing_manageable: sharing_manageable?) + component: Shares::CounterComponent.new( + entity: @entity, + count: current_visible_member_count, + sharing_manageable: sharing_manageable? + ) ) @new_shares.each do |share| prepend_via_turbo_stream( - component: Shares::ShareRowComponent.new(share:, - available_roles:, - sharing_manageable: sharing_manageable?), - target_component: Shares::ModalBodyComponent.new(entity: @entity, - available_roles:, - sharing_manageable: sharing_manageable?, - shares: load_shares, - errors: @errors) + component: Shares::ShareRowComponent.new( + share:, + available_roles:, + sharing_manageable: sharing_manageable? + ), + target_component: Shares::ModalBodyComponent.new( + entity: @entity, + available_roles:, + sharing_manageable: sharing_manageable?, + shares: load_shares, + errors: @errors + ) ) end @@ -194,37 +203,57 @@ def respond_with_prepend_shares end def respond_with_new_invite_form - replace_via_turbo_stream(component: Shares::InviteUserFormComponent.new(entity: @entity, - available_roles:, - sharing_manageable: sharing_manageable?, - errors: @errors)) + replace_via_turbo_stream( + component: Shares::InviteUserFormComponent.new( + entity: @entity, + available_roles:, + sharing_manageable: sharing_manageable?, + errors: @errors + ) + ) respond_with_turbo_streams end def respond_with_update_permission_button - replace_via_turbo_stream(component: Shares::PermissionButtonComponent.new(share: @share, - available_roles:, - data: { "test-selector": "op-share-dialog-update-role" })) + replace_via_turbo_stream( + component: Shares::PermissionButtonComponent.new( + share: @share, + available_roles:, + data: { "test-selector": "op-share-dialog-update-role" } + ) + ) respond_with_turbo_streams end def respond_with_remove_share - remove_via_turbo_stream(component: Shares::ShareRowComponent.new(share: @share, - available_roles:, - sharing_manageable: sharing_manageable?)) - update_via_turbo_stream(component: Shares::CounterComponent.new(entity: @entity, - count: current_visible_member_count, - sharing_manageable: sharing_manageable?)) + remove_via_turbo_stream( + component: Shares::ShareRowComponent.new( + share: @share, + available_roles:, + sharing_manageable: sharing_manageable? + ) + ) + update_via_turbo_stream( + component: Shares::CounterComponent.new( + entity: @entity, + count: current_visible_member_count, + sharing_manageable: sharing_manageable? + ) + ) respond_with_turbo_streams end def respond_with_update_user_details - update_via_turbo_stream(component: Shares::UserDetailsComponent.new(share: @share, - manager_mode: sharing_manageable?, - invite_resent: true)) + update_via_turbo_stream( + component: Shares::UserDetailsComponent.new( + share: @share, + manager_mode: sharing_manageable?, + invite_resent: true + ) + ) respond_with_turbo_streams end @@ -232,9 +261,11 @@ def respond_with_update_user_details def respond_with_bulk_updated_permission_buttons @selected_shares.each do |share| replace_via_turbo_stream( - component: Shares::PermissionButtonComponent.new(share:, - available_roles:, - data: { "test-selector": "op-share-dialog-update-role" }) + component: Shares::PermissionButtonComponent.new( + share:, + available_roles:, + data: { "test-selector": "op-share-dialog-update-role" } + ) ) end @@ -244,16 +275,20 @@ def respond_with_bulk_updated_permission_buttons def respond_with_bulk_removed_shares @selected_shares.each do |share| remove_via_turbo_stream( - component: Shares::ShareRowComponent.new(share:, - available_roles:, - sharing_manageable: sharing_manageable?) + component: Shares::ShareRowComponent.new( + share:, + available_roles:, + sharing_manageable: sharing_manageable? + ) ) end update_via_turbo_stream( - component: Shares::CounterComponent.new(entity: @entity, - count: current_visible_member_count, - sharing_manageable: sharing_manageable?) + component: Shares::CounterComponent.new( + entity: @entity, + count: current_visible_member_count, + sharing_manageable: sharing_manageable? + ) ) respond_with_turbo_streams @@ -286,10 +321,9 @@ def current_visible_member_count def load_query return @query if defined?(@query) - @query = ParamsToQueryService.new(Member, - current_user, - query_class: Queries::Members::EntityMemberQuery) - .call(params) + @query = ParamsToQueryService + .new(Member, current_user, query_class: Queries::Members::EntityMemberQuery) + .call(params) # Set default filter on the entity @query.where("entity_id", "=", @entity.id) @@ -315,23 +349,23 @@ def load_selected_shares def available_roles @available_roles ||= if @entity.is_a?(WorkPackage) - role_mapping = WorkPackageRole.unscoped.pluck(:builtin, :id).to_h - - [ - { label: I18n.t("work_package.permissions.edit"), - value: role_mapping[Role::BUILTIN_WORK_PACKAGE_EDITOR], - description: I18n.t("work_package.permissions.edit_description") }, - { label: I18n.t("work_package.permissions.comment"), - value: role_mapping[Role::BUILTIN_WORK_PACKAGE_COMMENTER], - description: I18n.t("work_package.permissions.comment_description") }, - { label: I18n.t("work_package.permissions.view"), - value: role_mapping[Role::BUILTIN_WORK_PACKAGE_VIEWER], - description: I18n.t("work_package.permissions.view_description"), - default: true } - ] - else - [] - end + role_mapping = WorkPackageRole.unscoped.pluck(:builtin, :id).to_h + + [ + { label: I18n.t("work_package.permissions.edit"), + value: role_mapping[Role::BUILTIN_WORK_PACKAGE_EDITOR], + description: I18n.t("work_package.permissions.edit_description") }, + { label: I18n.t("work_package.permissions.comment"), + value: role_mapping[Role::BUILTIN_WORK_PACKAGE_COMMENTER], + description: I18n.t("work_package.permissions.comment_description") }, + { label: I18n.t("work_package.permissions.view"), + value: role_mapping[Role::BUILTIN_WORK_PACKAGE_VIEWER], + description: I18n.t("work_package.permissions.view_description"), + default: true } + ] + else + [] + end end def sharing_contract_scope From ba1fb3829c135ea693ecf7118224efbeb342bf39 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Tue, 25 Jun 2024 16:02:03 +0200 Subject: [PATCH 30/33] A bit more Rubocop fixing --- app/components/shares/modal_body_component.rb | 2 +- app/components/shares/modal_upsale_component.rb | 2 +- spec/support/components/work_packages/share_modal.rb | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/components/shares/modal_body_component.rb b/app/components/shares/modal_body_component.rb index 741c2fd5eba2..c02507607d5a 100644 --- a/app/components/shares/modal_body_component.rb +++ b/app/components/shares/modal_body_component.rb @@ -71,7 +71,7 @@ def insert_target_modifier_id "op-share-dialog-active-shares" end - def blankslate_config + def blankslate_config # rubocop:disable Metrics/AbcSize @blankslate_config ||= {}.tap do |config| if params[:filters].blank? config[:icon] = :people diff --git a/app/components/shares/modal_upsale_component.rb b/app/components/shares/modal_upsale_component.rb index 6935205290aa..933a8f061612 100644 --- a/app/components/shares/modal_upsale_component.rb +++ b/app/components/shares/modal_upsale_component.rb @@ -27,7 +27,7 @@ #++ module Shares - class ModalUpsaleComponent < ApplicationComponent + class ModalUpsaleComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent include ApplicationHelper include OpTurbo::Streamable include OpPrimer::ComponentHelpers diff --git a/spec/support/components/work_packages/share_modal.rb b/spec/support/components/work_packages/share_modal.rb index ef9b4f67fdb2..ab5be93450f2 100644 --- a/spec/support/components/work_packages/share_modal.rb +++ b/spec/support/components/work_packages/share_modal.rb @@ -213,7 +213,7 @@ def invite_user(users, role_name) select_invite_role(role_name) within_modal do - click_button "Share" + click_on "Share" end end @@ -245,7 +245,7 @@ def remove_user(user) def select_invite_role(role_name) within modal_element.find('[data-test-selector="op-share-dialog-invite-role"]') do # Open the ActionMenu - click_button "View" + click_on "View" find(".ActionListContent", text: role_name).click end @@ -256,7 +256,7 @@ def change_role(user, role_name) find('[data-test-selector="op-share-dialog-update-role"]').click within ".ActionListWrap" do - click_button role_name + click_on role_name end end end @@ -285,7 +285,7 @@ def close def click_share within_modal do - click_button "Share" + click_on "Share" end end @@ -330,7 +330,7 @@ def expect_no_invite_option def resend_invite(user) within user_row(user) do - click_button I18n.t("sharing.user_details.resend_invite") + click_on I18n.t("sharing.user_details.resend_invite") end end From 812d10a6ebced48b04febdaa2bfc7918df70c9f2 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 26 Jun 2024 09:48:44 +0200 Subject: [PATCH 31/33] Rename `_option` args --- app/components/shares/modal_body_component.rb | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/app/components/shares/modal_body_component.rb b/app/components/shares/modal_body_component.rb index c02507607d5a..588f60d32016 100644 --- a/app/components/shares/modal_body_component.rb +++ b/app/components/shares/modal_body_component.rb @@ -106,26 +106,26 @@ def type_filter_options end end - def type_filter_option_active?(_option) + def type_filter_option_active?(option) principal_type_filter_value = current_filter_value(params[:filters], "principal_type") project_member_filter_value = current_filter_value(params[:filters], "also_project_member") return false if principal_type_filter_value.nil? || project_member_filter_value.nil? principal_type_checked = - _option[:value][:principal_type] == principal_type_filter_value + option[:value][:principal_type] == principal_type_filter_value membership_selected = - _option[:value][:project_member] == ActiveRecord::Type::Boolean.new.cast(project_member_filter_value) + option[:value][:project_member] == ActiveRecord::Type::Boolean.new.cast(project_member_filter_value) principal_type_checked && membership_selected end - def role_filter_option_active?(_option) + def role_filter_option_active?(option) role_filter_value = current_filter_value(params[:filters], "role_id") return false if role_filter_value.nil? - selected_role = @available_roles.find { _1[:value] == _option[:value] } + selected_role = @available_roles.find { _1[:value] == option[:value] } selected_role[:value] == role_filter_value.to_i end @@ -165,32 +165,32 @@ def role_filter_for(option) ] end - def apply_type_filter(_option) + def apply_type_filter(option) current_type_filter_value = current_filter_value(params[:filters], "principal_type") current_member_filter_value = current_filter_value(params[:filters], "also_project_member") filter = [] - if _option.nil? && current_type_filter_value.present? && current_member_filter_value.present? + if option.nil? && current_type_filter_value.present? && current_member_filter_value.present? # When there is already a type filter set and no new value passed, we want to keep that filter value = { value: { principal_type: current_type_filter_value, project_member: current_member_filter_value } } filter = type_filter_for(value) - elsif _option.present? && !type_filter_option_active?(_option) + elsif option.present? && !type_filter_option_active?(option) # Only when the passed filter option is not the currently selected one, we apply the filter - filter = type_filter_for(_option) + filter = type_filter_for(option) end filter end - def type_filter_for(_option) + def type_filter_for(option) filter = [] - if ActiveRecord::Type::Boolean.new.cast(_option[:value][:project_member]) + if ActiveRecord::Type::Boolean.new.cast(option[:value][:project_member]) filter.push({ also_project_member: { operator: "=", values: [OpenProject::Database::DB_VALUE_TRUE] } }) else filter.push({ also_project_member: { operator: "=", values: [OpenProject::Database::DB_VALUE_FALSE] } }) end - filter.push({ principal_type: { operator: "=", values: [_option[:value][:principal_type]] } }) + filter.push({ principal_type: { operator: "=", values: [option[:value][:principal_type]] } }) filter end From 3986d816fa169ad3090d953d7ebd708d90698c0c Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 26 Jun 2024 10:35:41 +0200 Subject: [PATCH 32/33] Move strings from BulkSelectionController into sharing namespace --- config/locales/js-en.yml | 8 +++++--- .../dynamic/shares/bulk-selection.controller.ts | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/config/locales/js-en.yml b/config/locales/js-en.yml index 76d0b619abd2..7e58a84fa88e 100644 --- a/config/locales/js-en.yml +++ b/config/locales/js-en.yml @@ -990,6 +990,11 @@ en: preformatted_text: "Preformatted Text" wiki_link: "Link to a Wiki page" image: "Image" + sharing: + selected_count: "%{count} selected" + selection: + mixed: "Mixed" + work_packages: bulk_actions: move: "Bulk change of project" @@ -1149,9 +1154,6 @@ en: share: "Share" title: "Share work package" show_all_users: "Show all users with whom the work package has been shared with" - selected_count: "%{count} selected" - selection: - mixed: "Mixed" upsale: description: "Share work packages with users who are not members of the project." table: diff --git a/frontend/src/stimulus/controllers/dynamic/shares/bulk-selection.controller.ts b/frontend/src/stimulus/controllers/dynamic/shares/bulk-selection.controller.ts index 3e01bc147cf7..a82042673b8a 100644 --- a/frontend/src/stimulus/controllers/dynamic/shares/bulk-selection.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/shares/bulk-selection.controller.ts @@ -32,7 +32,7 @@ import { Controller } from '@hotwired/stimulus'; export default class BulkSelectionController extends Controller { static values = { - bulkUpdateRoleLabel: { type: String, default: I18n.t('js.work_packages.sharing.selection.mixed') }, + bulkUpdateRoleLabel: { type: String, default: I18n.t('js.sharing.selection.mixed') }, }; declare bulkUpdateRoleLabelValue:string; @@ -152,7 +152,7 @@ export default class BulkSelectionController extends Controller { private updateBulkUpdateRoleLabelValue() { if (new Set(this.selectedPermissions).size > 1) { - this.bulkUpdateRoleLabelValue = I18n.t('js.work_packages.sharing.selection.mixed'); + this.bulkUpdateRoleLabelValue = I18n.t('js.sharing.selection.mixed'); this.bulkPermissionButtons.forEach((button) => button.setAttribute('aria-checked', 'false')); } else { this.bulkUpdateRoleLabelValue = this.selectedPermissions[0]; @@ -180,7 +180,7 @@ export default class BulkSelectionController extends Controller { } private showSelectedCounter() { - this.selectedCounterTarget.textContent = I18n.t('js.work_packages.sharing.selected_count', { count: this.checked.length }); + this.selectedCounterTarget.textContent = I18n.t('js.sharing.selected_count', { count: this.checked.length }); this.sharedCounterTarget.setAttribute('hidden', 'true'); this.selectedCounterTarget.removeAttribute('hidden'); } From fef400e031280b7e0e2fc056ac121332c05e9716 Mon Sep 17 00:00:00 2001 From: Klaus Zanders Date: Wed, 26 Jun 2024 10:38:11 +0200 Subject: [PATCH 33/33] Move some more strings into general sharing scope --- config/locales/js-en.yml | 3 +-- .../wp-buttons/wp-share-button/wp-share-button.component.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/config/locales/js-en.yml b/config/locales/js-en.yml index 7e58a84fa88e..3ab5a7eaa6d9 100644 --- a/config/locales/js-en.yml +++ b/config/locales/js-en.yml @@ -991,10 +991,10 @@ en: wiki_link: "Link to a Wiki page" image: "Image" sharing: + share: "Share" selected_count: "%{count} selected" selection: mixed: "Mixed" - work_packages: bulk_actions: move: "Bulk change of project" @@ -1151,7 +1151,6 @@ en: is_parent: "The dates of this work package are automatically deduced from its children. Activate 'Manual scheduling' to set the dates." is_switched_from_manual_to_automatic: "The dates of this work package may need to be recalculated after switching from manual to automatic scheduling due to relationships with other work packages." sharing: - share: "Share" title: "Share work package" show_all_users: "Show all users with whom the work package has been shared with" upsale: diff --git a/frontend/src/app/features/work-packages/components/wp-buttons/wp-share-button/wp-share-button.component.ts b/frontend/src/app/features/work-packages/components/wp-buttons/wp-share-button/wp-share-button.component.ts index 417c4dd01742..a6e356bb5b4e 100644 --- a/frontend/src/app/features/work-packages/components/wp-buttons/wp-share-button/wp-share-button.component.ts +++ b/frontend/src/app/features/work-packages/components/wp-buttons/wp-share-button/wp-share-button.component.ts @@ -57,7 +57,7 @@ export class WorkPackageShareButtonComponent extends UntilDestroyedMixin impleme shareCount$:Observable; public text = { - share: this.I18n.t('js.work_packages.sharing.share'), + share: this.I18n.t('js.sharing.share'), }; constructor(