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

Allow ProjectQuery to be marked as public #15609

Merged
merged 28 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
755c3e0
Add public flag to ProjectQuery
klaustopher May 19, 2024
e57eb91
Add `manage_public_project_queries` permission globally
klaustopher May 19, 2024
85235de
Rename scopes to remove duplication
klaustopher May 19, 2024
3768e72
Fix typo in permission scope
klaustopher May 19, 2024
ca6e583
Add methods to publish and unpublish a project list based on permission
klaustopher May 19, 2024
1954629
Add feature flag for Project List sharing while in development
klaustopher May 21, 2024
76f76d6
Fix Rubocop
klaustopher May 21, 2024
539e297
Add a list item to the sidebar to show public lists
klaustopher May 21, 2024
e13a8f6
Add i18n keys for everything I added
klaustopher May 21, 2024
b19d1c0
Allow access to public queries for everyone
klaustopher May 21, 2024
8a450a4
Only redirect to a query when it is still visibil
klaustopher May 21, 2024
cb61339
Tests for changes to PRojectQuery model
klaustopher May 21, 2024
62c9535
Add a convenience show route that redirects to the queried page
klaustopher May 21, 2024
6a835f4
Allow editing public queries when you have the permission
klaustopher May 21, 2024
282d02b
Fix publishing with more knowledge about Contracts
klaustopher May 21, 2024
6988711
Fix accidentally commited merge
klaustopher May 21, 2024
438afa8
Fix specs for the factory by mocking the new query
klaustopher May 22, 2024
eef97fc
Fix the QueriesController so that the user is the proper owner of the…
klaustopher May 22, 2024
175bacf
Fix specs
klaustopher May 22, 2024
dae9450
Add conroller tests
klaustopher May 22, 2024
03b9411
Fix Lint/UselessAssignment errors
klaustopher May 22, 2024
1358215
Move feature decision check into the can_publish? method
klaustopher May 23, 2024
ca42c30
Use unless in favor of if !
klaustopher May 23, 2024
28d39f4
Use positional arguments for user in viible methods
klaustopher May 23, 2024
92a8327
Remove `can_rename?` method and alias it to can_save?
klaustopher May 23, 2024
3264517
Fix specs
klaustopher May 23, 2024
1ba1481
Reintroduce can_rename?
klaustopher May 23, 2024
f81c34b
Add a TODO about removing the temporary menu items
klaustopher May 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 23 additions & 1 deletion app/components/projects/index_page_header_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,29 @@
end

if query.persisted?
# TODO: Remove section when the sharing modal is implemented (https://community.openproject.org/projects/openproject/work_packages/55163)
if can_publish?
klaustopher marked this conversation as resolved.
Show resolved Hide resolved
if query.public?
menu.with_item(
label: t(:button_unpublish),
scheme: :danger,
href: unpublish_projects_query_path(query),
content_arguments: { data: { method: :post } }
) do |item|
item.with_leading_visual_icon(icon: 'eye-closed')
end
else
menu.with_item(
label: t(:button_publish),
scheme: :default,
klaustopher marked this conversation as resolved.
Show resolved Hide resolved
href: publish_projects_query_path(query),
content_arguments: { data: { method: :post } }
) do |item|
item.with_leading_visual_icon(icon: 'eye')
end
end
end

menu.with_item(
label: t(:button_delete),
scheme: :danger,
Expand All @@ -92,7 +115,6 @@
end
end
%>

<%= render(Projects::ConfigureViewModalComponent.new(query:)) %>
<%= render(Projects::DeleteListModalComponent.new(query:)) if query.persisted? %>
<%= render(Projects::ExportListModalComponent.new(query:)) %>
Expand Down
30 changes: 28 additions & 2 deletions app/components/projects/index_page_header_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,35 @@ def may_save_as? = current_user.logged?

def can_save_as? = may_save_as? && query.changed?

def can_save? = can_save_as? && query.persisted? && query.user == current_user
def can_save?
return false unless current_user.logged?
return false unless query.persisted?
return false unless query.changed?

def can_rename? = may_save_as? && query.persisted? && query.user == current_user && !query.changed?
if query.public?
current_user.allowed_globally?(:manage_public_project_queries)
else
query.user == current_user
end
end

def can_rename?
return false unless current_user.logged?
return false unless query.persisted?
return false if query.changed?

if query.public?
current_user.allowed_globally?(:manage_public_project_queries)
else
query.user == current_user
end
end

def can_publish?
klaustopher marked this conversation as resolved.
Show resolved Hide resolved
OpenProject::FeatureDecisions.project_list_sharing_active? &&
current_user.allowed_globally?(:manage_public_project_queries) &&
query.persisted?
end

def show_state?
state == :show
Expand Down
42 changes: 21 additions & 21 deletions app/components/projects/row_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,19 @@ def hierarchy

def favored
klaustopher marked this conversation as resolved.
Show resolved Hide resolved
render(Primer::Beta::IconButton.new(
icon: currently_favored? ? "star-fill" : "star",
scheme: :invisible,
mobile_icon: currently_favored? ? "star-fill" : "star",
size: :medium,
tag: :a,
tooltip_direction: :e,
href: helpers.build_favorite_path(project, format: :html),
data: { method: currently_favored? ? :delete : :post },
classes: currently_favored? ? "op-primer--star-icon " : "op-project-row-component--favorite",
label: currently_favored? ? I18n.t(:button_unfavorite) : I18n.t(:button_favorite),
aria: { label: currently_favored? ? I18n.t(:button_unfavorite) : I18n.t(:button_favorite) },
test_selector: 'project-list-favorite-button'
))
icon: currently_favored? ? "star-fill" : "star",
scheme: :invisible,
mobile_icon: currently_favored? ? "star-fill" : "star",
size: :medium,
tag: :a,
tooltip_direction: :e,
href: helpers.build_favorite_path(project, format: :html),
data: { method: currently_favored? ? :delete : :post },
classes: currently_favored? ? "op-primer--star-icon " : "op-project-row-component--favorite",
label: currently_favored? ? I18n.t(:button_unfavorite) : I18n.t(:button_favorite),
aria: { label: currently_favored? ? I18n.t(:button_unfavorite) : I18n.t(:button_favorite) },
test_selector: "project-list-favorite-button"
))
end

