Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/57316 robots follow action links unnecessarily #16470

Merged
merged 4 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/components/projects/configure_view_modal_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
# ++

class Projects::ConfigureViewModalComponent < ApplicationComponent
include OpTurbo::Streamable

MODAL_ID = "op-project-list-configure-dialog"
QUERY_FORM_ID = "op-project-list-configure-query-form"
COLUMN_HTML_NAME = "columns"
Expand Down
4 changes: 3 additions & 1 deletion app/components/projects/delete_list_modal_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@
# See COPYRIGHT and LICENSE files for more details.
# ++

class Projects::DeleteListModalComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent
class Projects::DeleteListModalComponent < ApplicationComponent
include OpTurbo::Streamable

MODAL_ID = "op-project-list-delete-dialog"

options :query
Expand Down
3 changes: 2 additions & 1 deletion app/components/projects/export_list_modal_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
# See COPYRIGHT and LICENSE files for more details.
# ++

class Projects::ExportListModalComponent < ApplicationComponent # rubocop:disable OpenProject/AddPreviewForViewComponent
class Projects::ExportListModalComponent < ApplicationComponent
include OpTurbo::Streamable
MODAL_ID = "op-project-list-export-dialog"

options :query
Expand Down
21 changes: 11 additions & 10 deletions app/components/projects/index_page_header_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@
menu.with_item(
tag: :a,
label: t(:label_overall_activity),
href: activities_path
href: activities_path,
content_arguments: { rel: "nofollow" }
) do |item|
item.with_leading_visual_icon(icon: 'tasklist')
end
Expand All @@ -122,34 +123,34 @@
end

menu.with_item(
tag: :a,
label: t('js.label_export'),
content_arguments: { 'data-show-dialog-id': Projects::ExportListModalComponent::MODAL_ID }
href: export_list_modal_projects_path(projects_query_params),
content_arguments: { data: { controller: "async-dialog" }, rel: "nofollow" }
) do |item|
item.with_leading_visual_icon(icon: 'sign-out')
end

menu.with_item(
tag: :a,
label: t(:'queries.configure_view.heading'),
content_arguments: { 'data-show-dialog-id': Projects::ConfigureViewModalComponent::MODAL_ID }
href: configure_view_modal_project_queries_path(projects_query_params),
content_arguments: { data: { controller: "async-dialog" }}
) do |item|
item.with_leading_visual_icon(icon: :gear)
end

if query.persisted?
menu.with_item(
tag: :a,
label: t(:button_delete),
scheme: :danger,
content_arguments: { 'data-show-dialog-id': Projects::DeleteListModalComponent::MODAL_ID }
href: destroy_confirmation_modal_project_query_path(query.id),
content_arguments: { data: { controller: "async-dialog" }}
) do |item|
item.with_leading_visual_icon(icon: 'trash')
end
end
end
end
%>

<% if show_state? %>
<%= render(Projects::ConfigureViewModalComponent.new(query:)) %>
<%= render(Projects::DeleteListModalComponent.new(query:)) if query.persisted? %>
<%= render(Projects::ExportListModalComponent.new(query:)) %>
<% end %>
18 changes: 14 additions & 4 deletions app/controllers/projects/queries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@
class Projects::QueriesController < ApplicationController
include Projects::QueryLoading
include OpTurbo::ComponentStream
include OpTurbo::DialogStreamHelper

# No need for a more specific authorization check. That is carried out in the contracts.
no_authorization_required! :show, :new, :create, :rename, :update, :toggle_public, :destroy
before_action :require_login
before_action :find_query, only: %i[show rename update destroy toggle_public]
before_action :build_query_or_deny_access, only: %i[new create]
no_authorization_required! :show, :new, :create, :rename, :update, :toggle_public, :destroy, :destroy_confirmation_modal,
:configure_view_modal
before_action :require_login, except: %i[configure_view_modal]
before_action :find_query, only: %i[show rename update destroy toggle_public destroy_confirmation_modal]
before_action :build_query_or_deny_access, only: %i[new create configure_view_modal]

