Skip to content

Commit

Permalink
FIX: Allow dots in repo names (#75)
Browse files Browse the repository at this point in the history
Rails does not allow dots in route params by default.
This can be allowed using a route constraint. I used the
same one we use for usernames in core. Without this,
a 404 error is raised on has_webhooks for any repo with
a . in the name.
  • Loading branch information
martin-brennan committed Jun 22, 2021
1 parent 47f17c3 commit feda2b2
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
5 changes: 4 additions & 1 deletion plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,10 @@ def self.github_organizations

scope format: true, constraints: { format: 'json' } do
resources :organizations, only: [:index] do
resources :repos, only: [:index] do

# need to allow dots in the id, use the same username
# regex from core
resources :repos, only: [:index], id: /[\w.\-]+?/ do
member do
get '/has-configured-webhook' => 'repos#has_configured_webhook'
post '/configure-webhook' => 'repos#configure_webhook'
Expand Down
15 changes: 13 additions & 2 deletions spec/requests/discourse_code_review/repos_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ def set_client(client)
end

context "when a webhook is configured" do
let(:repo_name) { "repo" }
let!(:client) do
client = mock
client.stubs(:hooks).with('org/repo').returns([
client.stubs(:hooks).with("org/#{repo_name}").returns([
{
events: webhook_events,
config: {
Expand All @@ -125,9 +126,19 @@ def set_client(client)
end

it "says yes" do
get '/admin/plugins/code-review/organizations/org/repos/repo/has-configured-webhook.json'
get "/admin/plugins/code-review/organizations/org/repos/#{repo_name}/has-configured-webhook.json"
expect(JSON.parse(response.body)).to eq('has_configured_webhook' => true)
end

context "when repo name has a . in it" do
let(:repo_name) { "Some-coolrepo.org" }

it "returns the right response" do
get "/admin/plugins/code-review/organizations/org/repos/#{repo_name}/has-configured-webhook.json"
expect(response.status).to eq(200)
expect(JSON.parse(response.body)).to eq('has_configured_webhook' => true)
end
end
end

context "when no webhook is configured" do
Expand Down

0 comments on commit feda2b2

Please sign in to comment.