Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove HTTParty and use Faraday throughout. #1499

Merged
merged 6 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ gem "gds-sso"
gem "govuk_app_config"
gem "govuk_publishing_components"
gem "govuk_sidekiq"
gem "httparty"
gem "mysql2"
gem "octicons_helper"
gem "octokit"
Expand Down
6 changes: 0 additions & 6 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ GEM
bigdecimal
rexml
crass (1.0.6)
csv (3.3.0)
dartsass-rails (0.5.0)
railties (>= 6.0.0)
sass-embedded (~> 1.63)
Expand Down Expand Up @@ -216,10 +215,6 @@ GEM
http-accept (1.7.0)
http-cookie (1.0.6)
domain_name (~> 0.5)
httparty (0.22.0)
csv
mini_mime (>= 1.0.0)
multi_xml (>= 0.5.2)
i18n (1.14.5)
concurrent-ruby (~> 1.0)
io-console (0.7.2)
Expand Down Expand Up @@ -715,7 +710,6 @@ DEPENDENCIES
govuk_app_config
govuk_publishing_components
govuk_sidekiq
httparty
minitest
mocha
mysql2
Expand Down
20 changes: 7 additions & 13 deletions app/jobs/slack_poster_job.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
require "httparty"
require "faraday"

class SlackPosterJob < ApplicationJob
queue_as :default
Expand All @@ -9,13 +9,14 @@ class SlackPosterJob < ApplicationJob
def perform(text, channel, options = {})
raise "Invalid options, only #{VALID_OPTIONS.join(', ')} are permitted" unless valid_options?(options)

@webhook_url = ENV["SLACK_WEBHOOK_URL"]

post_to_slack(payload(text, channel, options)) if @webhook_url.present?
@webhook_url = ENV.fetch("SLACK_WEBHOOK_URL", nil)
sengi marked this conversation as resolved.
Show resolved Hide resolved
post_to_slack(payload(text, channel, options)) if @webhook_url
end

private

class SlackMessageError < StandardError; end

def payload(text, channel, options)
{
username: options["username"] || "Release app",
Expand All @@ -27,21 +28,14 @@ def payload(text, channel, options)
end

def post_to_slack(payload)
response = HTTParty.post(@webhook_url, body: payload.to_json)
response = Faraday.new.post(@webhook_url, payload.to_json)

unless successful?(response.code)
unless response.success?
raise SlackMessageError, "Slack error: #{response.body}"
end
end

def successful?(response_code)
success_response_code = 200
response_code == success_response_code
end

def valid_options?(options)
options.keys.all? { |key| VALID_OPTIONS.include?(key) }
end

class SlackMessageError < StandardError; end
end
20 changes: 11 additions & 9 deletions app/models/repo.rb
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
require "faraday"
require "json"

class Repo
include HTTParty
base_uri "docs.publishing.service.gov.uk"
REPO_JSON_URL = "https://docs.publishing.service.gov.uk/apps.json".freeze

def self.all
Rails.cache.fetch("apps_json_data", expires_in: 12.hours) do
response ||= get("/apps.json")
JSON.parse(response.body)
rescue HTTParty::Error, JSON::ParserError => e
Rails.logger.debug "Error fetching govuk repos: #{e.message}"
[]
Rails.cache.fetch("apps_json", expires_in: 12.hours) do
JSON.parse(Faraday.new.get(REPO_JSON_URL).body)
rescue Faraday::Error, JSON::ParserError => e
Rails.logger.warn "Error fetching govuk repos: #{e.message}"
[] # TODO: don't eat the error; raise a wrapped exception and let the caller decide.
end
end

def self.find_by(app_name:)
all.find { |app| app["app_name"] == app_name.parameterize }
app_name = app_name.parameterize
all.find { |app| app["app_name"] == app_name }
end

def self.url(app_name:)
Expand Down
6 changes: 1 addition & 5 deletions config/database.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,11 @@ default: &default
development:
<<: *default
database: release_development
username: release
password: release
url: <%= ENV['DATABASE_URL'] %>

test: &test
test:
<<: *default
database: release_test
username: release
password: release
url: <%= ENV["TEST_DATABASE_URL"].try(:sub, /([-_]development)?$/, '_test') %>

production:
Expand Down
16 changes: 8 additions & 8 deletions test/functional/applications_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class ApplicationsControllerTest < ActionController::TestCase
setup do
response_body = [{ "app_name" => "app1",
"links" => { "repo_url" => "https://github.com/user/app1" } }].to_json
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: response_body, headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: response_body, headers: {})
@app1 = FactoryBot.create(:application, name: "app1", default_branch: "main")
@app2 = FactoryBot.create(:application, name: "app2")
@deploy1 = FactoryBot.create(
Expand Down Expand Up @@ -45,7 +45,7 @@ class ApplicationsControllerTest < ActionController::TestCase

context "POST create" do
setup do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
end

context "valid request" do
Expand Down Expand Up @@ -87,7 +87,7 @@ class ApplicationsControllerTest < ActionController::TestCase

context "GET show" do
setup do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})

