Skip to content

Commit

Permalink
Fix error caused if Help Center is already set
Browse files Browse the repository at this point in the history
  • Loading branch information
@farhatahmad
farhatahmad committed Feb 9, 2024
1 parent 08b5816 commit 167c7dc
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions db/data/20240125154727_add_help_center_setting.rb
View file
Expand Up @@ -2,9 +2,9 @@

class AddHelpCenterSetting < ActiveRecord::Migration[7.1]
def up
setting = Setting.find_or_create_by(name: 'HelpCenter')
setting = Setting.create(name: 'HelpCenter') unless Setting.exists?(name: 'HelpCenter')

This comment has been minimized.

Sign in to view

Copy link
@Ithanil

Ithanil Feb 10, 2024

Contributor

This change was not needed and is actually a regression, because now the variable setting won't be set when Setting.exists? is true.

SiteSetting.find_or_create_by(setting:, value: '', provider: 'greenlight')
SiteSetting.create(setting:, value: '', provider: 'greenlight') unless SiteSetting.exists?(setting:, value: '', provider: 'greenlight')

This comment has been minimized.

Sign in to view

Copy link
@Ithanil

Ithanil Feb 9, 2024

Contributor

This won't fix the issue in my opinion, because you still have "value: ''': in the query. If the settings exists with a non-empty value, exists? will return false and the duplicate setting will be created. I.e. this is line is equivalent to the old one.

Tenant.all.each do |tenant|
SiteSetting.find_or_create_by(setting:, value: '', provider: tenant.name)

This comment has been minimized.

Sign in to view

Copy link
@Ithanil

Ithanil Feb 9, 2024 •
edited

Contributor

Isn't the problem still present in line 10 for Tenants?
Expand Down

0 comments on commit 167c7dc

Please sign in to comment.