Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grid digitizer - Error 500 when identifier is matched with a containerized specimen #1601

Closed
tmcelrath opened this issue Jul 2, 2020 · 8 comments
Labels
bug An existing function is broken.

Comments

@tmcelrath
Copy link

Describe the bug

Grid digitizer fails when one of the specimens that is matched is already a containerized specimen.

To Reproduce
Steps to reproduce the behavior:

  1. When I upload an image to grid digitizer.
  2. Then I add catalog numbers.
  3. And those specimens are matched to existing catalog numbers.
  4. And one of those specimens is containerized.
  5. The upload fails with a confusing "Error 500" code.

Error code(s):
image

Expected behavior
Error code should at minimum be more clear, and if possible, depictions should be added to all collection objects within the container.

Screenshots
Failed upload examples:
image

Why it fails:
image

Environment (please identify where you experience this bug:
*** One of ***

  • Production
  • Chrome
@tmcelrath tmcelrath added the bug An existing function is broken. label Jul 2, 2020
@LocoDelAssembly
Copy link
Contributor

Probably related to "sled_images update (ActiveModel::UnknownAttributeError) "unknown attribute 'total' for Container::Slide." error.

The failure is at https://github.com/SpeciesFileGroup/taxonworks/blob/d733e6a56/app/models/sled_image.rb#L236

Looking above it removes some params when the collection object already exists but not all of them (like total).

If I get it correctly the task sends these attributes:

  • total (always "1"?)
  • collecting_event_id (form-supplied, only if set)
  • preparation_type_id (form-supplied, only if set)
  • repository_id (form-supplied, only if set)
  • identifiers_attributes (form-supplied, param excluded at backend if CO exists)
  • notes_attributes (form-supplied, param excluded at backend if CO exists)
  • tags_attributes (form-supplied, param excluded at backend if CO exists)
  • data_attributes_attributes (always "[]"? Would it kill existing data attributes when updating?)
  • taxon_determinations_attributes (form-supplied, param excluded at backend if CO exists)

Besides adding p.delete :total, would be correct to do p.delete :data_attributes_attributes @mjy @jlpereira?

@jlpereira
Copy link
Member

data_attributes_attributes is never assigned in the interface, so i think it should be ok

@tmcelrath
Copy link
Author

@jlpereira @LocoDelAssembly Just got a 500 server error again uploading to containerized specimens. Need to reopen.

@LocoDelAssembly
Copy link
Contributor

@tmcelrath is this possible to reproduce? Can you provide me exact steps and sled_image_id? Trying to reproduce locally.

Fix above was required but ended up fixing a different problem... Problem is that the code is not checking whether the existing identifier is actually identifying a collection object. Current problem is that it is trying to set depository for Container::Slide, but would like to test more before releasing to production.

@tmcelrath
Copy link
Author

Yes. In INHS Insect Collection: sled_image_id=4095 - add this image, use down-across, select INHS Insect Collection namespace, starting identifier: 457900 (end should be 457912 when you deselect blank spaces). Two columns, eight rows. Repository = INHS, Preparation = Slide, Add a Tag of "INHS Slide Digitization". Hit create. Get error.

@tmcelrath
Copy link
Author

LocoDelAssembly added a commit that referenced this issue Jul 13, 2020
@LocoDelAssembly
Copy link
Contributor

I added code that handles identifiers assigned to Containers (and any other non-CollectionObject (sub)class) to be ignored so instead of crashing an error is reported when a CollectionObject with duplicated Identifier is detected. In @tmcelrath example, the offending catalog number of INHS Insect collection namespace is 457906. I also tried modifying the code to attempt to provide desired behavior:

diff --git a/app/models/sled_image.rb b/app/models/sled_image.rb
index 4758bfd05..baf01f0f2 100644
--- a/app/models/sled_image.rb
+++ b/app/models/sled_image.rb
@@ -226,7 +226,7 @@ class SledImage < ApplicationRecord
         j = identifier_for(i)
 
         # Check to see if object exists
-        if j && k = Identifier::Local::CatalogNumber.find_by(p[:identifiers_attributes].first.merge(identifier: j, identifier_object_type: 'CollectionObject'))
+        if j && k = Identifier::Local::CatalogNumber.find_by(p[:identifiers_attributes].first.merge(identifier: j, identifier_object_type: ['Container', 'CollectionObject']))
           # Remove the identifier attributes, identifier exists
           p.delete :identifiers_attributes
           p.delete :tags_attributes
@@ -235,7 +235,7 @@ class SledImage < ApplicationRecord
           p.delete :data_attributes_attributes
           p.delete :total
 
-          k.identifier_object.update!(p)
+          (k.identifier_object.try(:collection_objects) || [k.identifier_object]).each { |o| o.update!(p) }
         else
           c = CollectionObject.new(p)

However, sled_image depictions cannot be assigned to multiple objects (validates_uniqueness_of :sled_image_id, scope: [:project_id, :sled_image_x_position, :sled_image_y_position]):
image
The Container::Slide in question has a Lot and a Specimen.

Without the patch above this would happen:
image

What could be done here @mjy, @jlpereira?

I'll likely release to production tomorrow morning with current code (without aforementioned patch), as it is better behavior than 500 error and solves the reported main problem.

@tmcelrath
Copy link
Author

With the code you added them @LocoDelAssembly, will the process succeed for all slides but the offending containers? That would be enough of a fix for me. The other images/newly created collection objects could be assigned manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An existing function is broken.
Projects
None yet
Development

No branches or pull requests

3 participants