API data validation and administration improvements

assets/javascripts/discourse/controllers/admin-wizards-api.js.es6

@@ -1,8 +1,7 @@
 import { ajax } from 'discourse/lib/ajax';
 import { popupAjaxError } from 'discourse/lib/ajax-error';
 import CustomWizardApi from '../models/custom-wizard-api';
-import { default as computed, observes } from 'ember-addons/ember-computed-decorators';
+import { default as computed } from 'ember-addons/ember-computed-decorators';
-import DiscourseURL from 'discourse/lib/url';
 
 export default Ember.Controller.extend({
   queryParams: ['refresh_list'],
@@ -13,10 +12,11 @@ export default Ember.Controller.extend({
   isBasicAuth: Ember.computed.equal('api.authType', 'basic'),
   endpointMethods: ['GET', 'PUT', 'POST', 'PATCH', 'DELETE'],
   showRemove: Ember.computed.not('isNew'),
+  responseIcon: null,
 
-  @computed('saveDisabled', 'api.authType', 'api.authUrl')
+  @computed('saveDisabled', 'api.authType', 'api.authUrl', 'api.clientId', 'api.clientSecret')
-  authDisabled(saveDisabled, authType, authUrl) {
+  authDisabled(saveDisabled, authType, authUrl, clientId, clientSecret) {
-    return saveDisabled || !authType || !authUrl;
+    return saveDisabled || !authType || !authUrl || !clientId || !clientSecret;
   },
 
   @computed('api.name', 'api.authType')
@@ -24,15 +24,6 @@ export default Ember.Controller.extend({
     return !name || !authType;
   },
 
-  @observes('api.title')
-  titleWatcher() {
-    const title = this.get('api.title');
-
-    if (this.get('originalTitle')) {
-      this.set('originalTitle', title);
-    }
-  },
-
   actions: {
     addParam() {
       this.get('api.authParams').pushObject({});
@@ -76,6 +67,7 @@ export default Ember.Controller.extend({
       const name = api.name;
       const authType = api.authType;
       let refreshList = false;
+      let error;
 
       if (!name || !authType) return;
 
@@ -85,7 +77,9 @@ export default Ember.Controller.extend({
 
       if (api.title) data['title'] = api.title;
 
-      if (api.get('isNew') || (api.title !== this.get('originalTitle'))) {
+      const originalTitle = this.get('api.originalTitle');
+      console.log(api, originalTitle);
+      if (api.get('isNew') || (originalTitle && (api.title !== originalTitle))) {
         refreshList = true;
       }
 
@@ -93,29 +87,47 @@ export default Ember.Controller.extend({
         data['new'] = true;
       };
 
+      let requiredParams;
+
       if (authType === 'oauth') {
-        data['auth_url'] = api.authUrl;
+        requiredParams = ['authUrl', 'tokenUrl', 'clientId', 'clientSecret'];
-        data['client_id'] = api.clientId;
-        data['client_secret'] = api.clientSecret;
-
-        let params = api.authParams;
-
-        if (params) {
-          data['auth_params'] = JSON.stringify(params);
-        }
-
-        data['token_url'] = api.tokenUrl;
       } else if (authType === 'basic') {
-        data['username'] = api.username;
+        requiredParams = ['username', 'password'];
-        data['password'] = api.password;
+      }
+
+      for (let rp of requiredParams) {
+        if (!api[rp]) {
+          let key = rp.replace('auth', '');
+          error = `${I18n.t(`admin.wizard.api.auth.${key.underscore()}`)} is required for ${authType}`;
+          break;
+        }
+        data[rp.underscore()] = api[rp];
+      }
+
+      const params = api.authParams;
+      if (params.length) {
+        data['auth_params'] = JSON.stringify(params);
       }
 
       const endpoints = api.endpoints;
-
       if (endpoints.length) {
+        for (let e of endpoints) {
+          if (!e.name) {
+            error = 'Every endpoint must have a name';
+            break;
+          }
+        }
         data['endpoints'] = JSON.stringify(endpoints);
       }
 
+      if (error) {
+        this.set('error', error);
+        setTimeout(() => {
+          this.set('error', '');
+        }, 6000);
+        return;
+      }
+
       this.set('updating', true);
 
       ajax(`/admin/wizards/apis/${name.underscore()}`, {
@@ -130,7 +142,10 @@ export default Ember.Controller.extend({
               });
             } else {
               this.set('api', CustomWizardApi.create(result.api));
+              this.set('responseIcon', 'check');
             }
+          } else {
+            this.set('responseIcon', 'times');
           }
         }).finally(() => this.set('updating', false));
     },
@@ -146,7 +161,9 @@ export default Ember.Controller.extend({
       }).catch(popupAjaxError)
         .then(result => {
           if (result.success) {
-            DiscourseURL.routeTo('/admin/wizards/apis?refresh=true');
+            this.transitionToRoute('adminWizardsApis').then(() => {
+              this.send('refreshModel');
+            });
           }
         }).finally(() => this.set('updating', false));
     }

assets/javascripts/discourse/models/custom-wizard-api.js.es6

@@ -19,6 +19,7 @@ CustomWizardApi.reopenClass({
     api.setProperties({
       name: params.name,
       title: params.title,
+      originalTitle: params.title,
       authType: authorization.auth_type,
       authUrl: authorization.auth_url,
       tokenUrl: authorization.token_url,

assets/javascripts/discourse/templates/admin-wizards-api.hbs

@@ -1,23 +1,11 @@
 <div class="wizard-api-header page">
-  <div class="metadata">
-    <div class="title">
-      <label>{{i18n 'admin.wizard.api.title'}}</label>
-      {{input value=api.title placeholder=(i18n 'admin.wizard.api.title_placeholder')}}
-    </div>
-
-    <div class="name">
-      {{#if api.isNew}}
-        <label>{{i18n 'admin.wizard.api.name'}}</label>
-        {{input value=api.name placeholder=(i18n 'admin.wizard.api.name_placeholder')}}
-      {{else}}
-        {{api.name}}
-      {{/if}}
-    </div>
-  </div>
-
   <div class='buttons'>
     {{#if updating}}
       {{loading-spinner size="small"}}
+    {{else}}
+      {{#if responseIcon}}
+        {{d-icon responseIcon}}
+      {{/if}}
     {{/if}}
 
     {{d-button label="admin.wizard.api.save" action="save" class="btn-primary" disabled=saveDisabled}}
@@ -25,15 +13,41 @@
     {{#if showRemove}}
       {{d-button action="remove" label="admin.wizard.api.remove"}}
     {{/if}}
+
+    {{#if error}}
+      <div class="error">
+        {{error}}
+      </div>
+    {{/if}}
+  </div>
+
+  <div class="wizard-header">
+    {{#if api.isNew}}
+      {{i18n 'admin.wizard.api.new'}}
+    {{else}}
+      {{api.title}}
+    {{/if}}
+  </div>
+
+  <div class="metadata">
+    <div class="title">
+      <label>{{i18n 'admin.wizard.api.title'}}</label>
+      {{input value=api.title placeholder=(i18n 'admin.wizard.api.title_placeholder')}}
+    </div>
+
+    <div class="name">
+      <label>{{i18n 'admin.wizard.api.name'}}</label>
+      {{#if api.isNew}}
+        {{input value=api.name placeholder=(i18n 'admin.wizard.api.name_placeholder')}}
+      {{else}}
+        {{api.name}}
+      {{/if}}
+    </div>
   </div>
 </div>
 
 <div class="wizard-api-header">
-  <div class="wizard-header">
+  <div class="buttons">
-    {{i18n 'admin.wizard.api.auth.label'}}
-  </div>
-
-  <div class="actions">
     {{#if isOauth}}
       {{d-button label="admin.wizard.api.auth.btn"
                  action="authorize"
@@ -41,6 +55,10 @@
                  class="btn-primary"}}
     {{/if}}
   </div>
+
+  <div class="wizard-header">
+    {{i18n 'admin.wizard.api.auth.label'}}
+  </div>
 </div>
 
 <div class="wizard-api-authentication">
@@ -198,9 +216,20 @@
         {{#each api.endpoints as |endpoint|}}
           <li>
             <div class="endpoint">
-              {{combo-box content=endpointMethods value=endpoint.method none="admin.wizard.api.endpoint.method"}}
+              <div class="endpoint-">
-              {{input value=endpoint.url placeholder=(i18n 'admin.wizard.api.endpoint.url') class='endpoint-url'}}
+                {{input value=endpoint.name
-              {{d-button action='removeEndpoint' actionParam=endpoint icon='times' class='remove-endpoint'}}
+                        placeholder=(i18n 'admin.wizard.api.endpoint.name')}}
+                {{combo-box content=endpointMethods
+                            value=endpoint.method
+                            none="admin.wizard.api.endpoint.method"}}
+                {{input value=endpoint.url
+                        placeholder=(i18n 'admin.wizard.api.endpoint.url')
+                        class='endpoint-url'}}
+                {{d-button action='removeEndpoint'
+                           actionParam=endpoint
+                           icon='times'
+                           class='remove-endpoint'}}
+              </div>
             </div>
           </li>
         {{/each}}

assets/stylesheets/wizard_custom_admin.scss

@@ -316,15 +316,34 @@
   .metadata .title input {
     width: 400px;
   }
+
+  .buttons {
+    text-align: right;
+    vertical-align: middle;
+
+    > .d-icon, > .spinner {
+      margin-right: 7px;
+    }
+
+    .error {
+      margin-top: 10px;
+      color: $danger;
+    }
+  }
 }
 
 .wizard-api-header {
-  display: flex;
-  justify-content: space-between;
-
   &.page {
     margin-bottom: 20px;
   }
+
+  .buttons {
+    float: right;
+  }
+
+  .wizard-header {
+    overflow: hidden;
+  }
 }
 
 .wizard-api-authentication {
@@ -388,12 +407,18 @@
 
     .combo-box {
       width: 200px;
-      margin-right: 20px;
+      margin-right: 10px;
+      margin-top: -2px;
+      width: 150px;
+    }
+
+    input {
+      margin: 0;
+      margin-right: 10px;
     }
 
     .endpoint-url {
-      margin: 0;
+      width: 300px;
-      width: 450px;
     }
 
     .remove-endpoint {

config/locales/client.en.yml

@@ -170,6 +170,7 @@ en:
           endpoint:
             label: "Endpoints"
             add: "Add endpoint"
+            name: "Endpoint name"
             method: "Select a method"
             url: "Enter a url"
 

controllers/api.rb

@@ -46,7 +46,7 @@ class CustomWizard::ApiController < ::ApplicationController
 
     render json: success_json.merge(
       api: CustomWizard::ApiSerializer.new(
-        CustomWizard::Api.new(api_params[:name]),
+        CustomWizard::Api.get(api_params[:name]),
         root: false
       )
     )

lib/api/api.rb

@@ -6,7 +6,9 @@ class CustomWizard::Api
 
   def initialize(name, data={})
     @name = name
-    @title = data['title']
+    data.each do |k, v|
+      self.send "#{k}=", v if self.respond_to?(k)
+    end
   end
 
   def self.set(name, data)

lib/api/authorization.rb

@@ -3,8 +3,8 @@ require 'excon'
 class CustomWizard::Api::Authorization
   include ActiveModel::SerializerSupport
 
-  attr_accessor :authorized,
+  attr_accessor :api_name,
-                :name,
+                :authorized,
                 :auth_type,
                 :auth_url,
                 :token_url,
@@ -19,15 +19,11 @@ class CustomWizard::Api::Authorization
                 :username,
                 :password
 
-  def initialize(name, data, opts = {})
+  def initialize(api_name, data={})
-    unless opts[:data_only]
+    @api_name = api_name
-      @name = name
-    end
 
-    if data = data.is_a?(String) ? ::JSON.parse(data) : data
+    data.each do |k, v|
-      data.each do |k, v|
+      self.send "#{k}=", v if self.respond_to?(k)
-        self.send "#{k}=", v if self.respond_to?(k)
-      end
     end
   end
 
@@ -35,25 +31,32 @@ class CustomWizard::Api::Authorization
     @authorized ||= @access_token && @token_expires_at.to_datetime > Time.now
   end
 
-  def self.set(name, data = {})
+  def self.set(api_name, new_data = {})
-    record = self.get(name, data_only: true)
+    data = self.get(api_name, data_only: true) || {}
 
-    data.each do |k, v|
+    new_data.each do |k, v|
-      record.send "#{k}=", v if record.respond_to?(k)
+      data[k.to_sym] = v
     end
 
-    PluginStore.set("custom_wizard_api_#{name}", 'authorization', record.as_json)
+    PluginStore.set("custom_wizard_api_#{api_name}", 'authorization', data)
 
-    self.get(name)
+    self.get(api_name)
   end
 
-  def self.get(name, opts = {})
+  def self.get(api_name, opts = {})
-    data = PluginStore.get("custom_wizard_api_#{name}", 'authorization')
+    if data = PluginStore.get("custom_wizard_api_#{api_name}", 'authorization')
-    self.new(name, data, opts)
+      if opts[:data_only]
+        data
+      else
+        self.new(api_name, data)
+      end
+    else
+      nil
+    end
   end
 
-  def self.remove(name)
+  def self.remove(api_name)
-    PluginStore.remove("custom_wizard_api_#{name}", "authorization")
+    PluginStore.remove("custom_wizard_api_#{api_name}", "authorization")
   end
 
   def self.get_header_authorization_string(name)

lib/api/endpoint.rb

@@ -2,49 +2,59 @@ class CustomWizard::Api::Endpoint
   include ActiveModel::SerializerSupport
 
   attr_accessor :id,
+                :name,
+                :api_name,
                 :method,
                 :url
 
-  def initialize(name, params)
+  def initialize(api_name, data={})
-    @name = name
+    @api_name = api_name
-    if data = params.is_a?(String) ? ::JSON.parse(params) : params
-      data.each do |k, v|
-        self.send "#{k}=", v if self.respond_to?(k)
-      end
-    end
-  end
-
-  def self.set(name, data)
-    model = data[:endpoint_id] ? self.get(name, data[:endpoint_id]) : {}
-    endpoint_id = model[:endpoint_id] || SecureRandom.hex(8)
 
     data.each do |k, v|
-      model.send "#{k}=", v if model.respond_to?(k)
+      self.send "#{k}=", v if self.respond_to?(k)
+    end
+  end
+
+  def self.set(api_name, new_data)
+    data = new_data[:endpoint_id] ? self.get(api_name, new_data[:endpoint_id], data_only: true) : {}
+    endpoint_id = new_data[:endpoint_id] || SecureRandom.hex(3)
+
+    new_data.each do |k, v|
+      data[k.to_sym] = v
     end
 
-    PluginStore.set("custom_wizard_api_#{name}", "endpoint_#{endpoint_id}", model.as_json)
+    PluginStore.set("custom_wizard_api_#{api_name}", "endpoint_#{endpoint_id}", data)
 
-    self.get(name)
+    self.get(api_name, endpoint_id)
   end
 
-  def self.get(name, endpoint_id)
+  def self.get(api_name, endpoint_id, opts={})
     return nil if !endpoint_id
-    data = PluginStore.get("custom_wizard_api_#{name}", "endpoint_#{endpoint_id}")
+
-    data[:id] = endpoint_id
+    if data = PluginStore.get("custom_wizard_api_#{api_name}", "endpoint_#{endpoint_id}")
-    self.new(name, data)
+      data[:id] = endpoint_id
+
+      if opts[:data_only]
+        data
+      else
+        self.new(api_name, data)
+      end
+    else
+      nil
+    end
   end
 
-  def self.remove(name)
+  def self.remove(api_name)
-    PluginStoreRow.where("plugin_name = 'custom_wizard_api_#{name}' AND key LIKE 'endpoint_%'").destroy_all
+    PluginStoreRow.where("plugin_name = 'custom_wizard_api_#{api_name}' AND key LIKE 'endpoint_%'").destroy_all
   end
 
   def self.list
     PluginStoreRow.where("plugin_name LIKE 'custom_wizard_api_%' AND key LIKE 'endpoint_%'")
       .map do |record|
-        name = record['plugin_name'].sub("custom_wizard_api_", "")
+        api_name = record['plugin_name'].sub("custom_wizard_api_", "")
         data = ::JSON.parse(record['value'])
         data[:id] = record['key'].split('_').last
-        self.new(name, data)
+        self.new(api_name, data)
       end
   end
 end

serializers/api/endpoint_serializer.rb

@@ -1,5 +1,10 @@
 class CustomWizard::Api::EndpointSerializer < ApplicationSerializer
   attributes :id,
-             :type,
+             :name,
+             :method,
              :url
+
+  def method
+    object.send('method')
+  end
 end