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

feat: Filter pacticipant name on index page #446

Merged
merged 1 commit into from
Jul 6, 2021
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
16 changes: 8 additions & 8 deletions lib/pact_broker/hash_refinements.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ def snakecase_keys_private(params)
when Hash
params.inject({}) do |result, (key, value)|
snake_key = case key
when String then key.snakecase
when Symbol then key.to_s.snakecase.to_sym
else
key
when String then key.snakecase
when Symbol then key.to_s.snakecase.to_sym
else
key
end
result.merge(snake_key => snakecase_keys_private(value))
end
Expand All @@ -64,10 +64,10 @@ def camelcase_keys_private(params)
when Hash
params.inject({}) do |result, (key, value)|
snake_key = case key
when String then key.camelcase
when Symbol then key.to_s.camelcase.to_sym
else
key
when String then key.camelcase
when Symbol then key.to_s.camelcase.to_sym
else
key
end
result.merge(snake_key => camelcase_keys_private(value))
end
Expand Down
73 changes: 51 additions & 22 deletions lib/pact_broker/index/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,31 +142,29 @@ def self.db
end

def self.head_pact_publications(options = {})
base = PactBroker::Pacts::PactPublication.select(Sequel[:pact_publications][:id])
base = base_query(options)

if options[:consumer_name]
consumer = pacticipant_repository.find_by_name!(options[:consumer_name])
base = base.for_consumer(consumer)
end
if options[:pacticipant_name]
pacticipant_ids = pact_pacticipant_ids_by_name(options[:pacticipant_name])
base = base.where(Sequel.|(
{ Sequel[:pact_publications][:consumer_id] => pacticipant_ids },
{ Sequel[:pact_publications][:provider_id] => pacticipant_ids }
))

if options[:provider_name]
provider = pacticipant_repository.find_by_name!(options[:provider_name])
base = base.for_provider(provider)
# Return early if there is no pacticipant matches the input name
return base.paginate(options[:page_number] || DEFAULT_PAGE_NUMBER, options[:page_size] || DEFAULT_PAGE_SIZE) if pacticipant_ids.blank?
end

latest = base.overall_latest
ids_query = if options[:tags].is_a?(Array)
latest.union(base.latest_for_consumer_tag(options[:tags]))
elsif options[:tags]
latest.union(base.latest_by_consumer_tag)
else
latest
end

query = PactBroker::Pacts::PactPublication.select_all_qualified.where(Sequel[:pact_publications][:id] => ids_query)
.join_consumers(:consumers)
.join_providers(:providers)
.join(:versions, { Sequel[:pact_publications][:consumer_version_id] => Sequel[:cv][:id] }, { table_alias: :cv } )
ids_query = query_pact_publication_ids_by_tags(base, options[:tags])
query = PactBroker::Pacts::PactPublication
.select_all_qualified
.where(Sequel[:pact_publications][:id] => ids_query)
.join_consumers(:consumers)
.join_providers(:providers)
.join(:versions,
{ Sequel[:pact_publications][:consumer_version_id] => Sequel[:cv][:id] },
{ table_alias: :cv }
)

order_columns = [
Sequel.asc(Sequel.function(:lower, Sequel[:consumers][:name])),
Expand All @@ -175,7 +173,7 @@ def self.head_pact_publications(options = {})
]

query.order(*order_columns)
.paginate(options[:page_number] || DEFAULT_PAGE_NUMBER, options[:page_size] || DEFAULT_PAGE_SIZE)
.paginate(options[:page_number] || DEFAULT_PAGE_NUMBER, options[:page_size] || DEFAULT_PAGE_SIZE)
end

# eager loading the tag stuff doesn't seem to make it quicker
Expand All @@ -194,6 +192,37 @@ def self.latest_verifications_for_consumer_version_tags(options)
nil # should not be used
end
end

def self.query_pact_publication_ids_by_tags(base, tags)
latest = base.overall_latest
return latest.union(base.latest_for_consumer_tag(tags)) if tags.is_a?(Array)
return latest.union(base.latest_by_consumer_tag) if tags

latest
end

def self.base_query(options)
query = PactBroker::Pacts::PactPublication.select(Sequel[:pact_publications][:id])

if options[:consumer_name]
consumer = pacticipant_repository.find_by_name!(options[:consumer_name])
query = query.for_consumer(consumer)
end

if options[:provider_name]
provider = pacticipant_repository.find_by_name!(options[:provider_name])
query = query.for_provider(provider)
end

query
end


def self.pact_pacticipant_ids_by_name(pacticipant_name)
pacticipant_repository.search_by_name(pacticipant_name).collect(&:id)
end