@app = FactoryBot.create(:application)
stub_graphql(Github, :application, owner: "alphagov", name: @app.name.parameterize)
Expand Down Expand Up @@ -152,7 +152,7 @@ class ApplicationsControllerTest < ActionController::TestCase
@first_commit = "ee37124a286a0b8501776d9bbe55dcb18ccab645"
@second_commit = "1dac538d10b181e9b7b46766bc3a72d001a1f703"
@base_commit = "974d1aedf82c068b42dace07984025fd70dfb240"
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
FactoryBot.create(:deployment, application: @app, version:, deployed_sha: @first_commit)
end

Expand Down Expand Up @@ -183,7 +183,7 @@ class ApplicationsControllerTest < ActionController::TestCase
context "when format is json" do
setup do
response_body = [{ "app_name" => "application-2", "links" => { "repo_url" => "https://github.com/alphagov/application-2" } }].to_json
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: response_body, headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: response_body, headers: {})
@app = FactoryBot.create(:application, name: "Application 2")
end

Expand Down Expand Up @@ -224,7 +224,7 @@ class ApplicationsControllerTest < ActionController::TestCase

context "GET edit" do
setup do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
@app = FactoryBot.create(:application, name: "monkeys")
end

Expand All @@ -241,7 +241,7 @@ class ApplicationsControllerTest < ActionController::TestCase

context "PUT update" do
setup do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
@app = FactoryBot.create(:application)
end

Expand Down Expand Up @@ -275,7 +275,7 @@ class ApplicationsControllerTest < ActionController::TestCase

context "GET deploy" do
setup do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
@app = FactoryBot.create(:application, name: "app1", status_notes: "Do not deploy this without talking to core team first!")
@deployment = FactoryBot.create(:deployment, application_id: @app.id, created_at: "18/01/2013 11:57")
@release_tag = "hot_fix_1"
Expand Down
4 changes: 2 additions & 2 deletions test/functional/deployments_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class DeploymentsControllerTest < ActionController::TestCase

context "GET recent" do
setup do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
Deployment.delete_all
@application = FactoryBot.create(:application, name: "Foo")
@deployments = FactoryBot.create_list(:deployment, 10, application_id: @application.id)
Expand Down Expand Up @@ -57,7 +57,7 @@ class DeploymentsControllerTest < ActionController::TestCase
end

setup do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
end

should "create a deployment record" do
Expand Down
2 changes: 1 addition & 1 deletion test/helpers/breadcrumb_helper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

class BreadcrumbHelperTest < ActionView::TestCase
should "return a hash of title and url" do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
app_name = SecureRandom.hex
app = FactoryBot.create(:application, name: app_name)

Expand Down
4 changes: 2 additions & 2 deletions test/integration/deploy_page_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
class DeployPageTest < ActionDispatch::IntegrationTest
setup do
login_as_stub_user
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
end

test "page handles a single deployment without a previous deployment" do
Expand All @@ -16,7 +16,7 @@ class DeployPageTest < ActionDispatch::IntegrationTest
end

test "page handles a deployment with a previous deployment" do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
application = FactoryBot.create(:application)

