From c13f64d35b6d7eae6343d02f1aa80ba781c581ff Mon Sep 17 00:00:00 2001 From: Guhyoun Nam <70915823+rngus2344@users.noreply.github.com> Date: Fri, 7 Jun 2024 10:26:00 -0500 Subject: [PATCH] FEATURE: Add Filter for Webhook Events by Status (#27332) * FEATURE: Add Filter for Webhook Events by Status * Fixing multiple issues * Lint * Fixing multiple issues * Change the range of the status for webhook events --- .../admin/addon/adapters/web-hook-event.js | 18 +++++++++ .../admin/addon/components/webhook-events.hbs | 25 ++++++++---- .../admin/addon/components/webhook-events.js | 39 +++++++++++++++++-- .../addon/controllers/admin-web-hooks-show.js | 4 ++ .../admin/addon/templates/web-hooks-show.hbs | 2 +- app/assets/stylesheets/common/admin/api.scss | 5 ++- app/controllers/admin/web_hooks_controller.rb | 17 +++++--- app/models/web_hook_event.rb | 2 + config/locales/client.en.yml | 4 ++ spec/fabricators/web_hook_event_fabricator.rb | 7 ++++ .../admin/web_hooks_controller_spec.rb | 34 ++++++++++++++++ spec/system/admin_web_hook_events_spec.rb | 37 ++++++++++++++++++ .../pages/admin_web_hook_events.rb | 32 +++++++++++++++ 13 files changed, 207 insertions(+), 19 deletions(-) create mode 100644 spec/fabricators/web_hook_event_fabricator.rb create mode 100644 spec/system/admin_web_hook_events_spec.rb create mode 100644 spec/system/page_objects/pages/admin_web_hook_events.rb diff --git a/app/assets/javascripts/admin/addon/adapters/web-hook-event.js b/app/assets/javascripts/admin/addon/adapters/web-hook-event.js index 8a6fa2b082d..a9b8c5093ef 100644 --- a/app/assets/javascripts/admin/addon/adapters/web-hook-event.js +++ b/app/assets/javascripts/admin/addon/adapters/web-hook-event.js @@ -4,4 +4,22 @@ export default class WebHookEvent extends RestAdapter { basePath() { return "/admin/api/"; } + + appendQueryParams(path, findArgs, extension) { + const urlSearchParams = new URLSearchParams(); + + for (const [key, value] of Object.entries(findArgs)) { + if (value && key !== "webhookId") { + urlSearchParams.set(key, value); + } + } + + const queryString = urlSearchParams.toString(); + let url = `${path}/${findArgs.webhookId}${extension || ""}`; + + if (queryString) { + url = `${url}?${queryString}`; + } + return url; + } } diff --git a/app/assets/javascripts/admin/addon/components/webhook-events.hbs b/app/assets/javascripts/admin/addon/components/webhook-events.hbs index c163126cd8a..7bfdeb1111d 100644 --- a/app/assets/javascripts/admin/addon/components/webhook-events.hbs +++ b/app/assets/javascripts/admin/addon/components/webhook-events.hbs @@ -1,15 +1,26 @@
- +
+ + + +
{{#if this.events}} diff --git a/app/assets/javascripts/admin/addon/components/webhook-events.js b/app/assets/javascripts/admin/addon/components/webhook-events.js index 8d9e1f4d45f..fede8d838a7 100644 --- a/app/assets/javascripts/admin/addon/components/webhook-events.js +++ b/app/assets/javascripts/admin/addon/components/webhook-events.js @@ -6,6 +6,7 @@ import { service } from "@ember/service"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; import { bind } from "discourse-common/utils/decorators"; +import I18n from "discourse-i18n"; export default class WebhookEvents extends Component { @service messageBus; @@ -24,10 +25,40 @@ export default class WebhookEvents extends Component { } async loadEvents() { - this.events = await this.store.findAll( - "web-hook-event", - this.args.webhookId - ); + this.loading = true; + + try { + this.events = await this.store.findAll("web-hook-event", { + webhookId: this.args.webhookId, + status: this.args.status, + }); + } catch (error) { + popupAjaxError(error); + } finally { + this.loading = false; + } + } + + get statuses() { + return [ + { + id: "successful", + name: I18n.t("admin.web_hooks.events.filter_status.successful"), + }, + { + id: "failed", + name: I18n.t("admin.web_hooks.events.filter_status.failed"), + }, + ]; + } + + @bind + reloadEvents() { + if (this.loading) { + return; + } + + this.loadEvents(); } @bind diff --git a/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-show.js b/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-show.js index 6df416ab127..41fc6d26add 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-show.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-web-hooks-show.js @@ -1,3 +1,4 @@ +import { tracked } from "@glimmer/tracking"; import Controller, { inject as controller } from "@ember/controller"; import { action } from "@ember/object"; import { service } from "@ember/service"; @@ -8,6 +9,9 @@ export default class AdminWebHooksShowController extends Controller { @service dialog; @service router; @controller adminWebHooks; + @tracked status; + + queryParams = ["status"]; @action edit() { diff --git a/app/assets/javascripts/admin/addon/templates/web-hooks-show.hbs b/app/assets/javascripts/admin/addon/templates/web-hooks-show.hbs index a4f35ad4521..80e5461e7c3 100644 --- a/app/assets/javascripts/admin/addon/templates/web-hooks-show.hbs +++ b/app/assets/javascripts/admin/addon/templates/web-hooks-show.hbs @@ -31,4 +31,4 @@
- \ No newline at end of file + \ No newline at end of file diff --git a/app/assets/stylesheets/common/admin/api.scss b/app/assets/stylesheets/common/admin/api.scss index 29c3ded7413..607daf52c35 100644 --- a/app/assets/stylesheets/common/admin/api.scss +++ b/app/assets/stylesheets/common/admin/api.scss @@ -338,7 +338,10 @@ table.api-keys { } } -.webhook-events__ping-button { +.web-hook-events-actions { + display: flex; + align-items: center; + gap: 0.5rem; margin-bottom: 1rem; } diff --git a/app/controllers/admin/web_hooks_controller.rb b/app/controllers/admin/web_hooks_controller.rb index eeb489d663f..d471132363f 100644 --- a/app/controllers/admin/web_hooks_controller.rb +++ b/app/controllers/admin/web_hooks_controller.rb @@ -87,14 +87,19 @@ class Admin::WebHooksController < Admin::AdminController def list_events limit = 50 offset = params[:offset].to_i + events = @web_hook.web_hook_events + if params[:status] == "successful" + events = events.successful + elsif params[:status] == "failed" + events = events.failed + end + + total = events.count + events = events.limit(limit).offset(offset) json = { - web_hook_events: - serialize_data( - @web_hook.web_hook_events.limit(limit).offset(offset), - AdminWebHookEventSerializer, - ), - total_rows_web_hook_events: @web_hook.web_hook_events.count, + web_hook_events: serialize_data(events, AdminWebHookEventSerializer), + total_rows_web_hook_events: total, load_more_web_hook_events: web_hook_events_admin_api_index_path(limit: limit, offset: offset + limit, format: :json), extras: { diff --git a/app/models/web_hook_event.rb b/app/models/web_hook_event.rb index d9a53041f29..ca048bece45 100644 --- a/app/models/web_hook_event.rb +++ b/app/models/web_hook_event.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class WebHookEvent < ActiveRecord::Base + scope :successful, -> { where("status >= 200 AND status <= 299") } + scope :failed, -> { where("status < 200 OR status > 299") } belongs_to :web_hook after_save :update_web_hook_delivery_status diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index fbb6036110d..84841597601 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5357,6 +5357,10 @@ en: timestamp: "Created" completion: "Completion Time" actions: "Actions" + filter_status: + all: "All Events" + successful: "Delivered" + failed: "Failed" home: title: "Home" account: diff --git a/spec/fabricators/web_hook_event_fabricator.rb b/spec/fabricators/web_hook_event_fabricator.rb new file mode 100644 index 00000000000..93b451f6627 --- /dev/null +++ b/spec/fabricators/web_hook_event_fabricator.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +Fabricator(:web_hook_event) do + web_hook { Fabricate(:web_hook) } + payload { { some_key: "some_value" }.to_json } + status 200 +end diff --git a/spec/requests/admin/web_hooks_controller_spec.rb b/spec/requests/admin/web_hooks_controller_spec.rb index b5e3de66975..1f0d7d978db 100644 --- a/spec/requests/admin/web_hooks_controller_spec.rb +++ b/spec/requests/admin/web_hooks_controller_spec.rb @@ -197,6 +197,40 @@ RSpec.describe Admin::WebHooksController do end end + describe "#list_events" do + fab!(:web_hook_event1) { Fabricate(:web_hook_event, web_hook: web_hook, id: 1, status: 200) } + fab!(:web_hook_event2) { Fabricate(:web_hook_event, web_hook: web_hook, id: 2, status: 404) } + + before { sign_in(admin) } + + context "when status is 'successful'" do + it "lists the successfully delivered webhook events" do + get "/admin/api/web_hook_events/#{web_hook.id}.json", params: { status: "successful" } + expect(response.parsed_body["web_hook_events"].map { |c| c["id"] }).to eq( + [web_hook_event1.id], + ) + end + end + + context "when status is 'failed'" do + it "lists the failed webhook events" do + get "/admin/api/web_hook_events/#{web_hook.id}.json", params: { status: "failed" } + expect(response.parsed_body["web_hook_events"].map { |c| c["id"] }).to eq( + [web_hook_event2.id], + ) + end + end + + context "when there is no status param" do + it "lists all webhook events" do + get "/admin/api/web_hook_events/#{web_hook.id}.json" + expect(response.parsed_body["web_hook_events"].map { |c| c["id"] }).to match_array( + [web_hook_event1.id, web_hook_event2.id], + ) + end + end + end + describe "#ping" do context "when logged in as admin" do before { sign_in(admin) } diff --git a/spec/system/admin_web_hook_events_spec.rb b/spec/system/admin_web_hook_events_spec.rb new file mode 100644 index 00000000000..3c6c2897f14 --- /dev/null +++ b/spec/system/admin_web_hook_events_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +describe "Admin WebHook Events", type: :system do + fab!(:web_hook) + fab!(:admin) + fab!(:web_hook_event1) { Fabricate(:web_hook_event, web_hook: web_hook, status: 200) } + fab!(:web_hook_event2) { Fabricate(:web_hook_event, web_hook: web_hook, status: 404) } + + let(:admin_web_hooks_page) { PageObjects::Pages::AdminWebHookEvents.new } + + before { sign_in(admin) } + + it "shows all webhook events when filter is on 'All Events'" do + admin_web_hooks_page.visit(web_hook.id) + + expect(admin_web_hooks_page).to have_web_hook_event(web_hook_event1.id) + expect(admin_web_hooks_page).to have_web_hook_event(web_hook_event2.id) + end + + it "shows only successfully delivered webhook events when filter is on 'Delivered'" do + admin_web_hooks_page.visit(web_hook.id) + admin_web_hooks_page.click_filter_all + admin_web_hooks_page.click_filter_delivered + + expect(admin_web_hooks_page).to have_web_hook_event(web_hook_event1.id) + expect(admin_web_hooks_page).to have_no_web_hook_event(web_hook_event2.id) + end + + it "shows only webhook events that are failed to deliver when filter is on 'Failed'" do + admin_web_hooks_page.visit(web_hook.id) + admin_web_hooks_page.click_filter_all + admin_web_hooks_page.click_filter_failed + + expect(admin_web_hooks_page).to have_no_web_hook_event(web_hook_event1.id) + expect(admin_web_hooks_page).to have_web_hook_event(web_hook_event2.id) + end +end diff --git a/spec/system/page_objects/pages/admin_web_hook_events.rb b/spec/system/page_objects/pages/admin_web_hook_events.rb new file mode 100644 index 00000000000..8ccd6ba6f78 --- /dev/null +++ b/spec/system/page_objects/pages/admin_web_hook_events.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class AdminWebHookEvents < PageObjects::Pages::Base + def visit(id) + page.visit("/admin/api/web_hooks/#{id}") + self + end + + def click_filter_all + find(".select-kit-header", text: "All Events").click + end + + def click_filter_delivered + find(".select-kit-row", text: "Delivered").click + end + + def click_filter_failed + find(".select-kit-row", text: "Failed").click + end + + def has_web_hook_event?(id) + page.has_css?("li .event-id", text: id) + end + + def has_no_web_hook_event?(id) + page.has_no_css?("li .event-id", text: id) + end + end + end +end