Bug #13511

Regression causing data on related tables to be dropped on save

Added by Dan Gillean 5 months ago. Updated 15 days ago.

Status:VerifiedStart date:05/27/2021
Priority:CriticalDue date:
Assignee:-% Done:

0%

Category:Internals
Target version:Release 2.7.0
Google Code Legacy ID: Tested version:
Sponsored:No Requires documentation:No

Description

Some recent commit has caused a regression that is impacting multiple entities, and causing some data to be dropped on save. So far confirmed examples include:

  • event dates added to new or edited descriptions
  • Note data (any custom rad note type; general notes, etc)
  • actor-actor relationships

This was first found via Vagrant testing on 2021-05-25 when testing in Vagrant. The behavior has been confirmed by a forum user on 05-27, here:

To reproduce, create a new description, add a date of creation and a general note, and save.

Notes

First theory was the recent large community PR (here ) might have broken it. However, my VM did not include the commits from the PR when first reproduced - see my attached tig output

Next theory was that the STRICT_TRANS_TABLES sql_mode might be causing it. I checked and this was enabled in the vagrant box (so we can find more incompatibilities, per issue # ). However, when I changed sql modes, restarted MySQL, cleared the application cache/PHP-FPM, and reindexed, I was still unable to add event dates to descriptions. It's possible that this is still the culprit somehow but I did double-check that my setting changes were in effect.

There are no relevant error messages I can see in the webserver logs.

The forum user also added:

I enabled general_log in MySQL so I could trace the SQL being generated by attempts to change those values, to see if the INSERT/UPDATE queries were being generated, but failing silently on the DB side. As far as I can tell, no SQL call is being made for the activities that fail.

tig-2021-05-27.png (215 KB) Dan Gillean, 05/27/2021 04:29 PM


Related issues

Related to Access to Memory (AtoM) - Bug #13556: Can't translate ISAD note/RAD generalNote Verified 08/24/2021

History

#1 Updated by Steve Breker 5 months ago

Replicated in latest 2.7.0.2 Vagrant VM.
- created new VM
- pulled all latest commits into atom qa/2.x
- did a tools:purge --demo
- set default template to RAD
- created new description with event date
- save
- dates not recorded on description

- set default template to ISADG
- created new description with event date and note
- save
- dates and notes not saved with description.

#2 Updated by Dan Gillean 3 months ago

  • Priority changed from High to Critical

This issue is blocking the QA of other issues, and must be fixed before we can prepare a 2.7 release. Bumping up priority.

#3 Updated by David Juhasz 2 months ago

  • Status changed from New to In progress
  • Assignee set to David Juhasz

#4 Updated by David Juhasz 2 months ago

I think this regression is due to enabling CSRF form validation (https://github.com/artefactual/atom/commit/8e6f28d9d6952945b8f0c373f3e65fe578793da7). I've found that the archival description dates use a sub-form for each row of dates, and the CSRF token is not being properly passed to the sub-forms during form validation. I'm guessing that the other data that is being dropped on form save are also using sub-forms.

#5 Updated by David Juhasz 2 months ago

I've created a dev commit that fixes the form submit for date data: https://github.com/artefactual/atom/commit/dc92d7cff398f85ac22b27870e7f0fee2b1e2ae4

I'll keep working on the other data that are being dropped.

#6 Updated by Dan Gillean about 1 month ago

  • Related to Bug #13556: Can't translate ISAD note/RAD generalNote added

#7 Updated by Mike Cantelon about 1 month ago

Yeah, this definitely seems to be related to CRSF protection. If I add this line "if ($name == '_csrf_token') { return; }" at the start of the addError method in the sfValidatorErrorSchema class then the data gets submitted as expected.

#8 Updated by José Raddaoui Marín about 1 month ago

  • Status changed from In progress to QA/Review
  • Assignee deleted (David Juhasz)

Hi there,

If you're still working on this, we found and fixed the issue during the Bootstrap 5 work ...

https://github.com/artefactual/atom/pull/1432

#10 Updated by Dan Gillean 15 days ago

  • Status changed from QA/Review to Verified

Also available in: Atom PDF