def currently_favored?
Expand Down Expand Up @@ -197,7 +197,7 @@ def column_css_class(column)
def additional_css_class(column)
if column.attribute == :name
"project--hierarchy #{project.archived? ? 'archived' : ''}"
elsif [:status_explanation, :description].include?(column.attribute)
elsif %i[status_explanation description].include?(column.attribute)
"project-long-text-container"
elsif custom_field_column?(column)
cf = column.custom_field
Expand Down Expand Up @@ -254,7 +254,7 @@ def more_menu_favorite_item
href: helpers.build_favorite_path(project, format: :html),
data: { method: :post },
label: I18n.t(:button_favorite),
aria: { label: I18n.t(:button_favorite) },
aria: { label: I18n.t(:button_favorite) }
}
end

Expand All @@ -269,7 +269,7 @@ def more_menu_unfavorite_item
data: { method: :delete },
classes: "op-primer--star-icon",
label: I18n.t(:button_unfavorite),
aria: { label: I18n.t(:button_unfavorite) },
aria: { label: I18n.t(:button_unfavorite) }
}
end

Expand Down Expand Up @@ -302,7 +302,7 @@ def more_menu_activity_item
scheme: :default,
icon: :check,
label: I18n.t(:label_project_activity),
href: project_activity_index_path(project, event_types: ["project_attributes"]),
href: project_activity_index_path(project, event_types: ["project_attributes"])
}
end
end
Expand All @@ -317,7 +317,7 @@ def more_menu_archive_item
data: {
confirm: t("project.archive.are_you_sure", name: project.name),
method: :post
},
}
}
end
end
Expand All @@ -340,7 +340,7 @@ def more_menu_copy_item
scheme: :default,
icon: :copy,
label: I18n.t(:button_copy),
href: copy_project_path(project),
href: copy_project_path(project)
}
end
end
Expand All @@ -351,7 +351,7 @@ def more_menu_delete_item
scheme: :danger,
icon: :trash,
label: I18n.t(:button_delete),
href: confirm_destroy_project_path(project),
href: confirm_destroy_project_path(project)
}
end
end
Expand All @@ -361,7 +361,7 @@ def user_can_view_project?
end

def custom_field_column?(column)
column.is_a?(Queries::Projects::Selects::CustomField)
column.is_a?(::Queries::Projects::Selects::CustomField)
end
end
end
24 changes: 11 additions & 13 deletions app/components/projects/table_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,10 @@ def build_sort_header(column, options)
# We don't return the project row
# but the [project, level] array from the helper
def rows
klaustopher marked this conversation as resolved.
Show resolved Hide resolved
@rows ||=
begin
projects_enumerator = ->(model) { to_enum(:projects_with_levels_order_sensitive, model).to_a }
instance_exec(model, &projects_enumerator)
end
@rows ||= begin
projects_enumerator = ->(model) { to_enum(:projects_with_levels_order_sensitive, model).to_a }
instance_exec(model, &projects_enumerator)
end
end

def initialize_sorted_model
Expand Down Expand Up @@ -113,15 +112,14 @@ def sortable_column?(select)
end

def columns
@columns ||=
begin
columns = query.selects.reject { |select| select.is_a?(Queries::Selects::NotExistingSelect) }
@columns ||= begin
columns = query.selects.reject { |select| select.is_a?(::Queries::Selects::NotExistingSelect) }

index = columns.index { |column| column.attribute == :name }
columns.insert(index, Queries::Projects::Selects::Default.new(:hierarchy)) if index
index = columns.index { |column| column.attribute == :name }
columns.insert(index, ::Queries::Projects::Selects::Default.new(:hierarchy)) if index

