Bug #9027

Can't add existing donor to accession record

Added by David Juhasz about 5 years ago. Updated about 5 years ago.

Status:VerifiedStart date:10/02/2015
Priority:CriticalDue date:
Assignee:José Raddaoui Marín% Done:

0%

Category:Accessions
Target version:Release 2.2.1
Google Code Legacy ID: Tested version:2.2, 2.3
Sponsored:No Requires documentation:

Description

Two separate users have reported an error when trying to link an accession record to an existing donor.

To reproduce

  1. Create a new accession record
  2. Choose an existing donor from the "donor" automcomplete
  3. Save the accession record

Current result

500 Error is returned and accession record is not created.

Expected result

Accession record saves correctly.

History

#2 Updated by David Juhasz about 5 years ago

  • Priority changed from Medium to Critical

#3 Updated by David Juhasz about 5 years ago

  • Assignee changed from Jesús García Crespo to David Juhasz

This regression seems to have been introduced with https://github.com/artefactual/atom/commit/8ef2f8191fea4df4ff0e2a120891737374602d6c. Reverting this commit allows the accession form to be submitted and the selected donor is linked properly.

The hidden "relatedDonor[resource]" field is being set to "undefined" at selecting a donor triggers two javascript warnings about "Empty string passed to getElementById()."

#5 Updated by David Juhasz about 5 years ago

  • Assignee changed from David Juhasz to José Raddaoui Marín

#6 Updated by David Juhasz about 5 years ago

  • Status changed from New to In progress

#7 Updated by José Raddaoui Marín about 5 years ago

  • Status changed from In progress to Code Review
  • Assignee changed from José Raddaoui Marín to Nick Wilkinson

I've also fixed a problem loading existing relations, where the donor name and other fields were not loaded.

Ready for code review: PR 246

I'll add the fix to stable/2.2.x after it's merged and tested in qa/2.3.x.

#8 Updated by Jesús García Crespo about 5 years ago

  • Status changed from Code Review to Feedback
  • Assignee changed from Nick Wilkinson to José Raddaoui Marín

What was the dialog ID?

#9 Updated by José Raddaoui Marín about 5 years ago

  • Status changed from Feedback to Code Review
  • Assignee changed from José Raddaoui Marín to Jesús García Crespo

The dialog ID is usually the URL that returns the dialog data. In this case, it is an endpoint that loads the accession-donor relation and the primary contact information of that donor. For non exinting relations (new dialogs) the dialog ID is created when it's openned (new0, new1, etc).

In this particular dialog, we have a second call to a different endpoint to load the primary contact data when a donor is selected in the autocomplete. It's a different endpoint because the relation doesn't exists yet and instead of being a relation resource endpoint it's a donor one. Before the commit David said in update 3, that endpoint was misspelled and it was not loading the data but, after the fix, it was loading the data in a different data group inside the dialog.

The dialog.loadData(URL) funtion loads the data in a data group where ID=URL. We were using a relation URL on the first load and a donor URL when the donor is changed, this was creating two different data groups with different URLs in the ID. On save, when the ID with the URL exists, it tries to load the existing relation object and it was failing when the URL was from a donor instead of from a relation.

To update the data in the same group we can't call loadData(URL) with a different URL, but we can't also call it with the current dialog ID, as it will fail in not saved dialogs where the ID is not an URL (new0, new1, etc). We need to get the data without calling loadData(URL) and call updateDialog(ID, data) with the current dialog ID and the data from the donor URL. The updateDialog function will work when the ID is an URL or a new ID, as it only overrides the data group of that ID. The current dialog ID (URL or not) is saved in 'dialog.id' and it's updated each time a dialog is openned.

#10 Updated by Jesús García Crespo about 5 years ago

  • Status changed from Code Review to Feedback
  • Assignee changed from Jesús García Crespo to José Raddaoui Marín

Very nice! Thank you. LGTM.
Sounds like we could simplify this using promises, isn't it? I guess that'd be a different issue.

#11 Updated by José Raddaoui Marín about 5 years ago

  • Status changed from Feedback to QA/Review
  • Assignee changed from José Raddaoui Marín to Dan Gillean

Merged in qa/2.3.x

#12 Updated by Sarah Romkey about 5 years ago

  • Status changed from QA/Review to Verified

Tested and verified on qa-2.3.

#13 Updated by Sarah Romkey about 5 years ago

  • Status changed from Verified to Feedback
  • Assignee changed from Dan Gillean to José Raddaoui Marín

Radda, could you please cherry-pick to stable 2.2?

#14 Updated by José Raddaoui Marín about 5 years ago

  • Status changed from Feedback to Verified

Added to stable/2.2.x ;)

Also available in: Atom PDF