Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

7767 unable to download large guestbooks #7931

Merged
merged 21 commits into from Jun 23, 2021

Conversation

sekmiller
Copy link
Contributor

@sekmiller sekmiller commented Jun 8, 2021

What this PR does / why we need it: Adds an api for retrieving the guestbook responses for a dataverse. We need it because currently the only way to retrieve these responses is via the ui and with larger dataverses the download may not work properly.

Which issue(s) this PR closes:

Closes #7767 Unable to download certain guestbooks related to number of responses

Special notes for your reviewer: I split up the existing streaming method in the service bean in order to facilitate the api

Suggestions on how to test this: was tested under my local db which has nowhere near the number of records as prod.

Does this PR introduce a user interface change? If mockups are available, please link/include them here: no

Is there a release notes update needed for this change?: as new functionality, probably deserves a mention.

Additional documentation:

@coveralls
Copy link

coveralls commented Jun 8, 2021

Coverage Status

Coverage decreased (-0.007%) to 19.317% when pulling 480cf21 on 7767-unable-to-download-large-guestbooks into d2565b8 on develop.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this looks great. I'm excited to have yet another existing feature be available via API.

I left a minor doc comment.

I also suggested creating a command but please disregard if it's a ton of work.

export SERVER_URL=https://demo.dataverse.org
export ID=root

curl -O -J -H X-Dataverse-key:$API_TOKEN $SERVER_URL/api/dataverses/$ID/guestbookResponses?guestbookId=1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest adding a GUESTBOOK_ID variable to match the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 5186396

DataverseRequest req = createDataverseRequest(u);
if (permissionSvc.request(req)
.on(dv)
.has(Permission.EditDataverse)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing this permission check here in the API makes me wondering if we should create a command for both the API and UI to use.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with the doc fix and from talking to @sekmiller it sounds like refactoring into a command is something we can pick up later.

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🦁 to QA 🔎✅ Jun 8, 2021
@kcondon kcondon self-assigned this Jun 14, 2021
@kcondon kcondon merged commit 8bac500 into develop Jun 23, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Jun 23, 2021
@kcondon kcondon deleted the 7767-unable-to-download-large-guestbooks branch June 23, 2021 18:04
@djbrooke djbrooke added this to the 5.6 milestone Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Unable to download certain guestbooks, possibly related to number of responses (~150,000 responses)
5 participants