private_class_method :base_query, :query_pact_publication_ids_by_tags, :pact_pacticipant_ids_by_name
end
end
end
2 changes: 2 additions & 0 deletions lib/pact_broker/locale/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ en:
pact_broker:
messages:
response_body_hidden: For security purposes, the response details are not logged. To enable response logging, configure the webhook_host_whitelist property. See %{base_url}/doc/webhooks#whitelist for more information.
index:
title: Pacts
matrix:
pre_verified: This pact was "pre-verified" as it has identical content to a previously verified pact.
webhooks:
Expand Down
7 changes: 6 additions & 1 deletion lib/pact_broker/pacticipants/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ def replace(_pacticipant_name, open_struct_pacticipant)
def pacticipant_names
PactBroker::Domain::Pacticipant.select(:name).order(:name).collect(&:name)
end

def delete_if_orphan(pacticipant)
if PactBroker::Domain::Version.where(pacticipant: pacticipant).empty? &&
PactBroker::Pacts::PactPublication.where(provider: pacticipant).or(consumer: pacticipant).empty? &&
Expand All @@ -94,6 +93,12 @@ def handle_multiple_pacticipants_found(name, pacticipants)
names = pacticipants.collect(&:name).join(", ")
raise PactBroker::Error.new("Found multiple pacticipants with a case insensitive name match for '#{name}': #{names}. Please delete one of them, or set PactBroker.configuration.use_case_sensitive_resource_names = true")
end

def search_by_name(pacticipant_name)
terms = pacticipant_name.split.map { |v| v.gsub("_", '\\_') }
string_match_query = Sequel.|( *terms.map { |term| Sequel.ilike(Sequel[:pacticipants][:name], "%#{term}%") })
PactBroker::Domain::Pacticipant.where(string_match_query)
end
end
end
end
37 changes: 25 additions & 12 deletions lib/pact_broker/ui/controllers/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,48 @@ class Index < Base

get "/" do
set_headers
tags = nil
if params[:tags]
tags = params[:tags] == "true" ? true : [*params[:tags]].compact
end
tags = if params[:tags]
params[:tags] == "true" ? true : [*params[:tags]].compact
end
pacticipant_name = params[:pacticipant_name].present? ? params[:pacticipant_name] : nil
page_number = params[:page]&.to_i || 1
# Make page size smaller for data intensive query
page_size = params[:pageSize]&.to_i || (tags == true ? 30 : 100)
options = {
tags: tags,
page_number: page_number,
page_size: page_size
}
page_size: page_size,
pacticipant_name: pacticipant_name
}.compact
error_messages = []

index_items = index_service.find_index_items(options)

index_items = ViewDomain::IndexItems.new(index_service.find_index_items(options), base_url: base_url)
if index_items.blank? && !pacticipant_name.blank?
error_messages << "Pacticipant's name: \"#{pacticipant_name}\" cannot be found"
end

view_index_items = ViewDomain::IndexItems.new(index_items, base_url: base_url)

page = tags ? :'index/show-with-tags' : :'index/show'
locals = {
title: "Pacts",
index_items: index_items,
title: PactBroker::Messages.message("messages.index.title"),
index_items: view_index_items,
page_number: page_number,
page_size: page_size,
pagination_record_count: index_items.pagination_record_count,
current_page_size: index_items.size,
base_url: base_url
pagination_record_count: view_index_items.pagination_record_count,
current_page_size: view_index_items.size,
base_url: base_url,
errors: error_messages,
tags: tags,
pacticipant_name: pacticipant_name
}

haml page, { locals: locals, layout: :'layouts/main', escape_html: true }
end

private

def set_headers
response.headers["Cache-Control"] = "no-cache, no-store, must-revalidate"
response.headers["Pragma"] = "no-cache"
Expand Down
2 changes: 1 addition & 1 deletion lib/pact_broker/ui/view_models/index_items.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,4 @@ def size
end
end
end
end
end
37 changes: 36 additions & 1 deletion lib/pact_broker/ui/views/index/show-with-tags.haml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,24 @@
!= render :haml, :'index/_css_and_js', :layout => false
.container
!= render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: false, base_url: base_url}
- if index_items.empty?
- if index_items.empty? && pacticipant_name.blank?
!= render :haml, :'index/_getting-started', :layout => false
%h1.page-header
Pacts

- unless errors.blank?
- errors.each do | error |
%div.alert.alert-danger
= error

