-
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
Work-in-Progress: 2043-guestbook-at-request code review requested #6902
Conversation
Merge upstream changes
merge IQSS develop branch into mdmADA fork
pull request to merge IQSS into mine again
merge IQSS into fork
merge iqss develop into mdmADA develop
merge iqss develop into 2043-alter-fileaccessrequests branch
…onse_id as foreign key for request access that requires guestbook response
merge develop into 2043-alter-fileaccessrequests-tbl
2043 alter fileaccessrequests tbl
…opup-fragment.xhtml with guestbook-terms-popup-fragment.xhtml
…html is required due to toA/toU and/or guestbook.
…e calls to downloadPopupRequired()
…red with guestbookAndTermsPopupRequired(). replaced downloadPopup and requestAccessPopup with guestbookAndTermsPopup
pull dataverse main into branch
pull dataverse develop into branch
…d to fileaccessrequests table.
…Access (while still allowing guestbook-at-file-download).
Merge develop into branch
…oMany to OneToOne since one guestbookresponse will be associated with one fileaccessrequest
…o V4.16.0.4__2043-alter-fileaccess-requests-tbl as it has not yet been merged to the dataverse develop and it was clashing with V4.16.03__6156-FooterImage...
Changing the filename in my source code caused problems.
…quests by userid, fileid, requeststate instead of getting ALL the FileAccessRequests for a user/state combo - the latter would grant access to all requests the user has made.
FYI. I've read through and the code for backend changes to separate the guestbook from downloads looks reasonable (noted a bug and saw that other commits are coming in). There are a couple things I haven't been able to understand from the issue, Functional Requirements Google doc and PR. I know there have been other discussions so apologies if I've just missed something: What's the state diagram for the possible states: CREATED,EDITED,GRANTED,REJECTED,RESUBMIT,INVALIDATED,CLOSED How will guestbook entries and downloads be related now? Is it still 1-1 (with delay/approval process) or do guestbook entries now cover multiple downloads? Or? Part of the reason for my question is trying to understand whether there's a change from 1-to-1 and whether there might be a generic change even for people who won't be doing separate approval processes (e.g. should approvals be for any files covered for a certain time period? Should the files/datasets covered by an entry be changeable? Note that I'm not suggesting these changes since I'm not a stakeholder, but I'm wondering what changes ADA requires and am then trying to think about what other use cases might be addressed with the same design, etc.) What's the UI for this functionality? I understand that the basic change that guestbook entries would be required at request time, and that retrieving the guestbook entry from the permissions page is desirable, but I'm wondering if there's anything specific to the set of states above or to the ~manual/external approval process? Part of my question(s) is/are that I'm curious to know if anyone would object to always triggering the guestbook at request time/if there's anything that has to be different in the UI to support the manual/external approval or if this could be a generic change/without any flag to chose between approval modes. I'm not sure whether this is best discussed here, or in the issue, or as a doc/edits to the existing doc, or if we should get back on a web call - I'm open to any. |
Thank you very much for taking the time to look at the code, even in its current state, and for the feedback. The Requirements Google doc may need some revisiting and reworking - I had intended it as an initial cut. I also have to review and add in some notes I had taken in a meeting between myself and Gustavo many months ago. I do have to work through the Request Access state diagram and may have to pare it back after working a bit more on the code. (For example: The fileacessrequest is not revoked but rather the access itself once it has been granted is revoked, so 'REVOKED' probably isn't a valid file access request state.) The question regarding the relationship between guestbookresponse and filedownload is an excellent one. In ADA's original fork (and that ADA is still using), the guestbookreponse from an access request is written to the guestbookressponse table with 'Request Access' as the downloadtype and it's written for each datafile for which access is requested. Corresponding records are also written to the fileaccessrequests table. So after a user requests access, the button changes to 'Access Requested' based on the records in the fileaccessrequests table. Once the user has been granted access, they can download the file as many times as they like, and each download is recorded in the guestbookresponse table but with only the bare minimum written into the guestbookresponse details (not all the data from the original request). In the version I am developing in this PR, a single guestbookresponse will also cover multiple filedownloads but different requestaccess/download scenarios and their design implications have to be identified and worked through. WRT the UI, I have advocated for a flag be set in the UI to indicate where the guestbook would pop up - at request access or download (default could be download or maybe set as a setting in the database). @scolapasta will hopefully correct me if I'm wrong, but he advocated for not having a UI switch and instead handle it with logic:
But that leads to logic challenges in determining whether or not to popup the guestbook at 'Download' because it won't be easy to determine if the button is 'Download' because the file was never restricted or because it was restricted and access was granted through RA/gb (and will have already been answered). Or when there is a mix of the two types of 'Download' through the multi-select pathway. To me, a nice flag would simplify that logic at each workflow point quite nicely but then would not allow guestbook at request for Restricted files and guestbook at download for non-restricted files in the same dataset - it would have to be one or the other. All in all, on the surface it seems like it should be a fairly straightforward issue to implement guestbook at request access but there are quite a few details that need attention. I think I need to consult with @scolapasta to think a bit more about the issues that have been highlighted above (and to identify other possible scenarios). So I will get back in touch with @scolapasta (lucky you, Gustavo!) to restart that discussion. Then arrange another zoom meeting with Jim and Steve and Danny in as well when ready. Thank you! |
Sorry - was off work dealing with Covid in kids and myself. ADA has implemented questbook at request access in 4.6.1 but as 99% of datasets are mediated access (ie. people have to request access with responses to guestbook questions and decision to grant access based on those responses), I had implemented this by moving the guestbook to be called when the 'Request Access' button is clicked. This means that for ADA Dataverse 4.6.1, there is no possibility to have guestbook at download for what ADA considers 'open recorded' data (ie. free to download but want responses for the guestbook). The challenge is in allowing a dataset creator to specify if the guestbook is supposed to be at download or at request access (this would require a change to the UI to allow a selection of where the guestbook to be called). In 4.6.1, I created a field gb_popup_point (or something similar) with the default being 'download' but it being set to 'request' for ADA. I didn't make any changes to the dataset UI to actually use this flag though and the code itself calls it at Request Access only. In my discussions with Gustavo several years ago now, he had brought up different scenarios that I hadn't thought of (I can't remember what those are now but may be able to find them in notes from back then). ADA had also considered tracking the status of a Request Access (what Jim was referring to in his comments on CREATED,EDITED, etc.) - but it became too complicated. ADA also found the guestbook in its current flavour is too limiting - limited number of html input types, no logic branching, no way to reorder questions nor reordering the options. So ADA created a new fork and modified it by having the Request Access button call an external tool. This could be anything as long as the Dataverse call to the tool has the url and can pass parameters to the tool (I had considered calling a form created in smartsheet for example). In the end, ADA created its own form with the external tool url and it's parameters being encrypted. The external tool creates the guestbook as a form and writes the responses back to Dataverse. The external tool application also creates an 'edit' link when the user clicks submit and that is appended to the ticket for the access managers in ADA. If there is a problem with the user's request application, the access manager sends the user a ticket with the form re-opening link and they can edit their responses and re-submit. ADA is scheduled to deploy this new version in the future, exactly when has yet to be determined. |
Hi @adam3smith - Is your team still interested in this guestbook-at-request access functionality? ADA is still interested in getting it into the dataverse main branch so I am wondering if we can pick this issue up again. So much functionality has been implemented since I created the PR over 2 years ago now, I am not sure what knock-on effects (if any) there would be for implementing the guestbook-at-request in the current version (5.11.1). Interested in re-starting the conversation on this though, if others are interested. |
Yes, we're still quite interested in this. In our case, we're very closely tracking the current DV releases, so should be able to get a better sense of how this relates. I'm off next week, but will discuss with Jim when I get back on the 22nd |
@mdmADA - is there a time we can discuss this? (Could perhaps start in the community call today?) I think the updating the code here will probably cover what QDR is interested in but it seems you've gone further with an external tool, etc., so it's not clear to me whether an update to this PR is in-line with where you want to go or not/whether it can be done in a way to support a tool or at least decrease the work to add that. If we can figure this out, I should have time via QDR to help push it forward. |
Hi @qqmyers - I will be on the call this afternoon to discuss this. ADA had developed an external tool but then MIngjing took another position and the tool was never deployed. Having the option of guestbook at download would still be of interest to ADA to allow ADA to move to dataverse 5 (or maybe 6?) sooner rather than later. ADA is looking to have the Dataverse Request Access button to call an external platform that is currently being designed. Until that platform is launched (June 2023 for Beta), having the guestbook at request would be beneficial. And it seems that others would like that option as well. |
In this attempt I tried to copy/paste changes from the original PR into v5.12.1 and to guess at the required modifications when there had been changes in the code (versus applying the PR to v4.x and then trying to merge the 3 years of changes since it was created.)
FYI: I'm slowly working to create a PR to apply these changes to v5.12+. Until then, please leave this (now draft) PR as a placeholder/aid in making an updated version. |
Thank you for updating the issue Jim. I (for one) will be sure to leave this PR as is, and won't close it. ADA will be looking forward to having this implemented to move to the latest v5.x, even before the external tool ADA will be using for requesting access is deployed. |
HI @qqmyers and @pdurbin. Is there a probable timeline for having this guestbook functionality in an upcoming release? ADA needs to move dataverse 5.x as soon as possible but that requires this guestbook functionality at request access to be in place. If you could provide a timeline, that would be appreciated. Thanks! |
I have a branch started at https://github.com/QualitativeDataRepository/dataverse/tree/guestbookatrequest and QDR still has this on its roadmap, but I've been pulled onto a higher priority lately. It's still possible that I can get this done in time for a 5.14/6.x release before the community meeting in June, but not guaranteed. (I also have to make a separate branch without QDR's other changes to make a PR). |
Closing in favor of this new PR: |
THIS IS NOT READY TO BE MERGED - I AM ONLY ASKING FOR @scolapasta and @qqmyers to look at my code (so far) to implement the guestbook-at-request (or guestbook-at-download) functionality.
I would ideally like a flag/checkbox in the UI that allows the user who creates the dataset to specify where the guestbook should popup (download/request) - either at 'Request Access' or at 'Download'. It would simplify the logic (IMO) in the code where it has to be determined whether or not a guestbook is required at that point in the workflow. And also allow greater flexibility in the future if the guestbook needs to be popped up somewhere else rather than binding the guestbook to specific logic that has to be untangled again to add new functionality.
Also, by having the flag/checkbox in the UI, the user knows exactly what will happen with the guestbook rather than having that knowledge buried in the logic behind the UI.
Anyway, if you can look at the code - it's not complete and I have to determine where I was when I last left it. But it would be much appreciated to get feedback now if possible.
Thanks!