-
Notifications
You must be signed in to change notification settings - Fork 493
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
3609 large guestbooks #4057
3609 large guestbooks #4057
Conversation
May require/benefit from further optimizations though. [#3609]
…liminating the extra lookup for custom questions and answers, that was adding one extra query per every guestbookresponse in the search results. (#3609)
… guestbook data. Lots of optimizations and fixes. Will add more info in #3609 explaining what's been done and how things are supposed to work now.
…ed up the display limit to 5K. (#3609)
…et Guestbook and Guestbook Responses pgs. [ref #3609]
Minor edits to messaging for clarity and brevity
…t permission on the dataverse (similar to how manage-guestbooks operates).
|
||
queryString += " order by r.id desc"; | ||
|
||
if (limit != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we ever want to querry with non null
limits? if not limit shoud be long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the "???" I mentioned at #3609 (comment)
…to change the display limit on the number of guestbook entries. (#3609)
@@ -1052,7 +1052,7 @@ dataset.manageGuestbooks.tab.header.date=Date Created | |||
dataset.manageGuestbooks.tab.header.usage=Usage | |||
dataset.manageGuestbooks.tab.header.responses=Responses | |||
dataset.manageGuestbooks.tab.header.action=Action | |||
dataset.manageGuestbooks.tab.action.btn.view=View | |||
dataset.manageGuestbooks.tab.action.btn.view=Preview |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use this. Use preview
in the bundle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All my feedback has been addressed. Looks good!
New Contributors
Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Thanks!
Related Issues
Pull Request Checklist