From fd882da4c9897cbad2f447fa87fdb8f7e0d559c0 Mon Sep 17 00:00:00 2001 From: bangn Date: Tue, 6 Jul 2021 10:32:28 +1000 Subject: [PATCH] feat: filter pacticipant name on index page (#446) --- lib/pact_broker/hash_refinements.rb | 16 ++-- lib/pact_broker/index/service.rb | 73 +++++++++++++------ lib/pact_broker/locale/en.yml | 2 + lib/pact_broker/pacticipants/repository.rb | 7 +- lib/pact_broker/ui/controllers/index.rb | 37 +++++++--- lib/pact_broker/ui/view_models/index_items.rb | 2 +- .../ui/views/index/show-with-tags.haml | 37 +++++++++- lib/pact_broker/ui/views/index/show.haml | 20 ++++- public/stylesheets/index.css | 10 +++ spec/lib/pact_broker/index/service_spec.rb | 1 - .../pacticipants/repository_spec.rb | 42 +++++++++++ .../pact_broker/ui/controllers/index_spec.rb | 43 ++++++++++- 12 files changed, 241 insertions(+), 49 deletions(-) diff --git a/lib/pact_broker/hash_refinements.rb b/lib/pact_broker/hash_refinements.rb index 06f737fb3..d1d2e03fe 100644 --- a/lib/pact_broker/hash_refinements.rb +++ b/lib/pact_broker/hash_refinements.rb @@ -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 @@ -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 diff --git a/lib/pact_broker/index/service.rb b/lib/pact_broker/index/service.rb index 9b2428085..1b72848fe 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -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])), @@ -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 @@ -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 diff --git a/lib/pact_broker/locale/en.yml b/lib/pact_broker/locale/en.yml index 6a736e893..8914ee1c8 100644 --- a/lib/pact_broker/locale/en.yml +++ b/lib/pact_broker/locale/en.yml @@ -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: diff --git a/lib/pact_broker/pacticipants/repository.rb b/lib/pact_broker/pacticipants/repository.rb index 17789e5af..4a06e82d7 100644 --- a/lib/pact_broker/pacticipants/repository.rb +++ b/lib/pact_broker/pacticipants/repository.rb @@ -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? && @@ -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 diff --git a/lib/pact_broker/ui/controllers/index.rb b/lib/pact_broker/ui/controllers/index.rb index 178b38701..24db3b15f 100644 --- a/lib/pact_broker/ui/controllers/index.rb +++ b/lib/pact_broker/ui/controllers/index.rb @@ -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" diff --git a/lib/pact_broker/ui/view_models/index_items.rb b/lib/pact_broker/ui/view_models/index_items.rb index dd825e46c..733cb7610 100644 --- a/lib/pact_broker/ui/view_models/index_items.rb +++ b/lib/pact_broker/ui/view_models/index_items.rb @@ -33,4 +33,4 @@ def size end end end -end \ No newline at end of file +end diff --git a/lib/pact_broker/ui/views/index/show-with-tags.haml b/lib/pact_broker/ui/views/index/show-with-tags.haml index 2e4aef3d0..d63b4da5d 100644 --- a/lib/pact_broker/ui/views/index/show-with-tags.haml +++ b/lib/pact_broker/ui/views/index/show-with-tags.haml @@ -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 @@ -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}`); + } + }) diff --git a/lib/pact_broker/ui/views/index/show.haml b/lib/pact_broker/ui/views/index/show.haml index 85aefe339..663adedb2 100644 --- a/lib/pact_broker/ui/views/index/show.haml +++ b/lib/pact_broker/ui/views/index/show.haml @@ -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 @@ -75,3 +89,7 @@ $(td).tooltip({container: $(td).first()}); }); }); + + $(".reset-search").on("click", function() { + window.location.replace(window.location.origin); + }) diff --git a/public/stylesheets/index.css b/public/stylesheets/index.css index 6adf1dd57..6fd9e1366 100644 --- a/public/stylesheets/index.css +++ b/public/stylesheets/index.css @@ -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; +} diff --git a/spec/lib/pact_broker/index/service_spec.rb b/spec/lib/pact_broker/index/service_spec.rb index 777c5a713..484f0db42 100644 --- a/spec/lib/pact_broker/index/service_spec.rb +++ b/spec/lib/pact_broker/index/service_spec.rb @@ -359,7 +359,6 @@ module Index end end end - end end diff --git a/spec/lib/pact_broker/pacticipants/repository_spec.rb b/spec/lib/pact_broker/pacticipants/repository_spec.rb index d37861340..78ff9d341 100644 --- a/spec/lib/pact_broker/pacticipants/repository_spec.rb +++ b/spec/lib/pact_broker/pacticipants/repository_spec.rb @@ -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 diff --git a/spec/lib/pact_broker/ui/controllers/index_spec.rb b/spec/lib/pact_broker/ui/controllers/index_spec.rb index 37c560175..287bfe0cd 100644 --- a/spec/lib/pact_broker/ui/controllers/index_spec.rb +++ b/spec/lib/pact_broker/ui/controllers/index_spec.rb @@ -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 @@ -85,6 +93,37 @@ module Controllers get "/", { tags: "prod" } end end + + context "when parameter pacticipant_name presents" do + context "when it is blank" do + it "ignores it" do + get "/", { pacticipant_name_name: "" } + + expect(last_response.body).to include("Example App") + expect(last_response.status).to eq(200) + end + end + + context "when it is NOT blank and the pacticipant name exists" do + it "returns the pacticipant which matches the query" do + get "/", { pacticipant_name: "app" } + + expect(last_response.body).to include("Example 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 "returns no pacts" do + get "/", { pacticipant_name: "does not exist" } + + expect(last_response.body).not_to include("does not exit") + expect(last_response.body).not_to include("Example App") + expect(last_response.body).not_to include("Test App") + expect(last_response.status).to eq(200) + end + end + end end end end