Skip to content

Commit

Permalink
Filter pacticipant name using string matching
Browse files Browse the repository at this point in the history
  • Loading branch information
bangn committed Jun 11, 2021
1 parent 2c03a25 commit 94ca4e2
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 88 deletions.
16 changes: 10 additions & 6 deletions lib/pact_broker/index/service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,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 @@ -195,13 +195,17 @@ def self.latest_verifications_for_consumer_version_tags(options)
end
end

def self.validate_pacticipant(pacticipant_type, pacticipant_name)
error_messages = []

pacticipant = pacticipant_service.find_pacticipant_by_name(pacticipant_name)
def self.filter_item_by_pacticipant_name(index_items, pacticipant_name)
filtered_items = index_items.select do |item|
string_match?(item.consumer_name, pacticipant_name) || string_match?(item.provider_name, pacticipant_name)
end
Page.new(filtered_items, filtered_items.size)
end

error_messages << "#{pacticipant_type} #{pacticipant_name} cannot be found" unless pacticipant
def self.string_match?(pacticipant_name, str)
pacticipant_name.downcase.include?(str.downcase)
end
private_class_method :string_match?
end
end
end
33 changes: 10 additions & 23 deletions lib/pact_broker/ui/controllers/index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,39 +9,36 @@ class Index < Base
include PactBroker::Services

get "/" do
set_headers
tags = if params[:tags]
params[:tags] == "true" ? true : [*params[:tags]].compact
end
pacticipant_name = params[:pacticipant_name]
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
}

error_messages = pacticipants.reduce([]) do |agg, (pacticipant_type, pacticipant_name)|
agg << index_service.validate_pacticipant(pacticipant_type, pacticipant_name)
end.flatten

options.merge!(pacticipants) if error_messages.blank?
page_size: page_size,
pacticipant_name: pacticipant_name
}.compact
error_messages = []

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

page = tags ? :'index/show-with-tags' : :'index/show'
locals = {
title: PactBroker::Messages.message("messages.index.title"),
index_items: index_items,
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,
pagination_record_count: view_index_items.pagination_record_count,
current_page_size: view_index_items.size,
base_url: base_url,
errors: error_messages
}

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

Expand All @@ -52,16 +49,6 @@ def set_headers
response.headers["Pragma"] = "no-cache"
response.headers["Expires"] = "0"
end

def pacticipants
consumer_name = params[:consumer_name].blank? ? nil : params[:consumer_name]
provider_name = params[:provider_name].blank? ? nil : params[:provider_name]

@pacticipants ||= {
consumer_name: consumer_name,
provider_name: provider_name,
}.compact
end
end
end
end
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
12 changes: 4 additions & 8 deletions lib/pact_broker/ui/views/index/show.haml
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,11 @@

%form{action: "#{base_url}", onsubmit:'return onSubmit()'}
.field
%label{for: "consumer_name"}
Consumer name
%input{name: 'consumer_name', id: "consumer_name", class: 'pacticipant_name'}
.field
%label{for: "provider_name"}
Provider name
%input{name: 'provider_name', id: "provider_name", class: 'pacticipant_name'}
%label{for: 'pacticipant_name'}
Pacticipant name
%input{name: 'pacticipant_name', id: 'pacticipant_name', class: 'pacticipant_name'}
%div.top-of-group
%input{type: 'submit'}
%input{type: 'submit', value: 'Search'}

%table.table.table-bordered.table-striped{ id: 'relationships' }
%thead
Expand Down
38 changes: 25 additions & 13 deletions spec/lib/pact_broker/index/service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -389,23 +389,35 @@ module Index
end
end

describe ".validate_pacticipant" do
before { td.create_pact_with_hierarchy }
describe ".filter_item_by_pacticipant_name" do
let(:pact_1) { double(PactBroker::Pacts::PactPublication, consumer_name: "Consumer_1", provider_name: "Provider_2") }
let(:pact_2) { double(PactBroker::Pacts::PactPublication, consumer_name: "Consumer_2", provider_name: "Provider_2") }

context "when validating non existing pacticipant" do
it "returns an error" do
non_existing_pacticipant_name = "no name"
non_existing_pacticipant_type = :pacticipant_type
error_messages = subject.validate_pacticipant(non_existing_pacticipant_type, non_existing_pacticipant_name)
expect(error_messages.count).to eq(1)
expect(error_messages.first).to eq("pacticipant_type no name cannot be found")
context "when pacticipant name matches pact's provider name or consumer name" do
let(:matched_pacticipant_name) { "consumer" }

