FEATURE: Support additional metadata in theme about.json (#6944)

New `about.json` fields (all optional):
 - `authors`: An arbitrary string describing the theme authors
 - `theme_version`: An arbitrary string describing the theme version
 - `minimum_discourse_version`: Theme will be auto-disabled for lower versions. Must be a valid version descriptor.
 - `maximum_discourse_version`: Theme will be auto-disabled for lower versions. Must be a valid version descriptor.

A localized description for a theme can be provided in the language files under the `theme_metadata.description` key

The admin UI has been re-arranged to display this new information, and give more prominence to the remote theme options.

app/assets/javascripts/admin/components/themes-list.js.es6

@@ -31,7 +31,7 @@ export default Ember.Component.extend({
   )
   inactiveThemes(themes) {
     if (this.get("componentsTabActive")) {
-      return themes.filter(theme => theme.get("parentThemes.length") <= 0);
+      return themes.filter(theme => theme.get("parent_themes.length") <= 0);
     }
     return themes.filter(
       theme => !theme.get("user_selectable") && !theme.get("default")
@@ -46,7 +46,7 @@ export default Ember.Component.extend({
   )
   activeThemes(themes) {
     if (this.get("componentsTabActive")) {
-      return themes.filter(theme => theme.get("parentThemes.length") > 0);
+      return themes.filter(theme => theme.get("parent_themes.length") > 0);
     } else {
       themes = themes.filter(
         theme => theme.get("user_selectable") || theme.get("default")

app/assets/javascripts/admin/templates/customize-themes-show.hbs

@@ -16,75 +16,64 @@
     </div>
   {{/each}}
 
-  {{#if model.remote_theme}}
+  {{#unless model.enabled}}
-    {{#if model.remote_theme.remote_url}}
+    <div class="alert alert-error">
-      <a class="remote-url" href="{{model.remote_theme.remote_url}}">{{model.remote_theme.remote_url}}</a>
+      {{i18n "admin.customize.theme.required_version.error"}}
+      {{#if model.remote_theme.minimum_discourse_version}}
+        {{i18n "admin.customize.theme.required_version.minimum" version=model.remote_theme.minimum_discourse_version}}
       {{/if}}
-    <a class="url about-url" href="{{model.remote_theme.about_url}}">{{i18n "admin.customize.theme.about_theme"}}</a>
+      {{#if model.remote_theme.maximum_discourse_version}}
-    {{#if model.remote_theme.license_url}}
+        {{i18n "admin.customize.theme.required_version.minimum" version=model.remote_theme.maximum_discourse_version}}
-      <a class="url license-url" href="{{model.remote_theme.license_url}}">{{i18n "admin.customize.theme.license"}} {{d-icon "copyright"}}</a>
       {{/if}}
-  {{/if}}
-
-  {{#if model.parentThemes}}
-    <div class="control-unit">
-      <div class="mini-title">{{i18n "admin.customize.theme.component_of"}}</div>
-      <ul>
-        {{#each model.parentThemes as |theme|}}
-          <li>{{#link-to 'adminCustomizeThemes.show' theme replace=true}}{{theme.name}}{{/link-to}}</li>
-        {{/each}}
-      </ul>
     </div>
-  {{/if}}
+  {{/unless}}
 
   {{#unless model.component}}
     <div class="control-unit">
       {{inline-edit-checkbox action=(action "applyDefault") labelKey="admin.customize.theme.is_default" checked=model.default}}
       {{inline-edit-checkbox action=(action "applyUserSelectable") labelKey="admin.customize.theme.user_selectable" checked=model.user_selectable}}
     </div>
-
-    <div class="control-unit">
-      <div class="mini-title">{{i18n "admin.customize.theme.color_scheme"}}</div>
-      <div class="description">{{i18n "admin.customize.theme.color_scheme_select"}}</div>
-      <div class="control">{{combo-box content=colorSchemes
-                                       filterable=true
-                                       forceEscape=true
-                                       value=colorSchemeId
-                                       icon="paint-brush"}}
-         {{#if colorSchemeChanged}}
-            {{d-button action=(action "changeScheme") class="btn-primary submit-edit" icon="check"}}
-            {{d-button action=(action "cancelChangeScheme") class="btn-default cancel-edit" icon="times"}}
-         {{/if}}
-      </div>
-      {{#link-to 'adminCustomize.colors' class="btn btn-default edit"}}{{i18n 'admin.customize.colors.edit'}}{{/link-to}}
-    </div>
   {{/unless}}
 
+  {{#if model.remote_theme}}
+
+    {{#if model.remote_theme.remote_url}}
+      <a class="remote-url" href="{{model.remote_theme.remote_url}}">{{i18n "admin.customize.theme.source_url"}} {{d-icon "link"}}</a>
+    {{/if}}
+    {{#if model.remote_theme.about_url}}
+    <a class="url about-url" href="{{model.remote_theme.about_url}}">{{i18n "admin.customize.theme.about_theme"}} {{d-icon "link"}}</a>
+    {{/if}}
+    {{#if model.remote_theme.license_url}}
+      <a class="url license-url" href="{{model.remote_theme.license_url}}">{{i18n "admin.customize.theme.license"}} {{d-icon "link"}}</a>
+    {{/if}}
+
+    {{#if model.description}}
+      <span class="theme-description">{{model.description}}</span>
+    {{/if}}
+
+    <span class="metadata">
+      {{#if model.remote_theme.authors}}<span class="authors"><span class="heading">{{i18n "admin.customize.theme.authors"}}</span> {{model.remote_theme.authors}}</span>{{/if}}
+      {{#if model.remote_theme.theme_version}}<span class="version"><span class="heading">{{i18n "admin.customize.theme.version"}}</span> {{model.remote_theme.theme_version}}</span>{{/if}}
+    </span>
+
     <div class="control-unit">
-    <div class="mini-title">{{i18n "admin.customize.theme.css_html"}}</div>
+      {{#if model.remote_theme.is_git}}
-    {{#if model.hasEditedFields}}
+
-      <div class="description">{{i18n "admin.customize.theme.custom_sections"}}</div>
+        {{#if showRemoteError}}
-      <ul>
+          <div class="error-message">
-        {{#each editedFieldsFormatted as |field|}}
+            {{d-icon "exclamation-triangle"}} {{I18n "admin.customize.theme.repo_unreachable"}}
-          <li>{{field}}</li>
+          </div>
-        {{/each}}
+          <div class="raw-error">
-      </ul>
+            <code>{{model.remoteError}}</code>
-    {{else}}
-      <div class="description">
-        {{i18n "admin.customize.theme.edit_css_html_help"}}
           </div>
         {{/if}}
         
-    {{#if model.remote_theme.is_git}}
         {{#if model.remote_theme.commits_behind}}
           {{#d-button action=(action "updateToLatest") icon="download" class='btn-primary'}}{{i18n "admin.customize.theme.update_to_latest"}}{{/d-button}}
         {{else}}
           {{#d-button action=(action "checkForThemeUpdates") icon="refresh" class="btn-default"}}{{i18n "admin.customize.theme.check_for_updates"}}{{/d-button}}
         {{/if}}
-    {{/if}}
         
-    {{#d-button action=(action "editTheme") class="btn btn-default edit"}}{{i18n 'admin.customize.theme.edit_css_html'}}{{/d-button}}
-    {{#if model.remote_theme.is_git}}
         <span class='status-message'>
             {{#if updatingRemote}}
               {{i18n 'admin.customize.theme.updating'}}
@@ -103,20 +92,60 @@
               {{/if}}
             {{/if}}
           </span>
-      {{#if showRemoteError}}
+      {{else}}
-        <div class="error-message">
-          {{d-icon "exclamation-triangle"}} {{I18n "admin.customize.theme.repo_unreachable"}}
-        </div>
-        <div class="raw-error">
-          <code>{{model.remoteError}}</code>
-        </div>
-      {{/if}}
-    {{else if model.remote_theme}}
         <span class='status-message'>
           {{d-icon "info-circle"}} {{i18n "admin.customize.theme.imported_from_archive"}}
         </span>
       {{/if}}
     </div>
+  {{/if}}
+
+  {{#unless model.component}}
+    <div class="control-unit">
+      <div class="mini-title">{{i18n "admin.customize.theme.color_scheme"}}</div>
+      <div class="description">{{i18n "admin.customize.theme.color_scheme_select"}}</div>
+      <div class="control">{{combo-box content=colorSchemes
+                                       filterable=true
+                                       forceEscape=true
+                                       value=colorSchemeId
+                                       icon="paint-brush"}}
+         {{#if colorSchemeChanged}}
+            {{d-button action=(action "changeScheme") class="btn-primary submit-edit" icon="check"}}
+            {{d-button action=(action "cancelChangeScheme") class="btn-default cancel-edit" icon="times"}}
+         {{/if}}
+      </div>
+      {{#link-to 'adminCustomize.colors' class="btn btn-default edit"}}{{i18n 'admin.customize.colors.edit'}}{{/link-to}}
+    </div>
+  {{/unless}}
+
+  {{#if parentThemes}}
+    <div class="control-unit">
+      <div class="mini-title">{{i18n "admin.customize.theme.component_of"}}</div>
+      <ul>
+        {{#each parentThemes as |theme|}}
+          <li>{{#link-to 'adminCustomizeThemes.show' theme replace=true}}{{theme.name}}{{/link-to}}</li>
+        {{/each}}
+      </ul>
+    </div>
+  {{/if}}
+
+  <div class="control-unit">
+    <div class="mini-title">{{i18n "admin.customize.theme.css_html"}}</div>
+    {{#if model.hasEditedFields}}
+      <div class="description">{{i18n "admin.customize.theme.custom_sections"}}</div>
+      <ul>
+        {{#each editedFieldsFormatted as |field|}}
+          <li>{{field}}</li>
+        {{/each}}
+      </ul>
+    {{else}}
+      <div class="description">
+        {{i18n "admin.customize.theme.edit_css_html_help"}}
+      </div>
+    {{/if}}
+
+    {{#d-button action=(action "editTheme") class="btn btn-default edit"}}{{i18n 'admin.customize.theme.edit_css_html'}}{{/d-button}}
+  </div>
 
   <div class="control-unit">
     <div class="mini-title">{{i18n "admin.customize.theme.uploads"}}</div>

app/assets/stylesheets/common/admin/customize.scss

@@ -137,26 +137,35 @@
     display: inline-block;
     vertical-align: top;
 
-    .url {
-      margin-bottom: 10px;
-    }
-
     .title {
       font-size: $font-up-4;
       font-weight: bold;
       margin-bottom: 10px;
     }
+
+    .theme-description {
+      display: block;
+      margin: 10px 0;
+    }
+
+    .metadata {
+      .authors,
+      .version {
+        display: block;
+
+        .heading {
+          font-weight: bold;
+        }
+      }
+    }
+
     .remote-url,
     .about-url,
     .license-url {
-      display: block;
+      display: inline-block;
-      margin-bottom: 10px;
+      margin-right: 10px;
-    }
-    .remote-url {
-      margin-top: -5px;
-      font-size: $font-down-1;
-      font-style: italic;
     }
+
     .mini-title {
       font-size: $font-up-1;
       font-weight: bold;
@@ -347,6 +356,7 @@
     }
     .setting-label {
       width: 25%;
+      word-wrap: break-word;
       h3 {
         margin-top: 0;
         margin-bottom: 0.5rem;

app/models/remote_theme.rb

@@ -3,6 +3,14 @@ require_dependency 'theme_store/tgz_importer'
 require_dependency 'upload_creator'
 
 class RemoteTheme < ActiveRecord::Base
+  METADATA_PROPERTIES = %i{
+                            license_url
+                            about_url
+                            authors
+                            theme_version
+                            minimum_discourse_version
+                            maximum_discourse_version
+                          }
 
   class ImportError < StandardError; end
 
@@ -16,6 +24,8 @@ class RemoteTheme < ActiveRecord::Base
     joins("JOIN themes ON themes.remote_theme_id = remote_themes.id").where.not(remote_url: "")
   }
 
+  validates_format_of :minimum_discourse_version, :maximum_discourse_version, with: Discourse::VERSION_REGEXP, allow_nil: true
+
   def self.extract_theme_info(importer)
     JSON.parse(importer["about.json"])
   rescue TypeError, JSON::ParserError
@@ -123,8 +133,12 @@ class RemoteTheme < ActiveRecord::Base
       end
     end
 
-    self.license_url = theme_info["license_url"]
+    METADATA_PROPERTIES.each do |property|
-    self.about_url = theme_info["about_url"]
+      self.public_send(:"#{property}=", theme_info[property.to_s])
+    end
+    if !self.valid?
+      raise ImportError, I18n.t("themes.import_error.about_json_values", errors: self.errors.full_messages.join(","))
+    end
 
     importer.all_files.each do |filename|
       next unless opts = ThemeField.opts_from_file_path(filename)

app/models/theme.rb

@@ -7,7 +7,6 @@ require_dependency 'theme_translation_parser'
 require_dependency 'theme_translation_manager'
 
 class Theme < ActiveRecord::Base
-
   # TODO: remove in 2019
   self.ignored_columns = ["key"]
 
@@ -23,7 +22,7 @@ class Theme < ActiveRecord::Base
   has_many :child_themes, -> { order(:name) }, through: :child_theme_relation, source: :child_theme
   has_many :parent_themes, -> { order(:name) }, through: :parent_theme_relation, source: :parent_theme
   has_many :color_schemes
-  belongs_to :remote_theme
+  belongs_to :remote_theme, autosave: true
 
   has_one :settings_field, -> { where(target_id: Theme.targets[:settings], name: "yaml") }, class_name: 'ThemeField'
 
@@ -53,9 +52,6 @@ class Theme < ActiveRecord::Base
 
     Theme.expire_site_cache! if saved_change_to_user_selectable? || saved_change_to_name?
 
-    @dependant_themes = nil
-    @included_themes = nil
-
     remove_from_cache!
     clear_cached_settings!
     ColorScheme.hex_cache.clear
@@ -125,16 +121,24 @@ class Theme < ActiveRecord::Base
   end
 
   def self.transform_ids(ids, extend: true)
-    return [] if ids.blank?
+    get_set_cache "#{extend ? "extended_" : ""}transformed_ids_#{ids.join("_")}" do
+      next [] if ids.blank?
 
+      ids = ids.dup
       ids.uniq!
-    parent = ids.first
+      parent = ids.shift
 
-    components = ids[1..-1]
+      components = ids
       components.push(*components_for(parent)) if extend
       components.sort!.uniq!
 
-    [parent, *components]
+      all_ids = [parent, *components]
+
+      enabled_ids = Theme.where(id: all_ids).includes(:remote_theme)
+        .select(&:enabled?).pluck(:id)
+
+      all_ids & enabled_ids # Maintain ordering using intersection
+    end
   end
 
   def set_default!
@@ -151,6 +155,18 @@ class Theme < ActiveRecord::Base
     SiteSetting.default_theme_id == id
   end
 
+  def enabled?
+    if minimum_version = remote_theme&.minimum_discourse_version
+      return false unless Discourse.has_needed_version?(Discourse::VERSION::STRING, minimum_version)
+    end
+
+    if maximum_version = remote_theme&.maximum_discourse_version
+      return false unless Discourse.has_needed_version?(maximum_version, Discourse::VERSION::STRING)
+    end
+
+    true
+  end
+
   def component_validations
     return unless component
 
@@ -234,7 +250,7 @@ class Theme < ActiveRecord::Base
   end
 
   def notify_theme_change(with_scheme: false)
-    theme_ids = (dependant_themes&.pluck(:id) || []).unshift(self.id)
+    theme_ids = Theme.transform_ids([id])
     self.class.notify_theme_change(theme_ids, with_scheme: with_scheme)
   end
 
@@ -244,30 +260,6 @@ class Theme < ActiveRecord::Base
     end
   end
 
-  def dependant_themes
-    @dependant_themes ||= resolve_dependant_themes(:up)
-  end
-
-  def included_themes
-    @included_themes ||= resolve_dependant_themes(:down)
-  end
-
-  def resolve_dependant_themes(direction)
-    if direction == :up
-      join_field = "parent_theme_id"
-      where_field = "child_theme_id"
-    elsif direction == :down
-      join_field = "child_theme_id"
-      where_field = "parent_theme_id"
-    else
-      raise "Unknown direction"
-    end
-
-    return [] unless id
-
-    Theme.joins("JOIN child_themes ON themes.id = child_themes.#{join_field}").where("#{where_field} = ?", id)
-  end
-
   def self.resolve_baked_field(theme_ids, target, name)
     list_baked_fields(theme_ids, target, name).map { |f| f.value_baked || f.value }.join("\n")
   end
@@ -293,7 +285,7 @@ class Theme < ActiveRecord::Base
   end
 
   def list_baked_fields(target, name)
-    theme_ids = (included_themes&.pluck(:id) || []).unshift(self.id)
+    theme_ids = Theme.transform_ids([id])
     self.class.list_baked_fields(theme_ids, target, name)
   end
 
@@ -338,7 +330,7 @@ class Theme < ActiveRecord::Base
 
   def all_theme_variables
     fields = {}
-    ids = (included_themes&.pluck(:id) || []).unshift(self.id)
+    ids = Theme.transform_ids([id])
     ThemeField.find_by_theme_ids(ids).where(type_id: ThemeField.theme_var_type_ids).each do |field|
       next if fields.key?(field.name)
       fields[field.name] = field
@@ -349,18 +341,22 @@ class Theme < ActiveRecord::Base
   def add_child_theme!(theme)
     new_relation = child_theme_relation.new(child_theme_id: theme.id)
     if new_relation.save
-      @included_themes = nil
       child_themes.reload
       save!
+      Theme.clear_cache!
     else
       raise Discourse::InvalidParameters.new(new_relation.errors.full_messages.join(", "))
     end
   end
 
-  def translations
+  def internal_translations
+    translations(internal: true)
+  end
+
+  def translations(internal: false)
     fallbacks = I18n.fallbacks[I18n.locale]
     begin
-      data = theme_fields.find_first_locale_fields([id], fallbacks).first&.translation_data(with_overrides: false)
+      data = theme_fields.find_first_locale_fields([id], fallbacks).first&.translation_data(with_overrides: false, internal: internal)
       return {} if data.nil?
       best_translations = {}
       fallbacks.reverse.each do |locale|
@@ -400,7 +396,7 @@ class Theme < ActiveRecord::Base
   def included_settings
     hash = {}
 
-    self.included_themes.each do |theme|
+    Theme.where(id: Theme.transform_ids([id])).each do |theme|
       hash.merge!(theme.cached_settings)
     end
 
@@ -435,17 +431,21 @@ class Theme < ActiveRecord::Base
   end
 
   def generate_metadata_hash
-    {
+    {}.tap do |meta|
-      name: name,
+      meta[:name] = name
-      about_url: remote_theme&.about_url,
+      meta[:component] = component
-      license_url: remote_theme&.license_url,
+
-      component: component,
+      RemoteTheme::METADATA_PROPERTIES.each do |property|
-      assets: {}.tap do |hash|
+        meta[property] = remote_theme&.public_send(property)
+      end
+
+      meta[:assets] = {}.tap do |hash|
         theme_fields.where(type_id: ThemeField.types[:theme_upload_var]).each do |field|
           hash[field.name] = "assets/#{field.upload.original_filename}"
         end
-      end,
+      end
-      color_schemes: {}.tap do |hash|
+
+      meta[:color_schemes] = {}.tap do |hash|
         schemes = self.color_schemes
         # The selected color scheme may not belong to the theme, so include it anyway
         schemes = [self.color_scheme] + schemes if self.color_scheme
@@ -453,7 +453,8 @@ class Theme < ActiveRecord::Base
           hash[scheme.name] = {}.tap { |colors| scheme.colors.each { |color| colors[color.name] = color.hex } }
         end
       end
-    }
+
+    end
   end
 end
 

app/models/theme_field.rb

@@ -119,17 +119,17 @@ class ThemeField < ActiveRecord::Base
     [doc.to_s, errors&.join("\n")]
   end
 
-  def raw_translation_data
+  def raw_translation_data(internal: false)
     # Might raise ThemeTranslationParser::InvalidYaml
-    ThemeTranslationParser.new(self).load
+    ThemeTranslationParser.new(self, internal: internal).load
   end
 
-  def translation_data(with_overrides: true)
+  def translation_data(with_overrides: true, internal: false)
     fallback_fields = theme.theme_fields.find_locale_fields([theme.id], I18n.fallbacks[name])
 
     fallback_data = fallback_fields.each_with_index.map do |field, index|
       begin
-        field.raw_translation_data
+        field.raw_translation_data(internal: internal)
       rescue ThemeTranslationParser::InvalidYaml
         # If this is the locale with the error, raise it.
         # If not, let the other theme_field raise the error when it processes itself

app/serializers/theme_serializer.rb

@@ -45,9 +45,9 @@ class BasicThemeSerializer < ApplicationSerializer
 end
 
 class RemoteThemeSerializer < ApplicationSerializer
-  attributes :id, :remote_url, :remote_version, :local_version, :about_url,
+  attributes :id, :remote_url, :remote_version, :local_version, :commits_behind,
-             :license_url, :commits_behind, :remote_updated_at, :updated_at,
+             :remote_updated_at, :updated_at, :github_diff_link, :last_error_text, :is_git?,
-             :github_diff_link, :last_error_text, :is_git?
+             :license_url, :about_url, :authors, :theme_version, :minimum_discourse_version, :maximum_discourse_version
 
   # wow, AMS has some pretty nutty logic where it tries to find the path here
   # from action dispatch, tell it not to
@@ -61,7 +61,7 @@ class RemoteThemeSerializer < ApplicationSerializer
 end
 
 class ThemeSerializer < BasicThemeSerializer
-  attributes :color_scheme, :color_scheme_id, :user_selectable, :remote_theme_id, :settings, :errors
+  attributes :color_scheme, :color_scheme_id, :user_selectable, :remote_theme_id, :settings, :errors, :enabled?, :description
 
   has_one :user, serializer: UserNameSerializer, embed: :object
 
@@ -102,4 +102,8 @@ class ThemeSerializer < BasicThemeSerializer
   def include_errors?
     @errors.present?
   end
+
+  def description
+    object.internal_translations.find  { |t| t.key == "theme_metadata.description" } &.value
+  end
 end

config/locales/client.en.yml

@@ -3386,8 +3386,15 @@ en:
           is_private: "Theme is in a private git repository"
           remote_branch: "Branch name (optional)"
           public_key: "Grant the following public key access to the repo:"
-          about_theme: "About Theme"
+          about_theme: "About"
           license: "License"
+          version: "Version:"
+          authors: "Authored by:"
+          source_url: "Source"
+          required_version:
+            error: "This theme has been automatically disabled because it is not compatible with this version of Discourse."
+            minimum: "Requires Discourse version {{version}} or above."
+            maximum: "Requires Discourse version {{version}} or below."
           component_of: "Component of:"
           update_to_latest: "Update to Latest"
           check_for_updates: "Check for Updates"

config/locales/server.en.yml

@@ -77,6 +77,7 @@ en:
     import_error:
       generic: An error occured while importing that theme
       about_json: "Import Error: about.json does not exist, or is invalid"
+      about_json_values: "about.json contains invalid values: %{errors}"
       git: "Error cloning git repository, access is denied or repository is not found"
       unpack_failed: "Failed to unpack file"
     errors:

db/migrate/20190122132732_add_fields_to_remote_themes.rb

@@ -0,0 +1,8 @@
+class AddFieldsToRemoteThemes < ActiveRecord::Migration[5.2]
+  def change
+    add_column :remote_themes, :authors, :string
+    add_column :remote_themes, :theme_version, :string
+    add_column :remote_themes, :minimum_discourse_version, :string
+    add_column :remote_themes, :maximum_discourse_version, :string
+  end
+end

lib/theme_translation_parser.rb

@@ -1,8 +1,10 @@
 class ThemeTranslationParser
+  INTERNAL_KEYS = [:theme_metadata]
   class InvalidYaml < StandardError; end
 
-  def initialize(setting_field)
+  def initialize(setting_field, internal: internal)
     @setting_field = setting_field
+    @internal = internal
   end
 
   def self.check_contains_hashes(hash)
@@ -22,6 +24,9 @@ class ThemeTranslationParser
 
     parsed.deep_symbolize_keys!
 
+    parsed[@setting_field.name.to_sym].slice!(*INTERNAL_KEYS) if @internal
+    parsed[@setting_field.name.to_sym].except!(*INTERNAL_KEYS) if !@internal
+
     parsed
   end
 end

lib/version.rb

@@ -1,4 +1,6 @@
 module Discourse
+  VERSION_REGEXP = /\A\d+\.\d+\.\d+(\.beta\d+)?\z/ unless defined? ::Discourse::VERSION_REGEXP
+
   # work around reloader
   unless defined? ::Discourse::VERSION
     module VERSION #:nodoc:

spec/components/theme_store/tgz_exporter_spec.rb

@@ -11,7 +11,9 @@ describe ThemeStore::TgzExporter do
       image = file_from_fixtures("logo.png")
       upload = UploadCreator.new(image, "logo.png").create_for(-1)
       theme.set_field(target: :common, name: :logo, upload_id: upload.id, type: :theme_upload_var)
-      theme.build_remote_theme(remote_url: "", about_url: "abouturl", license_url: "licenseurl")
+      theme.build_remote_theme(remote_url: "", about_url: "abouturl", license_url: "licenseurl",
+                               authors: "David Taylor", theme_version: "1.0", minimum_discourse_version: "1.0.0",
+                               maximum_discourse_version: "3.0.0.beta1")
 
       cs1 = Fabricate(:color_scheme, name: 'Orphan Color Scheme', color_scheme_colors: [
         Fabricate(:color_scheme_color, name: 'header_primary',  hex: 'F0F0F0'),
@@ -71,6 +73,10 @@ describe ThemeStore::TgzExporter do
         "assets": {
           "logo": "assets/logo.png"
         },
+        "authors": "David Taylor",
+        "minimum_discourse_version": "1.0.0",
+        "maximum_discourse_version": "3.0.0.beta1",
+        "theme_version": "1.0",
         "color_schemes": {
           "Orphan Color Scheme": {
             "header_primary": "F0F0F0",

spec/models/remote_theme_spec.rb

@@ -24,6 +24,8 @@ describe RemoteTheme do
           "name": "awesome theme",
           "about_url": "#{about_url}",
           "license_url": "https://www.site.com/license",
+          "theme_version": "1.0",
+          "minimum_discourse_version": "1.0.0",
           "assets": {
             "font": "assets/awesome.woff2"
           },
@@ -72,6 +74,8 @@ describe RemoteTheme do
 
       expect(remote.about_url).to eq("https://www.site.com/about")
       expect(remote.license_url).to eq("https://www.site.com/license")
+      expect(remote.theme_version).to eq("1.0")
+      expect(remote.minimum_discourse_version).to eq("1.0.0")
 
       expect(@theme.theme_fields.length).to eq(6)
 

spec/models/theme_spec.rb

@@ -57,10 +57,12 @@ describe Theme do
 
   end
 
-  it 'can correctly find parent themes' do
+  it "can automatically disable for mismatching version" do
-    theme.add_child_theme!(child)
+    expect(theme.enabled?).to eq(true)
+    theme.create_remote_theme!(remote_url: "", minimum_discourse_version: "99.99.99")
+    expect(theme.enabled?).to eq(false)
 
-    expect(child.dependant_themes.length).to eq(1)
+    expect(Theme.transform_ids([theme.id])).to be_empty
   end
 
   it "doesn't allow multi-level theme components" do
@@ -174,30 +176,34 @@ HTML
   end
 
   describe ".transform_ids" do
+    let!(:orphan1) { Fabricate(:theme, component: true) }
     let!(:child) { Fabricate(:theme, component: true) }
     let!(:child2) { Fabricate(:theme, component: true) }
+    let!(:orphan2) { Fabricate(:theme, component: true) }
+    let!(:orphan3) { Fabricate(:theme, component: true) }
+    let!(:orphan4) { Fabricate(:theme, component: true) }
 
     before do
       theme.add_child_theme!(child)
       theme.add_child_theme!(child2)
     end
 
+    it "returns an empty array if no ids are passed" do
+      expect(Theme.transform_ids([])).to eq([])
+    end
+
     it "adds the child themes of the parent" do
       sorted = [child.id, child2.id].sort
 
       expect(Theme.transform_ids([theme.id])).to eq([theme.id, *sorted])
 
-      fake_id = [child.id, child2.id, theme.id].min - 5
+      expect(Theme.transform_ids([theme.id, orphan1.id, orphan2.id])).to eq([theme.id, orphan1.id, *sorted, orphan2.id])
-      fake_id2 = [child.id, child2.id, theme.id].max + 5
-
-      expect(Theme.transform_ids([theme.id, fake_id2, fake_id]))
-        .to eq([theme.id, fake_id, *sorted, fake_id2])
     end
 
     it "doesn't insert children when extend is false" do
-      fake_id = theme.id + 1
+      fake_id = orphan2.id
-      fake_id2 = fake_id + 2
+      fake_id2 = orphan3.id
-      fake_id3 = fake_id2 + 3
+      fake_id3 = orphan4.id
 
       expect(Theme.transform_ids([theme.id], extend: false)).to eq([theme.id])
       expect(Theme.transform_ids([theme.id, fake_id3, fake_id, fake_id2, fake_id2], extend: false))
@@ -466,6 +472,8 @@ HTML
     it "can list working theme_translation_manager objects" do
       en_translation = ThemeField.create!(theme_id: theme.id, name: "en", type_id: ThemeField.types[:yaml], target_id: Theme.targets[:translations], value: <<~YAML)
         en:
+          theme_metadata:
+            description: "Description of my theme"
           group_of_translations:
             translation1: en test1
             translation2: en test2
@@ -510,6 +518,18 @@ HTML
       ])
     end
 
+    it "can list internal theme_translation_manager objects" do
+      en_translation = ThemeField.create!(theme_id: theme.id, name: "en", type_id: ThemeField.types[:yaml], target_id: Theme.targets[:translations], value: <<~YAML)
+        en:
+          theme_metadata:
+            description: "Description of my theme"
+          another_translation: en test4
+      YAML
+      translations = theme.internal_translations
+      expect(translations.map(&:key)).to contain_exactly("theme_metadata.description")
+      expect(translations.map(&:value)).to contain_exactly("Description of my theme")
+    end
+
     it "can create a hash of overridden values" do
       en_translation = ThemeField.create!(theme_id: theme.id, name: "en", type_id: ThemeField.types[:yaml], target_id: Theme.targets[:translations], value: <<~YAML)
         en:

test/javascripts/admin/components/themes-list-test.js.es6

@@ -10,7 +10,8 @@ const components = [1, 2, 3, 4, 5].map(num =>
   Theme.create({
     name: `Child ${num}`,
     component: true,
-    parentThemes: [themes[num - 1]]
+    parentThemes: [themes[num - 1]],
+    parent_themes: [1, 2, 3, 4, 5]
   })
 );