columns
end
columns
end
end

def projects(query)
Expand Down Expand Up @@ -156,7 +154,7 @@ def projects_with_level(projects, &)
end

def favored_project_ids
@favored_projects ||= Favorite.where(user: current_user, favored_type: 'Project').pluck(:favored_id)
@favored_project_ids ||= Favorite.where(user: current_user, favored_type: "Project").pluck(:favored_id)
end

def sorted_by_lft?
Expand Down
25 changes: 22 additions & 3 deletions app/contracts/queries/projects/project_queries/base_contract.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,36 @@ def self.model
presence: true,
length: { maximum: 255 }

validate :user_is_current_user_and_logged_in
validate :name_select_included
validate :existing_selects
validate :user_is_logged_in
validate :allowed_to_modify_private_query
validate :allowed_to_modify_public_query

protected

def user_is_current_user_and_logged_in
unless user.logged? && user == model.user
def user_is_logged_in
unless user.logged?
errors.add :base, :error_unauthorized
end
end

def allowed_to_modify_private_query
return if model.public?

if model.user != user
errors.add :base, :can_only_be_modified_by_owner
end
klaustopher marked this conversation as resolved.
Show resolved Hide resolved
end

def allowed_to_modify_public_query
return unless model.public?

unless user.allowed_globally?(:manage_public_project_queries)
errors.add :base, :need_permission_to_modify_public_query
end
toy marked this conversation as resolved.
Show resolved Hide resolved
end

def name_select_included
if model.selects.none? { |s| s.attribute == :name }
errors.add :selects, :name_not_included
Expand Down
33 changes: 33 additions & 0 deletions app/contracts/queries/projects/project_queries/publish_contract.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#-- 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 Queries::Projects::ProjectQueries
class PublishContract < BaseContract
attribute :public
end
end
60 changes: 38 additions & 22 deletions app/controllers/projects/queries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,17 @@ class Projects::QueriesController < ApplicationController

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

current_menu_item [:new, :rename, :create, :update] do
:projects
end

def show
redirect_to projects_path(query_id: @query.id)
end
Comment on lines +41 to +43
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the action just to redirect really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a few cases, where the browser encountered an error on the POST to /queries/1 and that then showed up as the URL in the browser, when refreshing I was met with a 404 error. Personally, I want to surprise the user the least, and if they refresh they should land on a page that makes sense and that what this redirect does

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you mean resubmitting the url from the url bar, as refresh of post POST by browsers should still be a POST request (discussion about it). Not sure it will reduce user surprise, as changes that were in post data will be gone


def new
render template: "/projects/index",
layout: "global",
Expand All @@ -55,35 +59,31 @@ def create
.new(from: @query, user: current_user)
.call(permitted_query_params)

if call.success?
flash[:notice] = I18n.t("lists.create.success")

redirect_to projects_path(query_id: call.result.id)
else
flash[:error] = I18n.t("lists.create.failure", errors: call.errors.full_messages.join("\n"))

render template: "/projects/index",
layout: "global",
locals: { query: call.result, state: :edit }
end
render_result(call, success_i18n_key: "lists.create.success", error_i18n_key: "lists.create.failure")
end

def update
call = Queries::Projects::ProjectQueries::UpdateService
.new(user: current_user, model: @query)
.call(permitted_query_params)

if call.success?
flash[:notice] = I18n.t("lists.update.success")
render_result(call, success_i18n_key: "lists.update.success", error_i18n_key: "lists.update.failure")
end

redirect_to projects_path(query_id: call.result.id)
else
flash[:error] = I18n.t("lists.update.failure", errors: call.errors.full_messages.join("\n"))
def publish
call = Queries::Projects::ProjectQueries::PublishService
.new(user: current_user, model: @query)
.call(public: true)

render template: "/projects/index",
layout: "global",
locals: { query: call.result, state: :edit }
end
render_result(call, success_i18n_key: "lists.publish.success", error_i18n_key: "lists.publish.failure")
end

def unpublish
call = Queries::Projects::ProjectQueries::PublishService
.new(user: current_user, model: @query)
.call(public: false)

render_result(call, success_i18n_key: "lists.unpublish.success", error_i18n_key: "lists.unpublish.failure")
end

def destroy
Expand All @@ -95,7 +95,23 @@ def destroy

private

def render_result(service_call, success_i18n_key:, error_i18n_key:) # rubocop:disable Metrics/AbcSize
modified_query = service_call.result

if service_call.success?
flash[:notice] = I18n.t(success_i18n_key)

redirect_to modified_query.visible? ? projects_path(query_id: modified_query.id) : projects_path
else
flash[:error] = I18n.t(error_i18n_key, errors: service_call.errors.full_messages.join("\n"))

render template: "/projects/index",
layout: "global",
locals: { query: modified_query, state: :edit }
end
end

def find_query
@query = Queries::Projects::ProjectQuery.find(params[:id])
@query = Queries::Projects::ProjectQuery.visible(current_user).find(params[:id])
end
end