%form{action: "#{base_url}"}
.field
%label{for: 'pacticipant_name'}
Search
%input{name: 'pacticipant_name', id: 'pacticipant_name', class: 'pacticipant-name', value: pacticipant_name}
%input{type: 'button', class: 'submit-search', value: 'Submit'}
%input{type: 'button', class: 'reset-search', value: 'Reset' }

%table.table.table-bordered.table-striped{ id: 'relationships' }
%thead
%tr
Expand Down Expand Up @@ -118,3 +132,24 @@
$(td).tooltip({container: $(td).first()});
});
});

$(".reset-search").on("click", function() {
location = window.location.href
window.location.replace("#{base_url}/?tags=#{tags}");
})

$(".submit-search").on("click", function() {
pacticipant_name = $("#pacticipant_name").val();
window.location.replace("#{base_url}/?tags=#{tags}" + `&pacticipant_name=${pacticipant_name}`);
})

$(".pacticipant-name").keypress(function(event) {
const enterKeyCode = 13;

const key = event.which;
if (key === enterKeyCode) {
event.preventDefault();
pacticipant_name = $("#pacticipant_name").val();
window.location.replace("#{base_url}/?tags=#{tags}" + `&pacticipant_name=${pacticipant_name}`);
}
})
20 changes: 19 additions & 1 deletion lib/pact_broker/ui/views/index/show.haml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,24 @@
!= render :haml, :'index/_css_and_js', :layout => false
.container
!= render :haml, :'index/_navbar', :layout => false, locals: {tag_toggle: true, base_url: base_url}
- if index_items.empty?
- if index_items.empty? && pacticipant_name.blank?
!= render :haml, :'index/_getting-started', :layout => false
%h1.page-header
Pacts

- unless errors.blank?
- errors.each do | error |
%div.alert.alert-danger
= error

%form{action: "#{base_url}", onsubmit:'return onSubmit()'}
.field
%label{for: 'pacticipant_name'}
Search
%input{name: 'pacticipant_name', id: 'pacticipant_name', class: 'pacticipant_name', value: pacticipant_name}
%input{type: 'submit', value: 'Submit'}
%input{type: 'button', class: 'reset-search', value: 'Reset' }

%table.table.table-bordered.table-striped{ id: 'relationships' }
%thead
%tr
Expand Down Expand Up @@ -75,3 +89,7 @@
$(td).tooltip({container: $(td).first()});
});
});

$(".reset-search").on("click", function() {
window.location.replace(window.location.origin);
})
10 changes: 10 additions & 0 deletions public/stylesheets/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,13 @@ span.copy-success-icon {
width: 16px;
height: 16px;
}

.field {
margin-top: 15px;
margin-bottom: 15px;
}

div.top-of-group {
margin-top: 20px;
margin-bottom: 20px;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I test it locally, looks like the change in the index.css file does not
get reflected
Copy those changes and put into Firefox css style editor result in the image in
the PR description

Im not sure what I did wrong which made the index.css changes not get copied
to the server asset. Any hint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

Hm, I can't think of anything to be honest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is fixed now. Im not sure why it did not work before 🤷

1 change: 0 additions & 1 deletion spec/lib/pact_broker/index/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,6 @@ module Index
end
end
end

end
end

Expand Down
42 changes: 42 additions & 0 deletions spec/lib/pact_broker/pacticipants/repository_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,48 @@ module Pacticipants
end
end
end

describe "#search_by_name" do
let(:consumer_name) { "This is_a test-consumer" }
let(:provider_name) { "and a test/provider" }

before do
td
.create_consumer(consumer_name)
.create_consumer(provider_name)
end

context "when there is a consumer/provider name which matches the search term" do
it "returns the pacticipants" do
searched_dataset = Repository.new.search_by_name "consumer"
expect(searched_dataset.collect(&:name)).to eq([consumer_name])

searched_dataset = Repository.new.search_by_name "provider"
expect(searched_dataset.collect(&:name)).to eq([provider_name])

searched_dataset = Repository.new.search_by_name "test"
expect(searched_dataset.collect(&:name)).to include(*[consumer_name, provider_name])
end

# SQL escape character is '_'
it "escapes the '_' character" do
searched_dataset = Repository.new.search_by_name "is_a"
expect(searched_dataset.collect(&:name)).to eq([consumer_name])
end

it "searches case insentively" do
searched_dataset = Repository.new.search_by_name "TEST"
expect(searched_dataset.collect(&:name)).to include(*[consumer_name, provider_name])
end
end

context "when there is NO consumer/provider name which matches the search term" do
it "returns empty dataset" do
searched_dataset = Repository.new.search_by_name "this_will_not_yield_any_result"
expect(searched_dataset.count).to eq(0)
end
end
end
end
end
end
Loading