From 02fa04e333eda109de7c01788e2163b25df48a81 Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Fri, 11 Mar 2022 11:01:08 -0700 Subject: [PATCH] FIX: Update topic route id param (#16166) This update topic route has never worked. Better late than never. I am in favor of using non-slug urls when using the api so I do think we should fix this route. Just thought I would update the `:id` param to `:topic_id` here in the routes file instead of updating the controller to handle both params. Added a spec to test this route. Also added the same constraint we have on other topic routes to ensure we only pass in an ID that is a digit. --- config/routes.rb | 2 +- spec/requests/topics_controller_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index d11419c4c5e..b658b7f1ca8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -767,7 +767,7 @@ Discourse::Application.routes.draw do # Topics resource get "t/:id" => "topics#show" - put "t/:id" => "topics#update" + put "t/:topic_id" => "topics#update", constraints: { topic_id: /\d+/ } delete "t/:id" => "topics#destroy" put "t/:id/archive-message" => "topics#archive_message" put "t/:id/move-to-inbox" => "topics#move_to_inbox" diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index ed2cb355663..1be67ed2b99 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1317,6 +1317,27 @@ RSpec.describe TopicsController do expect(payload["title"]).to eq('This is a new title for the topic') end + it 'allows update on short non-slug url' do + put "/t/#{topic.id}.json", params: { + title: 'This is a new title for the topic' + } + + topic.reload + expect(topic.title).to eq('This is a new title for the topic') + end + + it 'only allows update on digit ids' do + non_digit_id = "asdf" + original_title = topic.title + put "/t/#{non_digit_id}.json", params: { + title: 'This is a new title for the topic' + } + + topic.reload + expect(topic.title).to eq(original_title) + expect(response.status).to eq(404) + end + it 'allows a change of then updating the OP' do topic.update(user: user) topic.first_post.update(user: user)