Work on new parameter model

assets/javascripts/discourse/components/param-input.js.es6

@@ -0,0 +1,92 @@
+const layoutMap = {
+  int: 'int',
+  bigint: 'int',
+  boolean: 'boolean',
+  string: 'generic',
+  time: 'generic',
+  date: 'generic',
+  datetime: 'generic',
+  double: 'string',
+  inet: 'generic',
+  user_id: 'user_id',
+  post_id: 'string',
+  topic_id: 'int',
+  category_id: 'int',
+  group_id: 'int',
+  badge_id: 'int',
+  int_list: 'generic',
+  string_list: 'generic'
+};
+
+function allowsInputTypeTime() {
+  try {
+    const inp = document.createElement('input');
+    inp.attributes.type = 'time';
+    inp.attributes.type = 'date';
+    return true;
+  } catch (e) {
+    return false;
+  }
+}
+
+export default Ember.Component.extend({
+
+  classNameBindings: ['valid:valid:invalid', ':param'],
+
+  boolTypes: [ {name: I18n.t('explorer.types.bool.true'), id: 'Y'}, {name: I18n.t('explorer.types.bool.false'), id: 'N'}, {name: I18n.t('explorer.types.bool.null_'), id: '#null'} ],
+
+  value: function(key, value, previousValue) {
+    if (arguments.length > 1) {
+      this.get('params')[this.get('info.identifier')] = value.toString();
+    }
+    return this.get('params')[this.get('info.identifier')];
+  }.property('params', 'pname'),
+
+  valid: function() {
+    const type = this.get('info.type'),
+      value = this.get('value');
+
+    if (Em.isEmpty(this.get('value'))) {
+      return this.get('info.nullable');
+    }
+
+    function matches(regex) {
+      return regex.test(value);
+    }
+
+    const intVal = parseInt(value, 10);
+    const intValid = !isNaN(intVal) && intVal < 2147483648 && intVal > -2147483649;
+    switch (type) {
+      case 'int':
+        return /^-?\d+$/.test(value) && intValid;
+      case 'bigint':
+        return /^-?\d+$/.test(value) && !isNaN(intVal);
+      case 'boolean':
+        return /^Y|N|#null|true|false/.test(value);
+      case 'double':
+        return !isNaN(parseFloat(value));
+      case 'int_list':
+        return value.split(',').every(function(i) {
+          return /^(-?\d+|null)$/.test(i.trim());
+        });
+      case 'post_id':
+        return /^\d+$/.test(value) || /\d+\/\d+(\?u=.*)?$/.test(value);
+    }
+    return true;
+  }.property('value', 'info.type', 'info.nullable'),
+
+  layoutType: function() {
+    const type = this.get('info.type');
+    if ((type === "time" || type === "date") && !allowsInputTypeTime()) {
+      return "string";
+    }
+    if (layoutMap[type]) {
+      return layoutMap[type];
+    }
+    return type;
+  }.property('info.type'),
+
+  layoutName: function() {
+    return "admin/components/q-params/" + this.get('layoutType');
+  }.property('layoutType')
+});

assets/javascripts/discourse/models/query.js.es6

@@ -7,37 +7,35 @@ const Query = RestModel.extend({
 
   _init: function() {
     this._super();
-    if (!this.get('options')) {
-      this.set('options', {defaults:{}});
-    }
     this.set('dirty', false);
   }.on('init'),
 
   _initParams: function() {
     this.resetParams();
-  }.on('init').observes('param_names'),
+  }.on('init').observes('param_info'),
-
-  // the server uses 'qopts' and the client uses 'options' due to ActiveRecord
-  // freaking out if a serialized value is named 'options'
-  options: Em.computed.alias('qopts'),
 
   markDirty: function() {
     this.set('dirty', true);
-  }.observes('name', 'description', 'sql', 'options', 'options.defaults'),
+  }.observes('name', 'description', 'sql'),
 
   markNotDirty() {
     this.set('dirty', false);
   },
 
