Updated new job composer project validation - Single Project Model (O…

…SC#2990)

* Updated new job composer project validation

* Improved project validation logic based on Jeff patch

* Fixed unnecessary parameter assignment

* Improvements to unit tests

apps/dashboard/app/controllers/projects_controller.rb

@@ -4,9 +4,10 @@
 class ProjectsController < ApplicationController
   # GET /projects/:id
   def show
-    @project = Project.find(show_project_params[:id])
+    project_id = show_project_params[:id]
+    @project = Project.find(project_id)
     if @project.nil?
-      redirect_to(projects_path, alert: "Cannot find project #{show_project_params[:id]}")
+      redirect_to(projects_path, alert: I18n.t('dashboard.jobs_project_not_found', project_id: project_id))
     else
       @scripts = Script.all(@project.directory)
     end
@@ -22,58 +23,68 @@ def index
   def new
     @templates = new_project_params[:template] == 'true' ? templates : []
 
-    if name_or_icon_nil?
-      @project = Project.new
-    else
-      returned_params = { name: new_project_params[:name], icon: new_project_params[:icon], directory: new_project_params[:directory] }
-      @project = Project.new(returned_params)
-    end
+    @project = Project.new
   end
 
   # GET /projects/:id/edit
   def edit
-    @project = Project.find(show_project_params[:id])
+    project_id = show_project_params[:id]
+    @project = Project.find(project_id)
 
     return unless @project.nil?
 
-    redirect_to(projects_path, alert: "Cannot find project #{show_project_params[:id]}")
+    redirect_to(projects_path, alert: I18n.t('dashboard.jobs_project_not_found', project_id: project_id))
   end
 
   # PATCH /projects/:id
   def update
-    @project = Project.find(show_project_params[:id])
+    project_id = show_project_params[:id]
+    @project = Project.find(project_id)
 
     if @project.nil?
-      redirect_to(projects_path, alert: "Cannot find project #{show_project_params[:id]}")
-    elsif @project.valid? && @project.update(project_params)
+      redirect_to(projects_path, alert: I18n.t('dashboard.jobs_project_not_found', project_id: project_id))
+      return
+    end
+
+    if @project.update(project_params)
       redirect_to projects_path, notice: I18n.t('dashboard.jobs_project_manifest_updated')
     else
-      flash[:alert] = @project.errors[:name].last || @project.errors[:icon].last
-      redirect_to edit_project_path
+      message = @project.errors[:save].empty? ? I18n.t('dashboard.jobs_project_validation_error') : I18n.t('dashboard.jobs_project_generic_error', error: @project.collect_errors)
+      flash.now[:alert] = message
+      render :edit
     end
   end
 
   # POST /projects
   def create
-    Rails.logger.debug("Project params are: #{project_params}")
-    id = Project.next_id
-    opts = project_params.merge(id: id).to_h.with_indifferent_access
-    @project = Project.new(opts)
+    @project = Project.new(project_params)
 
-    if @project.valid? && @project.save(project_params)
+    if @project.save
       redirect_to projects_path, notice: I18n.t('dashboard.jobs_project_created')
     else
-      # TODO: loop through all errors and show them instead of this
-      flash[:alert] = @project.errors[:name].last || @project.errors[:icon].last
-      redirect_to new_project_path(name: project_params[:name], directory: project_params[:directory], icon: project_params[:icon])
+      message = @project.errors[:save].empty? ? I18n.t('dashboard.jobs_project_validation_error') : I18n.t('dashboard.jobs_project_generic_error', error: @project.collect_errors)
+      flash.now[:alert] = message
+      render :new
     end
   end
 
+
+
   # DELETE /projects/:id
   def destroy
-    @project = Project.find(params[:id])
+    project_id = params[:id]
+    @project = Project.find(project_id)
 
-    redirect_to projects_path, notice: I18n.t('dashboard.jobs_project_deleted') if @project.destroy!
+    if @project.nil?
+      redirect_to projects_path, notice: I18n.t('dashboard.jobs_project_not_found', project_id: project_id)
+      return
+    end
+
+    if @project.destroy!
+      redirect_to projects_path, notice: I18n.t('dashboard.jobs_project_deleted')
+    else
+      redirect_to projects_path, notice: I18n.t('dashboard.jobs_project_generic_error', error: @project.collect_errors)
+    end
   end
 
   private
@@ -95,14 +106,10 @@ def templates
     end
   end
 
-  def name_or_icon_nil?
-    new_project_params[:name].nil? || new_project_params[:icon].nil?
-  end
-
   def project_params
     params
       .require(:project)
-      .permit(:name, :directory, :description, :icon, :id)
+      .permit(:name, :directory, :description, :icon, :id, :template)
   end
 
   def show_project_params

apps/dashboard/app/models/project.rb

@@ -67,46 +67,61 @@ def templates
 
   delegate :icon, :name, :description, to: :manifest
 
+  validates :name, presence: { message: :required }, on: [:create, :update]
+  validates :id, :directory, :icon, presence: { message: :required }, on: [:update]
+  validates :icon, format: { with: %r{\Afa[bsrl]://[\w-]+\z}, allow_blank: true, message: :format }, on: [:create, :update]
+  validate :project_directory_invalid, on: [:create, :update]
+  validate :project_directory_exist, on: [:create]
+
   # the template you created this project from
   attr_accessor :template
 
   def initialize(attributes = {})
-    if attributes.empty?
-      @manifest = Manifest.new({})
-    else
-      @id = attributes.delete(:id)
-      @directory = attributes.delete(:directory)
-      @directory = Project.dataroot.join(id.to_s).to_s if @directory.blank?
+    @id = attributes.delete(:id)
+    @directory = attributes.delete(:directory)
+    @directory = File.expand_path(@directory) unless @directory.blank?
 
-      @manifest = Manifest.new(attributes).merge(Manifest.load(manifest_path))
-    end
+    @manifest = Manifest.new(attributes)
+    @manifest = @manifest.merge(Manifest.load(manifest_path)) unless new_record?
   end
 
-  def save(attributes)
-    if id.nil? && attributes.fetch(:id, nil).nil?
-      message = 'Project ID must be set when saving.'
-      errors.add(:save, message)
-      Rails.logger.warn(message)
-      return false
-    end
+  def new_record?
+    return true if !id
+    return true if !directory
+
+    id && directory && !File.exist?(manifest_path)
+  end
+
+  def save(attributes={})
+    @id = attributes.delete(:id) if attributes.key?(:id)
+    @directory = attributes.delete(:directory) if attributes.key?(:directory)
+    @directory = File.expand_path(@directory) unless @directory.blank?
+    @manifest = manifest.merge(attributes)
+
+    return false unless valid?(:create)
+
+    # SET DEFAULTS
+    @id = Project.next_id if id.blank?
+    @directory = Project.dataroot.join(id.to_s).to_s if directory.blank?
+    @manifest = manifest.merge({ icon: 'fas://cog' }) if icon.blank?
 
     make_dir
-    update(attributes)
+    store_manifest
   end
 
   def update(attributes)
-    if valid_icon?(attributes)
-      manifest = Manifest.load(manifest_path)
-                         .merge(attributes)
-
-      if manifest.valid? && manifest.save(manifest_path) && add_to_lookup
-        true
-      else
-        errors.add(:update, "Cannot save manifest to #{manifest_path}")
-        false
-      end
+    @manifest = manifest.merge(attributes)
+
+    return false unless valid?(:update)
+
+    store_manifest
+  end
+
+  def store_manifest
+    if manifest.valid? && manifest.save(manifest_path) && add_to_lookup
+      true
     else
-      errors.add(:update, 'Invalid entry')
+      errors.add(:save, I18n.t('dashboard.jobs_project_save_error', path: manifest_path))
       false
     end
   end
@@ -136,7 +151,7 @@ def icon_class
 
   def destroy!
     remove_from_lookup
-    FileUtils.remove_dir(project_dataroot, force = true)
+    FileUtils.remove_dir(configuration_directory, true)
   end
 
   def configuration_directory
@@ -155,28 +170,32 @@ def manifest_path
     File.join(configuration_directory, 'manifest.yml')
   end
 
+  def collect_errors
+    errors.map do |field_error|
+      field_error.message()
+    end.join(', ')
+  end
+
   private
 
   attr_reader :manifest
 
-  def valid_icon?(attributes)
-    icon_pattern = %r{\Afa[bsrl]://[\w-]+\z}
-
-    if !attributes[:icon].nil? && !attributes[:icon].match?(icon_pattern)
-      errors.add(:icon, :invalid_format, message: 'Icon format invalid or missing')
-      false
-    elsif attributes[:icon].nil?
-      attributes[:icon] = 'fas://cog'
-      true
-    else
-      true
-    end
-  end
-
   def make_dir
     project_dataroot.mkpath         unless project_dataroot.exist?
     configuration_directory.mkpath  unless configuration_directory.exist?
   rescue StandardError => e
     errors.add(:make_directory, "Failed to make directory: #{e.message}")
   end
+
+  def project_directory_exist
+    if !directory.blank? && Project.lookup_table.map { |_id, directory| directory }.map(&:to_s).include?(directory.to_s)
+      errors.add(:directory, :used)
+    end
+  end
+
+  def project_directory_invalid
+    if !directory.blank? && Project.dataroot.to_s == directory
+      errors.add(:directory, :invalid)
+    end
+  end
 end

apps/dashboard/app/views/projects/_form.html.erb

@@ -1,17 +1,15 @@
-<%= bootstrap_form_for @project do |form| %>
-
 <div class='card'>
 <h5 class='card-header'>Project Details</h5>
   <div class='card-group'>
       <div class='card'>
       <div class='card-body'>
         <div class="col">
           <div class="field">
-            <%= form.text_field :name, placeholder: "Project name", 
+            <%= form.text_field :name, placeholder: "Project name",
                     help: I18n.t('dashboard.jobs_project_name_validation') %>
           </div>
           <div class="field">
-            <%= form.text_field :directory, readonly: action_name == "edit",
+            <%= form.text_field :directory, readonly: action_name != "new" && action_name != "create",
                     help: I18n.t('dashboard.jobs_project_directory_help') %>
           </div>
           <div class="field">
@@ -58,6 +56,5 @@
   <%= link_to 'Back', projects_path, class: 'btn btn-default', 
   title: 'Return to projects page' %>
 <p>
-<% end %>
 </div>
 

apps/dashboard/app/views/projects/edit.html.erb

@@ -5,5 +5,5 @@
 </div>
 
 <%= bootstrap_form_for(@project, url: project_path, method: :patch) do |form| %>
-  <%= render partial: "form" %>
+  <%= render partial: "form", locals: { form: form }%>
 <% end %>

apps/dashboard/app/views/projects/index.html.erb

@@ -31,8 +31,7 @@
       <div class="mt-5 justify-content-center text-center project-icon">
         <%= link_to(new_project_path,
                       title: I18n.t('dashboard.jobs_create_blank_project'),
-                      class: 'text-dark btn btn-link',
-                      method: 'get') do
+                      class: 'text-dark btn btn-link') do
         %>
           <%= icon_tag(URI.parse("fas://plus")) %><br>
           <%= I18n.t('dashboard.jobs_create_blank_project') %>
@@ -43,8 +42,7 @@
       <div class="mt-5 justify-content-center text-center project-icon">
         <%= link_to(new_project_path({template: true}),
                       title: I18n.t('dashboard.jobs_create_template_project'),
-                      class: 'text-dark btn btn-link',
-                      method: 'get') do
+                      class: 'text-dark btn btn-link') do
         %>
           <%= icon_tag(URI.parse("fas://plus")) %><br>
           <%= I18n.t('dashboard.jobs_create_template_project') %>

apps/dashboard/app/views/projects/new.html.erb

@@ -6,4 +6,6 @@
 
 <br>
 
-<%= render 'form' %>
+<%= bootstrap_form_for(@project, url: projects_path) do |form| %>
+  <%= render partial: "form", locals: { form: form }%>
+<% end %>

apps/dashboard/config/locales/en.yml

@@ -30,6 +30,14 @@
 # available at https://guides.rubyonrails.org/i18n.html.
 
 en:
+  activemodel:
+    errors:
+      messages:
+        required: "%{attribute} is required"
+        format: "%{attribute} format invalid or missing"
+        invalid: "%{attribute} is invalid"
+        used: "%{attribute} is already used"
+
   dashboard:
     # Site specific translations
     batch_connect_sessions_status_bad: "Your session has entered a bad state. Feel free to contact support for
@@ -216,12 +224,17 @@ en:
     jobs_project_created: "Project successfully created!"
     jobs_project_deleted: "Project successfully deleted!"
     jobs_project_delete_project_confirmation: "Delete all contents of project directory?"
+    jobs_project_not_found: "Cannot find project %{project_id}"
     jobs_project_manifest_updated: "Project manifest updated!"
     jobs_project_name_validation: "Project name may only contain letters, digits, dashes, underscores, and spaces"
     jobs_project_directory_help: "Override the default location for this project"
     jobs_create_blank_project: "Create a new project"
     jobs_create_template_project: "Create a new project from a template"
 
+    jobs_project_validation_error: "Invalid Request. Please review the errors below"
+    jobs_project_save_error: "Cannot save manifest to %{path}"
+    jobs_project_generic_error: "There was an error processing your request: %{error}"
+
     jobs_scripts_created: "Script successfully created!"
     jobs_scripts_updated: "Script manifest updated!"
     jobs_scripts_not_found: "Cannot find script %{script_id}"