before do
allow(subject).to receive(:string_match?).with(anything, matched_pacticipant_name).and_return(true)
end

it "returns an Index::Page with correct number of selected items" do
result = subject.filter_item_by_pacticipant_name([pact_1, pact_2], matched_pacticipant_name)
expect(result).is_a?(::PactBroker::Index::Page)
expect(result.count).to eq(2)
end
end

context "when validating existing pacticipant" do
it "returns no error" do
error_messages = subject.validate_pacticipant(:consumer_name, "Consumer")
expect(error_messages).to be_nil
context "when pacticipant name does NOT matches pact's provider name or consumer name" do
let(:not_matched_pacticipant_name) { "will not match" }

before do
allow(subject).to receive(:string_match?).with(anything, not_matched_pacticipant_name).and_return(false)
end

it "returns an Index::Page with emtpy items in it" do
result = subject.filter_item_by_pacticipant_name([pact_1, pact_2], not_matched_pacticipant_name)
expect(result).is_a?(::PactBroker::Index::Page)
expect(result.count).to eq(0)
end
end
end
Expand Down
64 changes: 27 additions & 37 deletions spec/lib/pact_broker/ui/controllers/index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,16 @@ module Controllers

before do
TestDataBuilder.new
.create_consumer
.create_provider
.create_consumer("Test App")
.create_provider("Test API")
.create_consumer_version
.create_pact
.create_webhook
.create_verification

TestDataBuilder.new
.create_consumer("Example App")
.create_provider("Example API")
.create_consumer_version
.create_pact
.create_webhook
Expand Down Expand Up @@ -86,52 +94,34 @@ module Controllers
end
end

context "when parameter consumer_name presents" do
context "when parameter pacticipant_name presents" do
context "when it is blank" do
it "ignores it" do
expect(PactBroker::Index::Service).not_to receive(:find_index_items).with(hash_including(:consumer_name))
get "/", { consumer_name: "" }
expect(PactBroker::Index::Service).not_to receive(:filter_item_by_pacticipant_name)
get "/", { pacticipant_name_name: "" }
end
end

context "when it is NOT blank and the pacticipant name exist" do
it "passes consumer_name to the service" do
allow(PactBroker::Index::Service).to receive(:validate_pacticipant).and_return([])
expect(PactBroker::Index::Service).to receive(:find_index_items).with(hash_including(consumer_name: "Consumer"))
get "/", { consumer_name: "Consumer" }
end
end

context "when it is NOT blank but the pacticipant name does NOT exist" do
it "passes consumer_name to the service" do
allow(PactBroker::Index::Service).to receive(:validate_pacticipant).and_return(["pacticipant does not exist"])
expect(PactBroker::Index::Service).not_to receive(:find_index_items).with(hash_including(:consumer_name))
get "/", { consumer_name: "Consumer" }
end
end
end

context "when parameter provider_name presents" do
context "when it is blank" do
it "ignores it" do
expect(PactBroker::Index::Service).not_to receive(:find_index_items).with(hash_including(:provider_name))
get "/", { provider_name: "" }
end
end
it "filters out the pacticipant which name does not match the pacticipant name" do
expect(PactBroker::Index::Service).to receive(:filter_item_by_pacticipant_name).and_call_original
get "/", { pacticipant_name: "example app" }

context "when it is NOT blank and the pacticipant exists" do
it "passes provider_name to the service" do
allow(PactBroker::Index::Service).to receive(:validate_pacticipant).and_return([])
expect(PactBroker::Index::Service).to receive(:find_index_items).with(hash_including(provider_name: "Provider"))
get "/", { provider_name: "Provider" }
expect(last_response.body).to include("Example App")
expect(last_response.body).not_to include("Test App")
expect(last_response.status).to eq(200)
end
end

context "when it is NOT blank but the pacticipant name does NOT exist" do
it "passes provider_name to the service" do
allow(PactBroker::Index::Service).to receive(:validate_pacticipant).and_return(["pacticipant does not exist"])
expect(PactBroker::Index::Service).not_to receive(:find_index_items).with(hash_including(:provider_name))
get "/", { provider_name: "Provider" }
it "returns all the pact items" do
expect(PactBroker::Index::Service).to receive(:filter_item_by_pacticipant_name).and_call_original
get "/", { pacticipant_name: "does not exist" }

expect(last_response.body).not_to include("does not exit")
expect(last_response.body).to include("Example App")
expect(last_response.body).to include("Test App")
expect(last_response.status).to eq(200)
end
end
end
Expand Down

0 comments on commit 94ca4e2

Please sign in to comment.