Guestbook form records pre-populated entries into DB without regard for changes

[jggautier]

Updated issue description based on @pdurbin's and @sekmiller's comments below. These issues are seen in Dataverse 4.8.4

a. The guestbook isn't recording the information people enter and is recording only pre-populated information:

If someone is logged in and downloads a file from a dataset with a guestbook, the form is pre-populated with info from that person's account (name, email, etc). Only that info is accepted, copied from the database's authenticateduser table to the guestbookresponse table. Changes someone makes to that pre-populated info are not recorded in the guestbook. Answers to any custom questions are not recorded.

If someone is not logged in and downloads a file from a dataset with a guestbook, the form is pre-populated with "Guest" in the name field, and the guestbook records only "Guest". Any other values entered into fields, including custom question fields, are not recorded.

b. There's no validation on the guestbook form, so someone can leave required fields blank and still download a file from a dataset with a guestbook.

Definition of done for this issue would be when:

• The guestbook accepts all text in all of the form's fields, pre-populated and user entered
• The form has validation for required fields

[pdurbin]

I've been poking around at the code and I just mentioned to @sekmiller that there's another bug I observed, which is that no matter what I type for "Name" (such as "Homer Simpson", below), the value "Guest" gets put into the database.

I know that "Guest" is the default value (see #3628) but we should be recording whatever value is entered, of course. I'm on commit 530377d on the "develop" branch.

Update: I can reproduce this bug http://phoenix.dataverse.org also on commit 530377d.

[pdurbin]

@sekmiller and I have been discussing this issue and some new information for me is that if a user is logged in and fills in a guestbook, the values for that user (name, email, etc.) are copied from the authenticateduser table to the guestbookresponse table. Let's look at "Dataverse Admin" as an example:

dataverse_db=# select id,name,datafile_id,authenticateduser_id from guestbookresponse; 
 id |      name       | datafile_id | authenticateduser_id 
----+-----------------+-------------+----------------------
 20 | Guest           |          13 |                     
 21 | Guest           |          13 |                     
 22 | Guest           |          13 |                     
 23 | Dataverse Admin |          13 |                    1
(4 rows)

Note above that rather than "Guest" we see "Dataverse Admin". However, if you try to change any values in the guestbook form (if you entered an alternate email address, for example), those changed values are not persisted as a guestbook response. In short, whatever the form is pre-populated with is what is persisted.

[pdurbin]

In standup I volunteered to review pull request #4404, especially because some of the same files are in play over in #3657 for Modular Explore. That is to say that there is a high potential for merge conflicts.

[pdurbin]

Actually, I tried merging the branches in both directions and the only file that git cannot automerge is GuestbookResponseServiceBean.java and it seems like it's only a minor conflict having to do with imports.

[pdurbin]

I just found two things that need to be fixed in pull request #4404

• There seems to be a bug when you make "institution" required. You can never fill out the form (it seems to be checking for email instead).
• For UI consistency, there should be a space before the red asterisk for custom questions. Screenshot below.

[pdurbin]

@sekmiller made some fixes in 5c5b19b. Moved to QA. The big fixes in this issue are:

• What you type in the form actually gets saved in the database now.
• Required fields are now honored.

We don't know exactly how long this stuff has been broken but the working theory is ever since the file landing page was introduced.

[kcondon]

OK, based on Julian's updated issue report this is ready to merge, just waiting to hear whether Tania needs to look at it.

[pdurbin]

@TaniaSchlatter and I talked. Not to put words in her mouth but she seemed good with it.