From 80388e9e5a39c9e000d4783ce815391a9fadc612 Mon Sep 17 00:00:00 2001 From: bangn Date: Thu, 3 Jun 2021 02:10:30 +1000 Subject: [PATCH] feat: Filter pacticipant name on index page --- lib/pact_broker/hash_refinements.rb | 16 ++-- lib/pact_broker/index/service.rb | 79 +++++++++++++------ lib/pact_broker/locale/en.yml | 2 + lib/pact_broker/ui/controllers/index.rb | 36 ++++++--- lib/pact_broker/ui/view_models/index_items.rb | 2 +- lib/pact_broker/ui/views/index/show.haml | 16 +++- public/stylesheets/index.css | 14 ++++ spec/lib/pact_broker/index/service_spec.rb | 1 - .../pact_broker/ui/controllers/index_spec.rb | 43 +++++++++- 9 files changed, 161 insertions(+), 48 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 564440ce6..542c7a6a2 100644 --- a/lib/pact_broker/index/service.rb +++ b/lib/pact_broker/index/service.rb @@ -142,31 +142,22 @@ 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[:provider_name] - provider = pacticipant_repository.find_by_name!(options[:provider_name]) - base = base.for_provider(provider) + if options[:pacticipant_name] + base = base.where(Sequel[:pact_publications][:id] => pact_pacticipant_ids_by_name(options[:pacticipant_name])) 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 +166,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 +185,48 @@ 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) + terms = pacticipant_name.split.map { |v| v.gsub("_", '\\_') } + string_match_query = Sequel.|( + Sequel.|( *terms.map { |term| Sequel.ilike(Sequel[:providers][:name], "%#{term}%") }), + Sequel.|( *terms.map { |term| Sequel.ilike(Sequel[:consumers][:name], "%#{term}%") }) + ) + + PactBroker::Pacts::PactPublication + .join_consumers(:consumers) + .join_providers(:providers) + .where(string_match_query) + .select(Sequel[:pact_publications][:id]) + .map(: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/ui/controllers/index.rb b/lib/pact_broker/ui/controllers/index.rb index 178b38701..b32daa7c4 100644 --- a/lib/pact_broker/ui/controllers/index.rb +++ b/lib/pact_broker/ui/controllers/index.rb @@ -10,35 +10,47 @@ 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] 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 = ViewDomain::IndexItems.new(index_service.find_index_items(options), base_url: base_url) + index_items = index_service.find_index_items(options) + + 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, + 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.haml b/lib/pact_broker/ui/views/index/show.haml index 85aefe339..584853d64 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'} + Pacticipant name + %input{name: 'pacticipant_name', id: 'pacticipant_name', class: 'pacticipant_name'} + %div.top-of-group + %input{type: 'submit', value: 'Search'} + %table.table.table-bordered.table-striped{ id: 'relationships' } %thead %tr diff --git a/public/stylesheets/index.css b/public/stylesheets/index.css index 6adf1dd57..bcc0e670b 100644 --- a/public/stylesheets/index.css +++ b/public/stylesheets/index.css @@ -232,3 +232,17 @@ span.copy-success-icon { width: 16px; height: 16px; } + +.field label { + width: 10em; + margin-top: 15px; +} + +.field .pacticipant_name { + width: 20em; +} + +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/ui/controllers/index_spec.rb b/spec/lib/pact_broker/ui/controllers/index_spec.rb index 37c560175..39e76f8ff 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: "example 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