commits = [
Expand Down
2 changes: 1 addition & 1 deletion test/integration/global_status_notes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

class GlobalStatusNotesTest < ActionDispatch::IntegrationTest
setup do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
@app1 = FactoryBot.create(:application)
@app2 = FactoryBot.create(:application)
login_as_stub_user
Expand Down
2 changes: 1 addition & 1 deletion test/integration/stats_page_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class StatsPageTest < ActionDispatch::IntegrationTest
end

test "page with stats for an application" do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})

application = FactoryBot.create(:application)

Expand Down
24 changes: 12 additions & 12 deletions test/unit/application_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class ApplicationTest < ActiveSupport::TestCase
@atts = {
name: "Tron-o-matic",
}
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
end

context "given valid attributes" do
Expand Down Expand Up @@ -94,7 +94,7 @@ class ApplicationTest < ActiveSupport::TestCase
@atts = {
name: "Tron-o-matic",
}
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
end

context "when the application is not continuously deployed" do
Expand Down Expand Up @@ -122,7 +122,7 @@ class ApplicationTest < ActiveSupport::TestCase
context "live environment" do
setup do
@atts = { name: "Tron-o-matic" }
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
end

should "return production" do
Expand All @@ -134,7 +134,7 @@ class ApplicationTest < ActiveSupport::TestCase

describe "#status" do
before do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
@app = FactoryBot.create(:application, name: SecureRandom.hex)
Deployment.delete_all
end
Expand Down Expand Up @@ -166,7 +166,7 @@ class ApplicationTest < ActiveSupport::TestCase

describe "#latest_deploys_by_environment" do
before do
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
end

should "orders main environments" do
Expand Down Expand Up @@ -232,7 +232,7 @@ class ApplicationTest < ActiveSupport::TestCase
describe "#repo_url" do
should "return the repository url for the apps" do
response_body = [{ "app_name" => "account-api", "links" => { "repo_url" => "https://github.com/alphagov/account-api" } }].to_json
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: response_body)
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: response_body)

app = FactoryBot.create(:application, name: "Account API")

Expand All @@ -241,7 +241,7 @@ class ApplicationTest < ActiveSupport::TestCase

should "create the repository url using the app name of the url is not provided or empty" do
response_body = [{ "app_name" => "account-api" }].to_json
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: response_body)
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: response_body)

app = FactoryBot.create(:application, name: "Account API")

Expand All @@ -257,7 +257,7 @@ class ApplicationTest < ActiveSupport::TestCase

should "return the shortname for the app" do
response_body = [{ "app_name" => "account-api", "shortname" => "account_api" }].to_json
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: response_body)
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: response_body)

app = FactoryBot.create(:application, name: "Account API")

Expand All @@ -266,7 +266,7 @@ class ApplicationTest < ActiveSupport::TestCase

should "create the shortname using the app name if the shortname is not provided or empty" do
response_body = [{ "app_name" => "account-api" }].to_json
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: response_body)
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: response_body)

app = FactoryBot.create(:application, name: "Account API")

Expand All @@ -283,7 +283,7 @@ class ApplicationTest < ActiveSupport::TestCase

should "return the name of the team that owns the app" do
response_body = [{ "app_name" => "account-api", "team" => "#tech-content-interactions-on-platform-govuk" }].to_json
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: response_body)
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: response_body)

app = FactoryBot.create(:application, name: "Account API", shortname: "account-api")

Expand All @@ -292,7 +292,7 @@ class ApplicationTest < ActiveSupport::TestCase

should "return general dev slack channel when it can't find team (because app names don't match)" do
response_body = [{ "app_name" => "content-data-admin", "team" => "#govuk-platform-security-reliability-team" }].to_json
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: response_body)
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: response_body)

app = FactoryBot.create(:application, name: "Content Data", shortname: "content-data")

Expand All @@ -304,7 +304,7 @@ class ApplicationTest < ActiveSupport::TestCase
before do
Application.delete_all
Deployment.delete_all
stub_request(:get, "http://docs.publishing.service.gov.uk/apps.json").to_return(status: 200, body: "", headers: {})
stub_request(:get, Repo::REPO_JSON_URL).to_return(status: 200, body: "", headers: {})
end

should "return the apps that are out of sync" do
Expand Down
Loading