Prevent terms from being created if blank

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65007
introduced the ability to remove t&cs in the admin panel
but this had the unintended consequence of removing
the terms whenever any other application
setting was changed.

This commit reverts the changes to avoid this problem
but does reintroduce the problem that admins can no longer
remove terms and conditions.

Changelog: fixed

app/models/application_setting/term.rb

@@ -3,13 +3,12 @@
 class ApplicationSetting
   class Term < ApplicationRecord
     include CacheMarkdownField
-    include NullifyIfBlank
 
     has_many :term_agreements
 
     cache_markdown_field :terms
 
-    nullify_if_blank :terms
+    validates :terms, presence: true
 
     def self.latest
       order(:id).last

app/services/application_settings/update_service.rb

@@ -67,8 +67,10 @@ module ApplicationSettings
     end
 
     def update_terms(terms)
+      return unless terms.present?
+
       # Avoid creating a new terms record if the text is exactly the same.
-      terms = terms&.strip
+      terms = terms.strip
       return if terms == @application_setting.terms
 
       ApplicationSetting::Term.create(terms: terms)

db/migrate/20210720083432_change_application_setting_terms_not_null.rb

+# frozen_string_literal: true
+
+class ChangeApplicationSettingTermsNotNull < ActiveRecord::Migration[6.1]
+  def up
+    execute("UPDATE application_setting_terms SET terms = '' WHERE terms IS NULL")
+    change_column_null :application_setting_terms, :terms, false
+  end
+
+  def down
+    change_column_null :application_setting_terms, :terms, true
+  end
+end

db/schema_migrations/20210720083432

+6096780be4fae007485f150a019fc4555153e4b22b893d5fe29be36834d970a9
\ No newline at end of file

db/structure.sql

@@ -9216,7 +9216,7 @@ ALTER SEQUENCE appearances_id_seq OWNED BY appearances.id;
 CREATE TABLE application_setting_terms (
     id integer NOT NULL,
     cached_markdown_version integer,
-    terms text,
+    terms text NOT NULL,
     terms_html text
 );
 

spec/models/application_setting/term_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe ApplicationSetting::Term do
-  it { is_expected.to nullify_if_blank(:terms) }
+  it { is_expected.to validate_presence_of(:terms) }
 
   describe '.latest' do
     it 'finds the latest terms' do

spec/services/application_settings/update_service_spec.rb

@@ -23,8 +23,8 @@ RSpec.describe ApplicationSettings::UpdateService do
     context 'when the passed terms are blank' do
       let(:params) { { terms: '' } }
 
-      it 'does create terms' do
-        expect { subject.execute }.to change { ApplicationSetting::Term.count }.by(1)
+      it 'does not create terms' do
+        expect { subject.execute }.not_to change { ApplicationSetting::Term.count }
       end
     end