+  hasParams: function() {
+    return this.get('param_info.length') > 0;
+  }.property('param_info'),
+
   resetParams() {
     const newParams = {};
     const oldParams = this.get('params');
-    const defaults = this.get('options.defaults') || {};
+    const paramInfo = this.get('param_info') || [];
-    (this.get('param_names') || []).forEach(function(name) {
+    paramInfo.forEach(function(pinfo) {
-      if (defaults[name]) {
+      const name = pinfo.identifier;
-        newParams[name] = defaults[name];
+      if (oldParams[pinfo.identifier]) {
-      } else if (oldParams[name]) {
         newParams[name] = oldParams[name];
+      } else if (pinfo['default'] !== null) {
+        newParams[name] = pinfo['default'];
       } else {
         newParams[name] = '';
       }
@@ -45,19 +43,6 @@ const Query = RestModel.extend({
     this.set('params', newParams);
   },
 
-  saveDefaults() {
-    const currentParams = this.get('params');
-    let defaults = {};
-    (this.get('param_names') || []).forEach(function(name) {
-      if (currentParams[name]) {
-        defaults[name] = currentParams[name];
-      } else {
-        delete defaults[name];
-      }
-    });
-    this.set('options.defaults', defaults);
-  },
-
   downloadUrl: function() {
     // TODO - can we change this to use the store/adapter?
     return Discourse.getURL("/admin/plugins/explorer/queries/" + this.get('id') + ".json?export=1");
@@ -88,15 +73,11 @@ const Query = RestModel.extend({
       props.id = this.get('id');
     }
     return props;
-  },
-
-  run() {
-    console.log("Called query#run");
   }
 });
 
 Query.reopenClass({
-  updatePropertyNames: ["name", "description", "sql", "qopts"]
+  updatePropertyNames: ["name", "description", "sql"]
 });
 
 export default Query;

assets/javascripts/discourse/templates/admin/components/q-params/boolean.hbs

@@ -0,0 +1,6 @@
+{{#if info.nullable}}
+  {{combo-box valueAttribute="id" value=value nameProperty="name" content=boolTypes}}
+{{else}}
+  {{input type="checkbox" checked=value}}
+{{/if}}
+<span class="param-name">{{info.identifier}}</span>

assets/javascripts/discourse/templates/admin/components/q-params/generic.hbs

@@ -0,0 +1,2 @@
+{{text-field value=value}}
+<span class="param-name">{{info.identifier}}</span>

assets/javascripts/discourse/templates/admin/components/q-params/int.hbs

@@ -0,0 +1,2 @@
+{{input type="number" value=value}}
+<span class="param-name">{{info.identifier}}</span>

assets/javascripts/discourse/templates/admin/components/q-params/string.hbs

@@ -0,0 +1,2 @@
+{{text-field value=value}}
+<span class="param-name">{{info.identifier}}</span>

assets/javascripts/discourse/templates/admin/components/q-params/user_id.hbs

@@ -0,0 +1,2 @@
+{{user-selector usernames=value single="true"}}
+<span class="param-name">{{info.identifier}}</span>

assets/javascripts/discourse/templates/admin/plugins-explorer.hbs

@@ -75,14 +75,20 @@
 
       <div class="clear"></div>
       <div class="pull-left">
-        {{d-button action="save" label="explorer.save" disabled=saveDisabled class="btn-primary"}}
+        {{#if everEditing}}
+          {{d-button action="save" label="explorer.save" disabled=saveDisabled class="btn-primary"}}
+        {{else}}
+          {{d-button action="editName" label="explorer.edit" icon="pencil" class="btn-primary"}}
+        {{/if}}
         {{d-button action="download" label="explorer.export" disabled=runDisabled icon="download"}}
       </div>
       <div class="pull-right">
         {{#if selectedItem.destroyed}}
           {{d-button action="recover" class="" icon="undo" label="explorer.recover"}}
         {{else}}
-          {{d-button action="discard" class="btn-danger" icon="undo" label="explorer.undo" disabled=saveDisabled}}
+          {{#if everEditing}}
+            {{d-button action="discard" class="btn-danger" icon="undo" label="explorer.undo" disabled=saveDisabled}}
+          {{/if}}
           {{d-button action="destroy" class="btn-danger" icon="trash" label="explorer.delete"}}
         {{/if}}
       </div>
@@ -91,17 +97,14 @@
   </div>
 
   <form class="query-run" {{action "run" on="submit"}}>
-    {{#if selectedItem.param_names}}
+    {{#if selectedItem.hasParams}}
       <div class="query-params">
-        <div class="param-save">
+        {{#each selectedItem.param_info as |pinfo|}}
-          {{d-button action="saveDefaults" label="explorer.save_params" type="button"}}
+          {{param-input params=selectedItem.params info=pinfo}}
-          {{d-button action="resetParams" label="explorer.reset_params" type="button"}}
+          {{! <div class="param">
-        </div>
+            {{param-field params=selectedItem.params pname=pinfo.identifier type=pinfo.type}
-        {{#each selectedItem.param_names as |pname|}}
+            <span class="param-name">{{pinfo.identifier}</span>
-          <div class="param">
+          </div> }}
-            {{param-field params=selectedItem.params pname=pname}}
-            <span class="param-name">{{pname}}</span>
-          </div>
         {{/each}}
       </div>
     {{/if}}

assets/stylesheets/explorer.scss

@@ -156,16 +156,21 @@
 
 .query-params {
   border: 1px solid dark-light-diff($primary, $secondary, 60%, -20%);
-  input {
+  .param > input {
     margin: 9px;
   }
+  .invalid > input {
+    background-color: mix($danger, $secondary, 20%);
+  }
+  .invalid .ac-wrap {
+    background-color: mix($danger, $secondary, 20%);
+  }
   .param {
     display: inline-block;
     overflow-x: visible;
-  }
+    .ac-wrap {
-  .param-save {
+      display: inline-block;
-    float: right;
+    }
-    margin: 9px;
   }
   .param-name {
     display: inline-block;

config/locales/client.en.yml

@@ -33,11 +33,17 @@ en:
         filter: "Search..."
         sensitive: "The contents of this column may contain particularly sensitive or private information. Please exercise caution when using the contents of this column."
         type_help: "<a href='http://www.postgresql.org/docs/9.3/static/datatype.html#DATATYPE-TABLE' target='_blank'>Types</a>"
+      types:
+        bool:
+          yes: "Yes"
+          no: "No"
+          null_: "Null"
       export: "Export"
       save: "Save Changes"
       saverun: "Save Changes and Run Query"
       run: "Run Query"
       undo: "Discard Changes"
+      edit: "Edit"
       delete: "Delete"
       recover: "Undelete Query"
       download_json: "Download Results"

plugin.rb

@@ -275,7 +275,7 @@ SQL
     end
 
     def check_params!
-      params
+      DataExplorer::Parameter.create_from_sql(sql, strict: true)
       nil
     end
 
@@ -389,7 +389,7 @@ SQL
     def self.types
       @types ||= Enum.new(
         # Normal types
-        :int, :bigint, :boolean, :string, :time, :double,
+        :int, :bigint, :boolean, :string, :date, :time, :datetime, :double, :inet,
         # Selection help
         :user_id, :post_id, :topic_id, :category_id, :group_id, :badge_id,
         # Arrays
@@ -401,6 +401,8 @@ SQL
       @type_aliases ||= {
         integer: :int,
         text: :string,
+        timestamp: :datetime,
+        ipaddr: :inet,
       }
     end
 
@@ -435,7 +437,7 @@ SQL
         when :bigint
           value = string.to_i
         when :boolean
-          value = !!(string =~ /t|true|y|yes|1/)
+          value = !!(string =~ /t|true|y|yes|1/i)
         when :string
           value = string
         when :time
@@ -444,26 +446,65 @@ SQL
           rescue ArgumentError => e
             invalid_format string, e.message
           end
+        when :date
+          begin
+            value = Date.parse string
+          rescue ArgumentError => e
+            invalid_format string, e.message
+          end
+        when :datetime
+          begin
+            value = DateTime.parse string
+          rescue ArgumentError => e
+            invalid_format string, e.message
+          end
+        when :ipaddr
+          begin
+            value = IPAddr.new string
+          rescue ArgumentError => e
+            invalid_format string, e.message
+          end
         when :double
           value = string.to_f
         when :user_id, :post_id, :topic_id, :category_id, :group_id, :badge_id
-          pkey = string.to_i
+          if string.gsub(/[ _]/, '') =~ /^-?\d+$/
-          if pkey != 0
             clazz_name = (/^(.*)_id$/.match(type.to_s)[1].classify.to_sym)
             begin
-              Object.const_get(clazz_name).find(pkey)
+              Object.const_get(clazz_name).find(string.gsub(/[ _]/, '').to_i)
               value = pkey
             rescue ActiveRecord::RecordNotFound
               invalid_format string, "The specified #{clazz_name} was not found"
             end
+          elsif type == :user_id
+            begin
+              object = User.find_by_username_or_email(string)
+              value = object.id
+            rescue ActiveRecord::RecordNotFound
+              invalid_format string, "The user named #{string} was not found"
+            end
+          elsif type == :post_id
+            if string =~ /(\d+)\/(\d+)(\?u=.*)?$/
+              object = Post.with_deleted.find_by(topic_id: $1, post_number: $2)
+              invalid_format string, "The post at topic:#{$1} post_number:#{$2} was not found" unless object
+              value = object.id
+            end
+          elsif type == :topic_id
+            if string =~ /\/t\/[^\/]+\/(\d+)/
+              begin
+                object = Topic.with_deleted.find($1)
+                value = object.id
+              rescue ActiveRecord::RecordNotFound
+                invalid_format string, "The topic with id #{$1} was not found"
+              end
+            end
           else
             invalid_format string
           end
         when :int_list
-          value = string.split(',').map(&:to_i)
+          value = string.split(',').map {|s| s.downcase == '#null' ? nil : s.to_i }
           invalid_format string, "can't be empty" if value.length == 0
         when :string_list
-          value = string.split(',')
+          value = string.split(',').map {|s| s.downcase == '#null' ? nil : s }
           invalid_format string, "can't be empty" if value.length == 0
         else
           raise TypeError.new('unknown parameter type??? should not get here')
@@ -472,7 +513,7 @@ SQL
       value
     end
 
-    def self.create_from_sql(sql)
+    def self.create_from_sql(sql, opts={})
       in_params = false
       ret_params = []
       sql.split("\n").find do |line|
@@ -492,7 +533,14 @@ SQL
             end
             type = type.strip
 
-            ret_params << DataExplorer::Parameter.new(ident, type, default, nullable)
+            begin
+              ret_params << DataExplorer::Parameter.new(ident, type, default, nullable)
+            rescue
+              if opts[:strict]
+                raise
+              end
+            end
+
             false
           elsif line =~ /^\s+$/
             false
@@ -568,6 +616,7 @@ SQL
       [:name, :sql, :description].each do |sym|
         query.send("#{sym}=", hash[sym]) if hash[sym]
       end
+
       query.check_params!
       query.save
 
@@ -640,7 +689,7 @@ SQL
           success: true,
           errors: [],
           duration: (result[:duration_secs].to_f * 1000).round(1),
-          params: result[:params_full],
+          params: query_params,
           columns: cols,
         }
         json[:explain] = result[:explain] if opts[:explain]