ADA/Guestbook-at-request #9599
Merged
ADA/Guestbook-at-request #9599
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sekmiller
reviewed
Sep 25, 2023
| @@ -63,30 +63,28 @@ public class GuestbookResponseServiceBean { | |||
| + " and r.dataset_id = o.id " | |||
| + " and r.guestbook_id = g.id ";*/ | |||
|
|
|||
| private static final String BASE_QUERY_STRING_FOR_DOWNLOAD_AS_CSV = "select r.id, g.name, o.id, r.responsetime, f.downloadtype," | |||
| private static final String BASE_QUERY_STRING_FOR_DOWNLOAD_AS_CSV = "select r.id, g.name, o.id, r.responsetime, r.downloadtype," | |||
There was a problem hiding this comment.
we need to say r.eventtype here
sekmiller
reviewed
Sep 25, 2023
| + " and r.dataset_id = o.id " | ||
| + " and r.guestbook_id = g.id "; | ||
|
|
||
| // And this query is used for retrieving guestbook responses for displaying | ||
| // on the guestbook-results.xhtml page (the info we show on the page is | ||
| // less detailed than what we let the users download as CSV files, so this | ||
| // query has fewer fields than the one above). -- L.A. | ||
| private static final String BASE_QUERY_STRING_FOR_PAGE_DISPLAY = "select r.id, v.value, r.responsetime, f.downloadtype, m.label, r.name " | ||
| + "from guestbookresponse r, filedownload f, datasetfieldvalue v, filemetadata m , dvobject o " | ||
| private static final String BASE_QUERY_STRING_FOR_PAGE_DISPLAY = "select r.id, v.value, r.responsetime, r.downloadtype, m.label, r.name " |
guestbookAndTermsPopupRequired (and downloadPopupRequired before it) was not defined and I think therefore was always false, meaning the option to show a popup never occurred. This may have been OK in practice since one would have to have accepted the terms popup to show the preview and preview pane with these buttons. The fix here should show the terms popup prior to allowing the explore button to be clicked if/when these buttons ever show and one hasn't already accepted the terms (and termsMet is therefore true).
sekmiller
approved these changes
Sep 26, 2023
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34)
automation
moved this from In Review 🔎
to Ready for QA ⏩
Sep 26, 2023
misplaced quote marks
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34)
automation
moved this from Ready for QA ⏩
to Done 🚀
Sep 29, 2023
This was referenced Oct 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it: This is a restart of #6902 which adds optional support for guestbooks to appear when files access is requested rather than after access has been granted and a download is started. This is key functionality ADA needs to be able to adopt Dataverse 5.x+.
The PR currently compiles but is not yet known to be working. (Making the PR now to make the progress more visible and to be able to monitor the automated test runs.)
Which issue(s) this PR closes:
Special notes for your reviewer:
Suggestions on how to test this:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: