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

Switch empty state when public flag is toggled #16036

Merged
merged 10 commits into from
Jul 9, 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: 1 addition & 1 deletion app/components/shares/modal_body_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def initialize(strategy:, shares:, errors: nil)
end

def self.wrapper_key
"share_list"
"share_modal_body"
end

private
Expand Down
2 changes: 1 addition & 1 deletion app/components/shares/modal_upsale_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ModalUpsaleComponent < ApplicationComponent # rubocop:disable OpenProject/
include OpPrimer::ComponentHelpers

def self.wrapper_key
"share_list"
"share_modal_body"
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def public_blankslate_config

def unfiltered_blankslate_config
{
icon: :people,
icon: "share-android",
heading_text: I18n.t("sharing.project_queries.blank_state.private.header"),
description_text: I18n.t("sharing.project_queries.blank_state.private.description")
}
Expand Down
19 changes: 16 additions & 3 deletions app/controllers/projects/queries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

class Projects::QueriesController < ApplicationController
include Projects::QueryLoading
include OpTurbo::ComponentStream

# 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
Expand Down Expand Up @@ -71,15 +72,27 @@ def update
render_result(call, success_i18n_key: "lists.update.success", error_i18n_key: "lists.update.failure")
end

def toggle_public
to_be_public = [email protected]?
def toggle_public # rubocop:disable Metrics/AbcSize
to_be_public = ActiveRecord::Type::Boolean.new.cast(params["value"])
i18n_key = to_be_public ? "lists.publish" : "lists.unpublish"

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

render_result(call, success_i18n_key: "#{i18n_key}.success", error_i18n_key: "#{i18n_key}.failure")
respond_to do |format|
format.turbo_stream do
# Load shares and replace the modal
strategy = SharingStrategies::ProjectQueryStrategy.new(@query, user: current_user)
shares = strategy.shares_query({}).results
replace_via_turbo_stream(component: Shares::ModalBodyComponent.new(strategy:, shares:, errors: []))
toy marked this conversation as resolved.
Show resolved Hide resolved
render turbo_stream: turbo_streams, status:
end

format.html do
render_result(call, success_i18n_key: "#{i18n_key}.success", error_i18n_key: "#{i18n_key}.failure")
end
end
end

def destroy
Expand Down
17 changes: 1 addition & 16 deletions app/controllers/shares_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -326,22 +326,7 @@ def current_visible_member_count
end

def load_query
return @query if defined?(@query)

@query = ParamsToQueryService
.new(Member, current_user, query_class: Queries::Members::NonInheritedMemberQuery)
.call(params)

# Set default filter on the entity
@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
@query = sharing_strategy.shares_query(params)
end

def load_shares
Expand Down
19 changes: 19 additions & 0 deletions app/models/sharing_strategies/base_strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,24 @@ def custom_empty_state_component?
def empty_state_component
nil
end

def shares_query(params) # rubocop:disable Metrics/AbcSize
return @query if defined?(@query)

@query = ParamsToQueryService
.new(Member, user, query_class: Queries::Members::NonInheritedMemberQuery)
.call(params)

# Set default filter on the entity
@query.where("entity_id", "=", entity.id)
@query.where("entity_type", "=", entity.class.name)
if entity.respond_to?(:project)
@query.where("project_id", "=", entity.project.id)
end

@query.order(name: :asc) unless params[:sortBy]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe check for params is not needed as calls to order are appended

Copy link
Contributor Author

@klaustopher klaustopher Jul 9, 2024

Choose a reason for hiding this comment

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

The default order for a member is created_at, so it makes sense to reorder here depending on params being present


@query
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
<div class="spot-modal--body spot-container">
<turbo-frame
#frameElement
id="share_list"
id="share_modal_body"
src="{{this.frameSrc}}">
<op-content-loader
viewBox="0 0 180 80"
Expand Down
91 changes: 52 additions & 39 deletions spec/controllers/projects/queries_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -267,70 +267,83 @@
let(:service_class) { Queries::Projects::ProjectQueries::PublishService }

it "requires login" do
post "toggle_public", params: { id: 42 }
post "toggle_public", params: { id: "42", value: "1" }

expect(response).not_to be_successful
end

context "when logged in" do
let(:query) { build_stubbed(:project_query, user:) }
let(:query_id) { "42" }
let(:query_params) { { public: true } }
let(:service_instance) { instance_double(service_class) }
let(:service_result) { instance_double(ServiceResult, success?: success?, result: query) }
let(:success?) { true }

before do
allow(controller).to receive(:permitted_query_params).and_return(query_params)
scope = instance_double(ActiveRecord::Relation)
allow(ProjectQuery).to receive(:visible).and_return(scope)
allow(scope).to receive(:find).with(query_id).and_return(query)
allow(service_class).to receive(:new).with(model: query, user:).and_return(service_instance)
allow(service_instance).to receive(:call).with(query_params).and_return(service_result)
[true, false].each do |public_value|
context "when toggling to #{public_value ? 'public' : 'private'}" do
let(:query_params) { { public: public_value } }
let(:params) do
{
id: query_id,
value: public_value ? "1" : "0"
}
end

login_as user
end
let(:i18n_scope) { public_value ? "lists.publish" : "lists.unpublish" }

it "calls publish service on query" do
post "toggle_public", params: { id: 42 }
before do
allow(controller).to receive(:permitted_query_params).and_return(query_params)
scope = instance_double(ActiveRecord::Relation)
allow(ProjectQuery).to receive(:visible).and_return(scope)
allow(scope).to receive(:find).with(query_id).and_return(query)
allow(service_class).to receive(:new).with(model: query, user:).and_return(service_instance)
allow(service_instance).to receive(:call).with(query_params).and_return(service_result)

expect(service_instance).to have_received(:call).with(query_params)
end
login_as user
end

context "when service call succeeds" do
it "redirects to projects" do
allow(I18n).to receive(:t).with("lists.publish.success").and_return("foo")
it "calls publish service on query" do
post("toggle_public", params:)

post "toggle_public", params: { id: 42 }
expect(service_instance).to have_received(:call).with(query_params)
end

expect(flash[:notice]).to eq("foo")
expect(response).to redirect_to(projects_path(query_id: query.id))
end
end
context "when service call succeeds" do
it "redirects to projects" do
allow(I18n).to receive(:t).with("#{i18n_scope}.success").and_return("foo")

context "when service call fails" do
let(:success?) { false }
let(:errors) { instance_double(ActiveModel::Errors, full_messages: ["something", "went", "wrong"]) }
post("toggle_public", params:)

before do
allow(service_result).to receive(:errors).and_return(errors)
end
expect(flash[:notice]).to eq("foo")
expect(response).to redirect_to(projects_path(query_id: query.id))
end
end

it "renders projects/index" do
allow(I18n).to receive(:t).with("lists.publish.failure", errors: "something\nwent\nwrong").and_return("bar")
context "when service call fails" do
let(:success?) { false }
let(:errors) { instance_double(ActiveModel::Errors, full_messages: ["something", "went", "wrong"]) }

post "toggle_public", params: { id: 42 }
before do
allow(service_result).to receive(:errors).and_return(errors)
end

expect(flash[:error]).to eq("bar")
expect(response).to render_template("projects/index")
end
it "renders projects/index" do
allow(I18n).to receive(:t).with("#{i18n_scope}.failure", errors: "something\nwent\nwrong").and_return("bar")

it "passes variables to template" do
allow(controller).to receive(:render).and_call_original
post("toggle_public", params:)

post "toggle_public", params: { id: 42 }
expect(flash[:error]).to eq("bar")
expect(response).to render_template("projects/index")
end

expect(controller).to have_received(:render).with(include(locals: { query:, state: :edit }))
it "passes variables to template" do
allow(controller).to receive(:render).and_call_original

post("toggle_public", params:)

expect(controller).to have_received(:render).with(include(locals: { query:, state: :edit }))
end
end
end
end
end
Expand Down
Loading