Bug #10075

Can't edit form first time if serialized property value invalid

Added by Tim Hutchinson about 4 years ago. Updated over 2 years ago.

Status:VerifiedStart date:06/27/2016
Priority:MediumDue date:
Assignee:-% Done:

0%

Category:Form validation
Target version:Release 2.4.0
Google Code Legacy ID: Tested version:2.2, 2.3
Sponsored:No Requires documentation:

Description

To reproduce:

step 1)
  • import a descriptive record with an incorrect value for Script (e.g. en or latn, all lowercase)
  • the value in property.i18n will be something like a:1:{i:0;s:4:"latn";}
  • Script does not appear on the description view page, and it is not populated in the edit form
step 2)
  • via the edit form, try to add the correct value, or even a different one, for Script
  • save the form
  • Script value does not appear, database value the same
step 3)
  • edit the form again
  • this time the change will take, and the database value will be correct, e.g. a:1:{i:0;s:4:"Latn";}

History

#1 Updated by Tim Hutchinson about 4 years ago

I've run into a similar issue (in 1.3.1) where the serialized value for language is blank - i.e. a:0:{}. In that case it was caused by a custom import routine, but the behaviour is the same (need to edit twice before that field can be changed).

#2 Updated by Dan Gillean about 4 years ago

  • Tested version 2.2, 2.3 added

Thanks for the report, Tim!

Mentioned in user forum thread: https://groups.google.com/d/msg/ica-atom-users/KTfkg4D43NE/QqEbKpOPAgAJ

#3 Updated by Nick Wilkinson over 3 years ago

  • Assignee set to Mike Cantelon

#4 Updated by Mike Cantelon over 3 years ago

  • Status changed from New to Feedback
  • Assignee changed from Mike Cantelon to Nick Wilkinson

I can't replicate this in qa/2.4.x. Seems it was likely fixed (or some precondition in data is different: I did this test on a fresh install of AtoM).

#5 Updated by Dan Gillean over 3 years ago

  • Category set to Form validation
  • Assignee changed from Nick Wilkinson to Tim Hutchinson

Tim, I can try to reproduce this, but since you filed the issue, I wonder if you want to try as well in your local 2.4 test environment?

#6 Updated by Tim Hutchinson over 3 years ago

Will do. I did verify the incorrect serialization for null values (in particular the resulting edit behaviour) the other day - I wanted to use that as an example of a MySQL batch fix. But in that case the bad data was created by a custom import script, not normal workflow. I'll test to see whether bad language/script values are being handled differently.

#7 Updated by Tim Hutchinson over 3 years ago

I don't have a fresh install, but I purged data in a current 2.4 environment, and I was able to reproduce the issue.

The most important point might be that if you have incorrect language or script values, they will get imported incorrectly, with no warning, without being displayed. It seems they get deleted if that field is subsequently edited. E.g. latn instead of Latn.

#8 Updated by Dan Gillean about 3 years ago

  • Assignee changed from Tim Hutchinson to Mike Cantelon

Mike, if you are still looking at this:

A simple improvement might be to make the import code case insensitive. This bug was first discovered when someone tried to import "latn" instead of "Latn" as a script code value. If AtoM could better handle that we might avoid a lot of issues. We've had the same problem in the past with other fields (like the publication status field) and resolved it with a strtolower or its opposite function, I believe - sanitizing the input data bit before throwing a warning or inputting a bad value.

If that seems doable, I think it would probably help a lot?

Better reporting on bad values in the CLI could help too. For example, we know, based on the way we've chosen to implement ISO 15924, that we are always expecting a 4-letter value starting with a capital for this field. Can we add better controls on the field so 1/2/3-letter or 5+-letter values produce a warning and are skipped (not stopping the import) instead?

#9 Updated by Mike Cantelon about 3 years ago

  • Status changed from Feedback to Code Review
  • Assignee changed from Mike Cantelon to Nick Wilkinson

I've made these changes (case normalization and error upon incorrect value).

PR for CR: https://github.com/artefactual/atom/pull/554

#10 Updated by Nick Wilkinson about 3 years ago

  • Assignee changed from Nick Wilkinson to Mike Gale

#11 Updated by Mike Gale about 3 years ago

  • Assignee changed from Mike Gale to Mike Cantelon

looks good

#12 Updated by Mike Cantelon about 3 years ago

  • Status changed from Code Review to QA/Review
  • Assignee changed from Mike Cantelon to Dan Gillean

Merged into qa/2.4. for QA.

#13 Updated by Dan Gillean about 3 years ago

  • Assignee deleted (Dan Gillean)

#14 Updated by Dan Gillean over 2 years ago

  • Status changed from QA/Review to Verified
  • Target version set to Release 2.4.0

Also available in: Atom PDF