FIX: better handling of edge cases (#187)

- Require query name is present
- Ensure all routes are treated by default as .json, so errors flow correctly
- Remove superflous save/cancel controls from group settings
- Remove group control when item is destroyed
- Disable editing of query when it is deleted

Co-authored-by: Osama Sayegh <asooomaasoooma90@gmail.com>

app/models/data_explorer/query.rb

@@ -6,6 +6,7 @@ module DataExplorer
     has_many :query_groups
     has_many :groups, through: :query_groups
     belongs_to :user
+    validates :name, presence: true
 
     scope :for_group, ->(group) do
       where(hidden: false)

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

@@ -88,36 +88,21 @@
               </div>
             {{/if}}
 
-            <div class="pull-left">
+            {{#if (not selectedItem.destroyed)}}
-              <div class="groups">
+              <div class="pull-left">
-                <span class="label">{{i18n "explorer.allow_groups"}}</span>
+                <div class="groups">
-                <span>
+                  <span class="label">{{i18n "explorer.allow_groups"}}</span>
-                  {{multi-select
+                  <span>
-                    value=selectedItem.group_ids
+                    {{multi-select
-                    content=groupOptions
+                      value=selectedItem.group_ids
-                    allowAny=false
+                      content=groupOptions
-                    onSelect=(action (mut selectedItem.group_ids))
+                      allowAny=false
-                  }}
+                      onSelect=(action (mut selectedItem.group_ids))
-                </span>
+                    }}
-
+                  </span>
-                {{#if runDisabled}}
+                </div>
-                  {{#unless editing}}
-                    <span class="setting-controls">
-                      {{d-button
-                        class="ok"
-                        action=(action "save")
-                        icon="check"
-                      }}
-                      {{d-button
-                        class="cancel"
-                        action=(action "discard")
-                        icon="times"
-                      }}
-                    </span>
-                  {{/unless}}
-                {{/if}}
               </div>
-            </div>
+            {{/if}}
 
             <div class="clear"></div>
 
@@ -126,7 +111,7 @@
               <div class="query-editor {{if hideSchema "no-schema"}}">
                 <div class="panels-flex">
                   <div class="editor-panel">
-                    {{ace-editor content=selectedItem.sql mode="sql"}}
+                    {{ace-editor content=selectedItem.sql mode="sql" disabled=selectedItem.destroyed}}
                   </div>
 
                   <div class="right-panel">

plugin.rb

@@ -886,14 +886,16 @@ SQL
 
   DataExplorer::Engine.routes.draw do
     root to: "query#index"
-    get 'schema' => "query#schema"
     get 'queries' => "query#index"
-    get 'groups' => "query#groups"
+    scope "/", defaults: { format: :json } do
-    post 'queries' => "query#create"
+      get 'schema' => "query#schema"
-    get 'queries/:id' => "query#show"
+      get 'groups' => "query#groups"
-    put 'queries/:id' => "query#update"
+      post 'queries' => "query#create"
-    delete 'queries/:id' => "query#destroy"
+      get 'queries/:id' => "query#show"
-    post 'queries/:id/run' => "query#run", constraints: { format: /(json|csv)/ }
+      put 'queries/:id' => "query#update"
+      delete 'queries/:id' => "query#destroy"
+      post 'queries/:id/run' => "query#run", constraints: { format: /(json|csv)/ }
+    end
   end
 
   Discourse::Application.routes.append do

spec/requests/query_controller_spec.rb

@@ -107,6 +107,19 @@ describe DataExplorer::QueryController do
 
         expect(response.status).to eq(200)
       end
+
+      it "returns a proper json error for invalid updates" do
+
+        query = DataExplorer::Query.find(-4)
+        put "/admin/plugins/explorer/queries/#{query.id}", params: {
+          "query" => {
+            "name" => "",
+          },
+          "id" => query.id }
+
+        expect(response.status).to eq(422)
+        expect(response.parsed_body["errors"]).to eq(["Name can't be blank"])
+      end
     end
 
     describe "#run" do
@@ -304,8 +317,6 @@ describe DataExplorer::QueryController do
             DataExplorer.send(:remove_const, "QUERY_RESULT_MAX_LIMIT")
             DataExplorer.const_set("QUERY_RESULT_MAX_LIMIT", 2)
 
-            ids = Post.order(:id).pluck(:id)
-
             query = make_query <<~SQL
             SELECT id FROM posts
             SQL