current_menu_item [:new, :rename, :create, :update] do
:projects
Expand Down Expand Up @@ -101,6 +103,14 @@ def destroy
redirect_to projects_path
end

def destroy_confirmation_modal
respond_with_dialog Projects::DeleteListModalComponent.new(query: @query)
end

def configure_view_modal
respond_with_dialog Projects::ConfigureViewModalComponent.new(query: @query)
end

private

def render_result(service_call, success_i18n_key:, error_i18n_key:) # rubocop:disable Metrics/AbcSize
Expand Down
11 changes: 8 additions & 3 deletions app/controllers/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,20 @@ class ProjectsController < ApplicationController
menu_item :overview
menu_item :roadmap, only: :roadmap

before_action :find_project, except: %i[index new]
before_action :load_query_or_deny_access, only: %i[index]
before_action :find_project, except: %i[index new export_list_modal]
before_action :load_query_or_deny_access, only: %i[index export_list_modal]
before_action :authorize, only: %i[copy deactivate_work_package_attachments]
before_action :authorize_global, only: %i[new]
before_action :require_admin, only: %i[destroy destroy_info]

no_authorization_required! :index
no_authorization_required! :index, :export_list_modal

include SortHelper
include PaginationHelper
include QueriesHelper
include ProjectsHelper
include Projects::QueryLoading
include OpTurbo::DialogStreamHelper

helper_method :has_managed_project_folders?

Expand Down Expand Up @@ -105,6 +106,10 @@ def deactivate_work_package_attachments
end
end

def export_list_modal
respond_with_dialog Projects::ExportListModalComponent.new(query: @query)
end

private

def has_managed_project_folders?(project)
Expand Down
2 changes: 1 addition & 1 deletion app/models/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Project < ApplicationRecord
IDENTIFIER_MAX_LENGTH = 100

# reserved identifiers
RESERVED_IDENTIFIERS = %w[new menu queries].freeze
RESERVED_IDENTIFIERS = %w[new menu queries export_list_modal].freeze

has_many :members, -> {
# TODO: check whether this should
Expand Down
9 changes: 9 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@
member do
get :rename
post :toggle_public
get :destroy_confirmation_modal
end

collection do
get :configure_view_modal
end
end

Expand Down Expand Up @@ -248,6 +253,10 @@
post :deactivate_work_package_attachments
end

collection do
get :export_list_modal
end

resources :versions, only: %i[new create] do
collection do
put :close_completed
Expand Down
12 changes: 12 additions & 0 deletions lookbook/docs/patterns/05-dialogs.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ class TestController < ApplicationControler
end
```

For this to work, the `OpTurbo::Streamable` module needs to be included in the modal component:

```ruby
class MyDialogComponent < ApplicationControler
# include the module
include OpTurbo::Streamable

# ...
end
```


Assuming the MyDialogComponent template looks like this:

```erb
Expand Down
9 changes: 9 additions & 0 deletions spec/routing/project_queries_routing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,13 @@
expect(delete("/project_queries/42")).to route_to("projects/queries#destroy",
id: "42")
end

it "/project_queries/:id/destroy_confirmation_modal GET routes to projects/queries#destroy_confirmation_modal" do
expect(get("/project_queries/42/destroy_confirmation_modal")).to route_to("projects/queries#destroy_confirmation_modal",
id: "42")
end

it "/project_queries/:id/configure_view_modal GET routes to projects/queries#configure_view_modal" do
expect(get("/project_queries/configure_view_modal")).to route_to("projects/queries#configure_view_modal")
end
end
8 changes: 8 additions & 0 deletions spec/routing/project_routing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@
end
end

describe "export_list_modal" do
it do
expect(get("/projects/export_list_modal")).to route_to(
controller: "projects", action: "export_list_modal"
)
end
end

describe "templated" do
it do
expect(delete("/projects/123/templated"))
Expand Down
Loading