FIX: type object setting not redirecting on saving (#36150)

With https://github.com/discourse/discourse/pull/35349, now site
settings do not return a body on update, which broke the type object
setting editor's save flow. This commit adds handling for that case.

And also adds testing to ensure that after saving the setting, we are
redirected to another page.

Renamed `admin_objects_theme_setting_editor` to
`admin_objects_setting_editor` since it can be used for both themes and
settings.


Before:


https://github.com/user-attachments/assets/5e019428-e126-4084-80a6-eb324c851427


Now:


https://github.com/user-attachments/assets/9ac59ca2-1896-490a-866c-630a5a201471

A future todo would be to migrate [ThemeController#update_single_setting
](https://github.com/discourse/discourse/blob/71834c898f2f3f5d11df3db6f9a5bab12acbccaa/app/controllers/admin/themes_controller.rb#L343-L365)
to use the same pattern as
[`SiteSettingController#update`](https://github.com/discourse/discourse/blob/d8e7741d9645d39a637037c7720dd6fc5f261284/app/controllers/admin/site_settings_controller.rb#L25-L53)
This commit is contained in:
Gabriel Grubba
2025-11-21 10:26:08 -03:00
committed by GitHub
parent 3b2d0b45b7
commit 6800d63bfc
6 changed files with 93 additions and 42 deletions
@@ -282,7 +282,9 @@ export default class SchemaSettingNewEditor extends Component {
this.args.setting
.updateSetting(this.args.id, this.data)
.then((result) => {
this.args.setting.set("value", result[this.args.setting.setting]);
if (result) {
this.args.setting.set("value", result[this.args.setting.setting]);
}
this.router.transitionTo(this.args.routeToRedirect, this.args.id);
})
.catch((e) => {
@@ -26,7 +26,8 @@ export default class AdminSchemaRoute extends Route {
return {
setting,
settingName: params.setting_name,
goBackUrl: this.routeHistory.lastURL,
goBackUrl:
this.routeHistory.lastURL || "/admin/site_settings/category/required",
};
}
}
+21
View File
@@ -0,0 +1,21 @@
category:
objects_setting:
type: objects
default: []
schema:
name: section
properties:
name:
type: string
required: true
links:
type: objects
schema:
name: link
properties:
name:
type: string
validations:
max_length: 20
url:
type: string
@@ -1,32 +1,29 @@
# frozen_string_literal: true
RSpec.describe "Admin editing objects type theme setting", type: :system do
RSpec.describe "Admin editing objects type", type: :system do
let(:admin_objects_setting_editor_page) { PageObjects::Pages::AdminObjectsSettingEditor.new }
fab!(:admin)
fab!(:theme)
let(:objects_setting) do
theme.set_field(
target: :settings,
name: "yaml",
value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"),
)
theme.save!
theme.settings[:objects_setting]
end
let(:admin_customize_themes_page) { PageObjects::Pages::AdminCustomizeThemes.new }
let(:admin_objects_theme_setting_editor_page) do
PageObjects::Pages::AdminObjectsThemeSettingEditor.new
end
before do
objects_setting
sign_in(admin)
end
before { sign_in(admin) }
describe "when editing a theme setting of objects type" do
fab!(:theme)
let(:objects_setting) do
theme.set_field(
target: :settings,
name: "yaml",
value: File.read("#{Rails.root}/spec/fixtures/theme_settings/objects_settings.yaml"),
)
theme.save!
theme.settings[:objects_setting]
end
let(:admin_customize_themes_page) { PageObjects::Pages::AdminCustomizeThemes.new }
before { objects_setting }
it "should display the right label and description for each property if the label and description has been configured in a locale file" do
theme.set_field(
target: :translations,
@@ -36,30 +33,30 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do
theme.save!
admin_objects_theme_setting_editor_page.visit(theme, "objects_setting")
admin_objects_setting_editor_page.visit_theme(theme, "objects_setting")
expect(admin_objects_theme_setting_editor_page).to have_setting_field_description(
expect(admin_objects_setting_editor_page).to have_setting_field_description(
"name",
"Section Name",
)
expect(admin_objects_theme_setting_editor_page).to have_setting_field_label("name", "Name")
expect(admin_objects_setting_editor_page).to have_setting_field_label("name", "Name")
admin_objects_theme_setting_editor_page.click_child_link("link 1")
admin_objects_setting_editor_page.click_child_link("link 1")
expect(admin_objects_theme_setting_editor_page).to have_setting_field_description(
expect(admin_objects_setting_editor_page).to have_setting_field_description(
"name",
"Name of the link",
)
expect(admin_objects_theme_setting_editor_page).to have_setting_field_label("name", "Name")
expect(admin_objects_setting_editor_page).to have_setting_field_label("name", "Name")
expect(admin_objects_theme_setting_editor_page).to have_setting_field_description(
expect(admin_objects_setting_editor_page).to have_setting_field_description(
"url",
"URL of the link",
)
expect(admin_objects_theme_setting_editor_page).to have_setting_field_label("url", "URL")
expect(admin_objects_setting_editor_page).to have_setting_field_label("url", "URL")
end
it "should allow admin to edit the theme setting of objects type" do
@@ -73,7 +70,7 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do
)
admin_objects_theme_setting_editor =
admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting")
admin_customize_themes_page.click_edit_objects_setting_button("objects_setting")
expect(page).to have_current_path(
"/admin/customize/themes/#{theme.id}/schema/objects_setting",
@@ -81,10 +78,12 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do
admin_objects_theme_setting_editor.fill_in_field("name", "some new name").save
expect(page).to have_current_path("/admin/customize/themes/#{theme.id}")
expect(admin_customize_themes_page).to have_overridden_setting("objects_setting")
admin_objects_theme_setting_editor =
admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting")
admin_customize_themes_page.click_edit_objects_setting_button("objects_setting")
expect(admin_objects_theme_setting_editor).to have_setting_field("name", "some new name")
@@ -93,7 +92,7 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do
admin_customize_themes_page.reset_overridden_setting("objects_setting")
admin_objects_theme_setting_editor =
admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting")
admin_customize_themes_page.click_edit_objects_setting_button("objects_setting")
expect(admin_objects_theme_setting_editor).to have_setting_field("name", "section 1")
end
@@ -102,7 +101,7 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do
visit("/admin/customize/themes/#{theme.id}")
admin_objects_theme_setting_editor =
admin_customize_themes_page.click_edit_objects_theme_setting_button("objects_setting")
admin_customize_themes_page.click_edit_objects_setting_button("objects_setting")
admin_objects_theme_setting_editor
.fill_in_field("name", "")
@@ -157,4 +156,22 @@ RSpec.describe "Admin editing objects type theme setting", type: :system do
)
end
end
describe "when editing a site setting of objects type" do
before do
SiteSetting.load_settings(
File.join("#{Rails.root}/spec/fixtures/site_settings/object_settings.yml"),
)
end
it "allows an admin to edit a site setting of objects type via the settings editor" do
admin_objects_setting_editor_page.visit_setting("objects_setting")
admin_objects_setting_editor_page.add_object_in_root.fill_in_field("name", "new section").save
expect(page).to have_current_path("/admin/site_settings/category/required")
expect(JSON.parse(SiteSetting.objects_setting)).to eq(
[{ "links" => [], "name" => "new section" }],
)
end
end
end
@@ -202,9 +202,9 @@ module PageObjects
self
end
def click_edit_objects_theme_setting_button(setting_name)
def click_edit_objects_setting_button(setting_name)
find(".theme-setting[data-setting=\"#{setting_name}\"] .setting-value-edit-button").click
PageObjects::Pages::AdminObjectsThemeSettingEditor.new
PageObjects::Pages::AdminObjectsSettingEditor.new
end
def click_theme_settings_editor_button
@@ -2,12 +2,17 @@
module PageObjects
module Pages
class AdminObjectsThemeSettingEditor < PageObjects::Pages::Base
def visit(theme, setting_name)
class AdminObjectsSettingEditor < PageObjects::Pages::Base
def visit_theme(theme, setting_name)
page.visit "/admin/customize/themes/#{theme.id}/schema/#{setting_name}"
self
end
def visit_setting(setting_name)
page.visit "/admin/schema/#{setting_name}"
self
end
def has_setting_field?(field_name, value)
expect(input_field(field_name).value).to eq(value)
end
@@ -38,6 +43,11 @@ module PageObjects
self
end
def add_object_in_root
find(".schema-setting-editor__tree-add-button.--root").click
self
end
def save
click_button(I18n.t("js.save"))
self