Merge pull request #696 from coyote-team/661-creating-two-resources-w…

…ith-identical-uris-triggers-http-500

661 creating two resources with identical uris triggers http 500

app/controllers/concerns/form_errors.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+# Used to rescue form errors that would otherwise raise an exception and display the standard rails 500 error page
+# Needed to rescue from resources controller create action for identical/invalid source URIs and Canonical IDs
+# Can add additional rescue_from statements as needed for other controllers/forms
+module FormErrors
+  extend ActiveSupport::Concern
+
+  included do
+  rescue_from ActiveRecord::RecordNotUnique, with: :not_unique_response
+  rescue_from URI::InvalidURIError, with: :invalid_uri_response
+  end
+
+  private 
+
+  def not_unique_response(e)
+    response = check_unique_error(e)
+    resource.errors.add(:base, response)
+    logger.warn "Unable to create resource due to '#{e.message}'"
+    render :new, status: :unprocessable_entity
+  end
+
+  def check_unique_error(e)
+    if e.message.include?("canonical_id")
+      "The Canonical ID is already in use for this organization."
+    elsif e.message.include?("source_uri")
+      "The Source URI is already in use for this organization."
+    end
+  end
+
+  def invalid_uri_response(e)
+    response = "The Source URI is invalid."
+    resource.errors.add(:base, response)
+    logger.warn "Unable to create resource due to '#{e.message}'"
+    render :new, status: :unprocessable_entity
+  end
+end

app/controllers/concerns/permitted_parameters.rb

@@ -62,10 +62,11 @@ module PermittedParameters
       union_host_uris
       union_resource_groups
       uploaded_resource
+      organization_id
     ] + [{
       host_uris:          [],
       representations:    REPRESENTATION_PARAMS,
-      resource_group_ids: [],
+      resource_group_ids: []
     }]).freeze
 
   private
@@ -76,8 +77,9 @@ def clean_representation_params(representation_params)
     end
   end
 
-  def clean_resource_params(resource_params)
+  def clean_resource_params(resource_params, org_id)
     resource_params = ActionController::Parameters.new(resource_params) unless resource_params.respond_to?(:permit)
+    resource_params[:organization_id] = org_id
     resource_params.permit(*RESOURCE_PARAMS).tap do |params|
       representations = params.delete(:representations)
       if representations.present?
@@ -101,7 +103,7 @@ def representation_params
     clean_representation_params(params.require(:representation))
   end
 
-  def resource_params
-    clean_resource_params(params.require(:resource))
+  def resource_params(org_id)
+    clean_resource_params(params.require(:resource), org_id)
   end
 end

app/controllers/resources_controller.rb

@@ -5,6 +5,7 @@
 # @see ResourcePolicy
 class ResourcesController < ApplicationController
   include PermittedParameters
+  include FormErrors
 
   before_action :check_for_canonical_id, only: %i[show]
   before_action :set_resource, only: %i[show edit update destroy]
@@ -14,9 +15,9 @@ class ResourcesController < ApplicationController
   helper_method :resource, :record_filter, :filter_params, :resource_groups
 
   def create
-    self.resource = current_organization.resources.new
-
-    if resource.update(resource_params)
+    self.resource = current_organization.resources.new(resource_params(current_organization.id))
+    
+    if resource.save
       logger.info "Created #{resource}"
       redirect_to resource, notice: "The resource has been created"
     else

app/views/resources/_form.html.slim

@@ -1,5 +1,7 @@
 = simple_form_for target, validate: true, html: { multipart: true } do |f|
-  = f.error_notification
+  - if f.object.errors.any?
+    - f.object.errors.full_messages.each do |message|
+      .error_notification= message
 
   = f.input :name, label: 'Caption'
   = f.input :source_uri, label: 'Source URI', hint: 'Identifies the canonical location of the resource', required: true

spec/features/resource_adding_and_changing_spec.rb

@@ -9,10 +9,12 @@
     attributes_for(:resource, canonical_id: "abc123").symbolize_keys
   end
 
-  it "succeeds" do
+  before(:each) do
     click_first_link "Resources"
     click_first_link("Add Resource")
+  end
 
+  it "succeeds" do
     within(".form-field.resource_resource_groups") do
       user_organization.resource_groups.default.each do |other_group|
         uncheck(other_group.name)
@@ -37,4 +39,49 @@
     expect(page).to have_current_path(resource_path(resource, organization_id: user_organization), ignore_query: true)
     expect(page).to have_content(resource.name)
   end
+
+  context "when invalid resource attributes are submitted" do
+    let!(:resource) { create(:resource, organization_id: user_organization.id, canonical_id: '4') }
+    let(:resource_uri) { resource.source_uri }
+    let(:resource_canonical_id) { resource.canonical_id }
+
+    describe 'source URI is invalid' do
+      it "should display an error message and re-render the form" do
+        fill_in "Caption", with: resource_attributes[:name]
+        fill_in "Canonical ID", with: resource_attributes[:canonical_id]
+        fill_in "Source URI", with: "Hello World!"
+        fill_in "Host URIs", with: "http://example.com/abc\nhttp://example.com/xyz"
+        click_button("Create Resource")
+
+        expect(page).to have_current_path(resources_path(organization_id: user_organization))
+        expect(page).to have_content("The Source URI is invalid.")
+      end
+    end
+
+    describe 'source uri is not unique' do
+      it "should display an error message and re-render the form" do
+        fill_in "Caption", with: resource_attributes[:name]
+        fill_in "Canonical ID", with: resource_attributes[:canonical_id]
+        fill_in "Source URI", with: resource_uri
+        fill_in "Host URIs", with: "http://example.com/abc\nhttp://example.com/xyz"
+        click_button("Create Resource")
+
+        expect(page).to have_current_path(resources_path(organization_id: user_organization))
+        expect(page).to have_content("The Source URI is already in use for this organization.")
+      end
+    end
+
+    describe 'canonical uri is not unique' do
+      it "should display an error message and re-render the form" do
+        fill_in "Caption", with: resource_attributes[:name]
+        fill_in "Canonical ID", with: resource_canonical_id
+        fill_in "Source URI", with: resource_attributes[:source_uri]
+        fill_in "Host URIs", with: "http://example.com/abc\nhttp://example.com/xyz"
+        click_button("Create Resource")
+
+        expect(page).to have_current_path(resources_path(organization_id: user_organization))
+        expect(page).to have_content("The Canonical ID is already in use for this organization.")
+      end
+    end
+  end
 end