From ef955a4c086f4d0602f0f7e6d6d1173a2b827617 Mon Sep 17 00:00:00 2001 From: Juan David Martinez Date: Fri, 4 Oct 2024 23:40:13 -0500 Subject: [PATCH 1/5] DEV: Implement ProblemCheck for plugin configuration. Improved error logging and updated Webinar composer UI --- .../problem_check/s2s_webinar_subscription.rb | 5 + .../components/modal/webinar-picker.gjs | 31 ++-- assets/stylesheets/common/webinar-picker.scss | 10 ++ config/locales/server.en.yml | 7 + lib/oauth_client.rb | 35 ++-- plugin.rb | 2 + public/javascripts/webinar-join.js | 2 +- spec/requests/webinars_controller_spec.rb | 149 ++++++++++++++++++ .../services/s2s_webinar_subscription_spec.rb | 20 +++ 9 files changed, 235 insertions(+), 26 deletions(-) create mode 100644 app/services/problem_check/s2s_webinar_subscription.rb create mode 100644 spec/services/s2s_webinar_subscription_spec.rb diff --git a/app/services/problem_check/s2s_webinar_subscription.rb b/app/services/problem_check/s2s_webinar_subscription.rb new file mode 100644 index 0000000..b650f12 --- /dev/null +++ b/app/services/problem_check/s2s_webinar_subscription.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class ProblemCheck::S2sWebinarSubscription < ProblemCheck::InlineProblemCheck + self.priority = "high" +end diff --git a/assets/javascripts/discourse/components/modal/webinar-picker.gjs b/assets/javascripts/discourse/components/modal/webinar-picker.gjs index 1801f64..433a0ee 100644 --- a/assets/javascripts/discourse/components/modal/webinar-picker.gjs +++ b/assets/javascripts/discourse/components/modal/webinar-picker.gjs @@ -57,7 +57,7 @@ export default class WebinarPicker extends Component { id = this.scrubWebinarId(id.toString()); this.loading = true; this.error = false; - + try { const json = await ajax(`/zoom/webinars/${id}/preview`); this.webinar = json; @@ -65,7 +65,7 @@ export default class WebinarPicker extends Component { } catch (error) { this.webinar = null; this.selected = false; - this.error = true; + this.error = error.jqXHR.responseJSON.errors[0]; } finally { this.loading = false; } @@ -250,7 +250,7 @@ export default class WebinarPicker extends Component { {{else}} {{#if this.error}}
- {{i18n "zoom.error"}} + {{this.error}}
{{/if}} @@ -285,15 +285,22 @@ export default class WebinarPicker extends Component { {{else}}
- - + +
{ userEmail: res.email, success: function (res) {}, error: function (join_result) { - console.log(join_result); + // console.log(join_result); if (join_result.errorCode === 1) { const params = getParams(window.location.href); if (params.fallback) { diff --git a/spec/requests/webinars_controller_spec.rb b/spec/requests/webinars_controller_spec.rb index 3a50d0d..d97625b 100644 --- a/spec/requests/webinars_controller_spec.rb +++ b/spec/requests/webinars_controller_spec.rb @@ -95,6 +95,155 @@ end end + describe "#preview" do + context "Webinar plan missing" do + before do + stub_request(:get, "https://api.zoom.us/v2/webinars/#{webinar.id}").with( + headers: { + "Authorization" => "Bearer Test_Token", + "Content-Type" => "application/json", + "Host" => "api.zoom.us", + }, + ).to_return( + { status: 401, body: { "code" => 200, "message" => "Webinar plan is missing." }.to_json }, + { status: 401, body: { "code" => 200, "message" => "Webinar plan is missing." }.to_json }, + ) + + stub_request(:get, "https://api.zoom.us/v2/webinars/#{webinar.id}/panelists").to_return( + status: 201, + body: { panelists: [{ id: "123", email: user.email }] }.to_json, + ) + + stub_request( + :post, + "https://zoom.us/oauth/token?account_id=&grant_type=account_credentials", + ).with( + headers: { + "Authorization" => "Basic Og==", + "Content-Type" => "application/json", + "Host" => "zoom.us", + }, + ).to_return( + { + body: { access_token: "token" }.to_json, + headers: { + content_type: "application/json", + }, + status: 200, + }, + { + body: { access_token: "token" }.to_json, + headers: { + content_type: "application/json", + }, + status: 200, + }, + ) + + stub_request(:get, "https://api.zoom.us/v2/webinars/#{webinar.id}").with( + headers: { + "Authorization" => "Bearer token", + "Content-Type" => "application/json", + "Host" => "api.zoom.us", + }, + ).to_return( + { status: 401, body: { "code" => 200, "message" => "Webinar plan is missing." }.to_json }, + ) + + sign_in(user) + end + it "creates problem check error" do + get "/zoom/webinars/#{webinar.id}/preview.json" + json = JSON.parse(response.body) + + expect(response.status).to eq(403) + expect(AdminNotice.problem.last.message).to eq( + I18n.t("dashboard.problem.s2s_webinar_subscription", message: "Webinar plan is missing."), + ) + end + end + + context "Webinar missing" do + let(:fake_logger) { FakeLogger.new } + + before do + stub_request(:get, "https://api.zoom.us/v2/webinars/#{webinar.id}").with( + headers: { + "Authorization" => "Bearer Test_Token", + "Content-Type" => "application/json", + "Host" => "api.zoom.us", + }, + ).to_return( + { + status: 404, + body: { code: 3001, message: "Meeting is not found or has expired." }.to_json, + }, + { + status: 404, + body: { code: 3001, message: "Meeting is not found or has expired." }.to_json, + }, + ) + + stub_request(:get, "https://api.zoom.us/v2/webinars/#{webinar.id}/panelists").to_return( + status: 201, + body: { panelists: [{ id: "123", email: user.email }] }.to_json, + ) + + stub_request( + :post, + "https://zoom.us/oauth/token?account_id=&grant_type=account_credentials", + ).with( + headers: { + "Authorization" => "Basic Og==", + "Content-Type" => "application/json", + "Host" => "zoom.us", + }, + ).to_return( + { + body: { access_token: "token" }.to_json, + headers: { + content_type: "application/json", + }, + status: 200, + }, + { + body: { access_token: "token" }.to_json, + headers: { + content_type: "application/json", + }, + status: 200, + }, + ) + + stub_request(:get, "https://api.zoom.us/v2/webinars/#{webinar.id}").with( + headers: { + "Authorization" => "Bearer token", + "Content-Type" => "application/json", + "Host" => "api.zoom.us", + }, + ).to_return( + { + status: 404, + body: { code: 3001, message: "Meeting is not found or has expired." }.to_json, + }, + ) + + Rails.logger.broadcast_to(fake_logger) + + sign_in(user) + end + + after { Rails.logger.stop_broadcasting_to(fake_logger) } + + it "creates problem check error" do + get "/zoom/webinars/#{webinar.id}/preview.json" + json = JSON.parse(response.body) + + expect(response.status).to eq(404) + end + end + end + describe "#remove_panelist" do before do stub_request(:get, "https://api.zoom.us/v2/webinars/#{webinar.zoom_id}/panelists").to_return( diff --git a/spec/services/s2s_webinar_subscription_spec.rb b/spec/services/s2s_webinar_subscription_spec.rb new file mode 100644 index 0000000..6f9614c --- /dev/null +++ b/spec/services/s2s_webinar_subscription_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require "rails_helper" +RSpec.describe ProblemCheck::S2sWebinarSubscription do + before do + stub_request(:get, "https://api.zoom.us/v2/webinars/123456").to_return( + status: 401, + body: { "error" => { "code" => 200, "message" => "Webinar plan is missing." } }.to_json, + ) + end + it "raise a error and trigger a problem check when the server returns a code 200" do + ProblemCheckTracker[:s2s_webinar_subscription].no_problem! + + get "/zoom/webinars/123456/preview.json" + + expect(AdminNotice.problem.last.message).to eq( + I18n.t("dashboard.problem.s2s_webinar_subscription", message: "Webinar plan is missing."), + ) + end +end From 18900aaf9c96e96c858118f7a9fdff42224be1e0 Mon Sep 17 00:00:00 2001 From: Juan David Martinez Date: Sun, 6 Oct 2024 20:17:05 -0500 Subject: [PATCH 2/5] fixed rubocop issues --- lib/oauth_client.rb | 3 ++- spec/requests/webinars_controller_spec.rb | 6 +++--- .../services/s2s_webinar_subscription_spec.rb | 20 ------------------- 3 files changed, 5 insertions(+), 24 deletions(-) delete mode 100644 spec/services/s2s_webinar_subscription_spec.rb diff --git a/lib/oauth_client.rb b/lib/oauth_client.rb index 961e41c..ac378ac 100644 --- a/lib/oauth_client.rb +++ b/lib/oauth_client.rb @@ -56,7 +56,8 @@ def send_request(method, body = nil) elsif [400, 401].include?(response.status) && @tries == @max_tries self.parse_response_body(response) # This code 200 is a code sent by Zoom not the request status - if response&.body[:code] == 200 + + if response&.body&.dig(:code) == 200 ProblemCheckTracker["s2s_webinar_subscription"].problem!( details: { message: response.body[:message], diff --git a/spec/requests/webinars_controller_spec.rb b/spec/requests/webinars_controller_spec.rb index d97625b..893cec8 100644 --- a/spec/requests/webinars_controller_spec.rb +++ b/spec/requests/webinars_controller_spec.rb @@ -96,7 +96,7 @@ end describe "#preview" do - context "Webinar plan missing" do + context "when Webinar plan missing" do before do stub_request(:get, "https://api.zoom.us/v2/webinars/#{webinar.id}").with( headers: { @@ -163,7 +163,7 @@ end end - context "Webinar missing" do + context "when Webinar missing" do let(:fake_logger) { FakeLogger.new } before do @@ -235,7 +235,7 @@ after { Rails.logger.stop_broadcasting_to(fake_logger) } - it "creates problem check error" do + it "shows the correct error message for code 3001" do get "/zoom/webinars/#{webinar.id}/preview.json" json = JSON.parse(response.body) diff --git a/spec/services/s2s_webinar_subscription_spec.rb b/spec/services/s2s_webinar_subscription_spec.rb deleted file mode 100644 index 6f9614c..0000000 --- a/spec/services/s2s_webinar_subscription_spec.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" -RSpec.describe ProblemCheck::S2sWebinarSubscription do - before do - stub_request(:get, "https://api.zoom.us/v2/webinars/123456").to_return( - status: 401, - body: { "error" => { "code" => 200, "message" => "Webinar plan is missing." } }.to_json, - ) - end - it "raise a error and trigger a problem check when the server returns a code 200" do - ProblemCheckTracker[:s2s_webinar_subscription].no_problem! - - get "/zoom/webinars/123456/preview.json" - - expect(AdminNotice.problem.last.message).to eq( - I18n.t("dashboard.problem.s2s_webinar_subscription", message: "Webinar plan is missing."), - ) - end -end From 03d3ae390a369ed872cc370d8aeb0d619d71fab8 Mon Sep 17 00:00:00 2001 From: Juan David Martinez Date: Sun, 6 Oct 2024 21:30:18 -0500 Subject: [PATCH 3/5] linter issues --- .../discourse/components/modal/webinar-picker.gjs | 8 ++++---- assets/stylesheets/common/webinar-picker.scss | 2 +- public/javascripts/webinar-join.js | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/assets/javascripts/discourse/components/modal/webinar-picker.gjs b/assets/javascripts/discourse/components/modal/webinar-picker.gjs index 433a0ee..bd170d7 100644 --- a/assets/javascripts/discourse/components/modal/webinar-picker.gjs +++ b/assets/javascripts/discourse/components/modal/webinar-picker.gjs @@ -57,7 +57,7 @@ export default class WebinarPicker extends Component { id = this.scrubWebinarId(id.toString()); this.loading = true; this.error = false; - + try { const json = await ajax(`/zoom/webinars/${id}/preview`); this.webinar = json; @@ -295,12 +295,12 @@ export default class WebinarPicker extends Component { />
- +
{ let request = new XMLHttpRequest(); request.open("GET", `/zoom/webinars/${meetingId}/signature.json`, true); - + request.onload = function () { if (this.status >= 200 && this.status < 400) { let res = JSON.parse(this.response); From 684a4b4f5b77e3b6627af4d9798d08c09aa33be7 Mon Sep 17 00:00:00 2001 From: Juan David Martinez Date: Sun, 6 Oct 2024 21:44:58 -0500 Subject: [PATCH 4/5] fixed test --- spec/requests/oauth_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/oauth_spec.rb b/spec/requests/oauth_spec.rb index 4f851a9..b1d9d79 100644 --- a/spec/requests/oauth_spec.rb +++ b/spec/requests/oauth_spec.rb @@ -91,7 +91,7 @@ headers: { content_type: "application/json", }, - status: 200, + status: 400, ) end it "can't request a new oauth_token" do From fec15e9183b091a5c4257db31c808ede9be780ad Mon Sep 17 00:00:00 2001 From: Juan David Martinez Date: Thu, 10 Oct 2024 20:37:38 -0500 Subject: [PATCH 5/5] fixed tests issues --- Gemfile.lock | 67 ++++++++++++++++----------------- app/views/zoom/webinars/sdk.erb | 16 ++++---- lib/oauth_client.rb | 3 +- spec/requests/oauth_spec.rb | 2 +- 4 files changed, 43 insertions(+), 45 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 4a518e2..2a2bf1f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,82 +1,79 @@ GEM remote: https://rubygems.org/ specs: - activesupport (7.1.3.3) + activesupport (7.2.1) base64 bigdecimal - concurrent-ruby (~> 1.0, >= 1.0.2) + concurrent-ruby (~> 1.0, >= 1.3.1) connection_pool (>= 2.2.5) drb i18n (>= 1.6, < 2) + logger (>= 1.4.2) minitest (>= 5.1) - mutex_m - tzinfo (~> 2.0) + securerandom (>= 0.3) + tzinfo (~> 2.0, >= 2.0.5) ast (2.4.2) base64 (0.2.0) bigdecimal (3.1.8) - concurrent-ruby (1.2.3) + concurrent-ruby (1.3.4) connection_pool (2.4.1) drb (2.2.1) - i18n (1.14.5) + i18n (1.14.6) concurrent-ruby (~> 1.0) json (2.7.2) language_server-protocol (3.17.0.3) - minitest (5.23.1) - mutex_m (0.2.0) - parallel (1.24.0) - parser (3.3.1.0) + logger (1.6.1) + minitest (5.25.1) + parallel (1.26.3) + parser (3.3.5.0) ast (~> 2.4.1) racc prettier_print (1.2.1) - racc (1.8.0) - rack (3.0.11) + racc (1.8.1) + rack (3.1.7) rainbow (3.1.1) regexp_parser (2.9.2) - rexml (3.3.6) - strscan - rubocop (1.64.0) + rubocop (1.66.1) json (~> 2.3) language_server-protocol (>= 3.17.0) parallel (~> 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 1.8, < 3.0) - rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.31.1, < 2.0) + regexp_parser (>= 2.4, < 3.0) + rubocop-ast (>= 1.32.2, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.31.3) + rubocop-ast (1.32.3) parser (>= 3.3.1.0) - rubocop-capybara (2.20.0) + rubocop-capybara (2.21.0) rubocop (~> 1.41) - rubocop-discourse (3.8.0) + rubocop-discourse (3.8.1) activesupport (>= 6.1) rubocop (>= 1.59.0) rubocop-capybara (>= 2.0.0) rubocop-factory_bot (>= 2.0.0) rubocop-rails (>= 2.25.0) - rubocop-rspec (>= 2.25.0) - rubocop-factory_bot (2.25.1) - rubocop (~> 1.41) - rubocop-rails (2.25.0) + rubocop-rspec (>= 3.0.1) + rubocop-rspec_rails (>= 2.30.0) + rubocop-factory_bot (2.26.1) + rubocop (~> 1.61) + rubocop-rails (2.26.2) activesupport (>= 4.2.0) rack (>= 1.1) - rubocop (>= 1.33.0, < 2.0) + rubocop (>= 1.52.0, < 2.0) rubocop-ast (>= 1.31.1, < 2.0) - rubocop-rspec (2.29.2) - rubocop (~> 1.40) - rubocop-capybara (~> 2.17) - rubocop-factory_bot (~> 2.22) - rubocop-rspec_rails (~> 2.28) - rubocop-rspec_rails (2.28.3) - rubocop (~> 1.40) + rubocop-rspec (3.1.0) + rubocop (~> 1.61) + rubocop-rspec_rails (2.30.0) + rubocop (~> 1.61) + rubocop-rspec (~> 3, >= 3.0.1) ruby-progressbar (1.13.0) - strscan (3.1.0) + securerandom (0.3.1) syntax_tree (6.2.0) prettier_print (>= 1.2.0) tzinfo (2.0.6) concurrent-ruby (~> 1.0) - unicode-display_width (2.5.0) + unicode-display_width (2.6.0) PLATFORMS ruby diff --git a/app/views/zoom/webinars/sdk.erb b/app/views/zoom/webinars/sdk.erb index e7cbe44..b14763d 100644 --- a/app/views/zoom/webinars/sdk.erb +++ b/app/views/zoom/webinars/sdk.erb @@ -1,11 +1,11 @@ - - - - - - - - + + + + + + + + diff --git a/lib/oauth_client.rb b/lib/oauth_client.rb index ac378ac..00daefc 100644 --- a/lib/oauth_client.rb +++ b/lib/oauth_client.rb @@ -56,7 +56,6 @@ def send_request(method, body = nil) elsif [400, 401].include?(response.status) && @tries == @max_tries self.parse_response_body(response) # This code 200 is a code sent by Zoom not the request status - if response&.body&.dig(:code) == 200 ProblemCheckTracker["s2s_webinar_subscription"].problem!( details: { @@ -69,6 +68,7 @@ def send_request(method, body = nil) end log("Zoom verbose log:\n API error = #{response.inspect}") if response.status != 200 + if response&.body.present? result = JSON.parse(response.body) meeting_not_found if (response.status) == 404 && result["code"] == 3001 @@ -106,6 +106,7 @@ def get_oauth ) response.body = JSON.parse(response.body, symbolize_names: true) if response.body.present? + if response.status == 200 SiteSetting.s2s_oauth_token = response.body[:access_token] @authorization = response.body[:access_token] diff --git a/spec/requests/oauth_spec.rb b/spec/requests/oauth_spec.rb index b1d9d79..27ea886 100644 --- a/spec/requests/oauth_spec.rb +++ b/spec/requests/oauth_spec.rb @@ -39,7 +39,7 @@ Authorization: "Bearer not_a_valid_token", Host: "api.zoom.us", }, - ).to_return(status: 400) + ).to_return(body: ZoomApiStubs.get_webinar(user.id), status: 400) end describe "valid/present" do before { SiteSetting.s2s_oauth_token = valid_token }