Skip to content

2043 alter fileaccessrequests tbl#6207

Closed
mdmADA wants to merge 7 commits intoIQSS:developfrom
mdmADA:2043-alter-fileaccessrequests-tbl
Closed

2043 alter fileaccessrequests tbl#6207
mdmADA wants to merge 7 commits intoIQSS:developfrom
mdmADA:2043-alter-fileaccessrequests-tbl

Conversation

@mdmADA
Copy link
Contributor

@mdmADA mdmADA commented Sep 24, 2019

Submitting flyway script to add guestbookresponse_id column to fileaccessrequests table to allow File Access Request with Guestbook.

When a File Access Request with Guestbook is created in forked code, the guestbook data is written to the guestbookresponse table with 'Request Access' as the Download type. This worked for that code but the preferred solution, in consulting with @scolapasta is to write the corresponding guestbookresponse_id to the fileaccessrequests table.

Adding this column to fileaccessrequests will affect the @manytomany relationship created in DataFile.java. Feedback as to the preferred way to manage this third column in the @manytomany fileaccessrequests relationship in DataFile.java is appreciated.

I hope the flyway script is correct.

mdmADA added 7 commits May 16, 2019 17:06
Merge upstream changes
merge IQSS develop branch into mdmADA fork
pull request to merge IQSS into mine again
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
@dataversebot
Copy link

Can one of the admins verify this patch?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.001%) to 19.506% when pulling f488153 on mdmADA:2043-alter-fileaccessrequests-tbl into 6d22ea4 on IQSS:develop.

@scolapasta
Copy link
Contributor

@mdmADA Can you explain a little more what you mean about affecting the @manytomany relationship? I just want to verify that this PR can be merged now independent of that (and that figured out later, when this new column is "used"), or if if you think that needs to be resolved first?

@mdmADA
Copy link
Contributor Author

mdmADA commented Oct 1, 2019

Hi @scolapasta - in my original forked code, to implement the guestbook-at-request functionality, when the guestbook is filled out at request access, it is then written to the guestbookresponse with downloadType='Request Access'.

But now in the main dataverse codebase, since guestbookresponse has been split into guestbookresponse and filedownload, without changing any code right now, the 'request access' will be written to filedownload with downloadType='Request Access'.

Since the only things being written to filedownload should only be records related to actual file downloads, it was agreed that instead of writing 'Request Access' to the filedownload table, to add the column guestbookresponse_id to the fileaccessrequests table.

So then on 'Request Access', the guestbookresponse would be written to the guestbookresponse table and the guestbookresponse_id written to fileaccessrequests along with the other column values (datafile_id, authenticateduser_id, guestbookresponse_id). (In the case of no guestbook-at-request-access, the guestbookresponse_id in fileaccessrequests will be null.)

This guestbookresponse_id in fileaccessrequests will also allow dataverse to pull the guestbookresponse associated with a specific fileaccessrequest to display to the user who will be granting/rejecting access.

The issue I need to figure out is the @manytomany relationship for fileaccessrequests currently specified in DataFile:


@manytomany
@jointable(name = "fileaccessrequests",
joinColumns = @joincolumn(name = "datafile_id"),
inverseJoinColumns = @joincolumn(name = "authenticated_user_id"))

private List<AuthenticatedUser> fileAccessRequesters;
public List<AuthenticatedUser> getFileAccessRequesters() {
    return fileAccessRequesters;
}
public void setFileAccessRequesters(List<AuthenticatedUser> fileAccessRequesters) {
    this.fileAccessRequesters = fileAccessRequesters;
}

Since fileaccessrequests now has the third guestbookresponse_id column, how do I change the above @manytomany relationship in DataFile to write the guestbookresponse_id value to that column? I am not sure it's possible which is why I am asking for some advice.

After a bit of research, to replace this @ManyTonMany code above, I implemented a new class FileAccessRequest.java to replace that @manytomany code in DataFile. The FileAccessRequest is mapped to the fileaccessrequests table with (and the getter and setter methods):

@ManyToOne(cascade = {CascadeType.PERSIST, CascadeType.MERGE})
@JoinColumn(name = "datafile_id")
private DataFile dataFile;

@ManyToOne(cascade = {CascadeType.PERSIST, CascadeType.MERGE})
@JoinColumn(name = "authenticated_user_id")
private AuthenticatedUser user;

@ManyToOne(cascade = {CascadeType.PERSIST, CascadeType.MERGE})
@JoinColumn(name = "guestbookresponse_id")
private GuestbookResponse guestbookResponse; 

The @onetomany set of fileAccessRequests is also added to DataFile, AuthenticatedUser and GuestbookResponse.

This requires a lot more modification of the code, though, and I can't quite get it to work
properly. It will also require, I believe, a generated id column for the fileaccessrequests table so there would be 4 columns: id, datafile_id, authenticated_user_id, guestbook_response_id.

So my questions are:

  1. Is there a way to add the guestbookresponse_id into the code so that the guestbookresponse_id is written to the fileaccessrequests table:
@ManyToMany
@JoinTable(name = "fileaccessrequests",
joinColumns = @JoinColumn(name = "datafile_id"),
inverseJoinColumns = @JoinColumn(name = "authenticated_user_id"))
  1. If 1. isn't possible, is the FileAccessRequest (with possibly a corresponding FileAccessRequestServiceBean) the right way to go?

I hope my explanation and questions are understandable (maybe?)...

@scolapasta
Copy link
Contributor

Ah, I understand now. And yes, I had forgotten that the fileaccessrequest table was generated directly via the @manytomany, i.e. there was no fileAccessRequest entity.

So, with all that, I think that you are correct, the way to move forward is to make it it's own entity and shift the relationships as you suggested above.

You also said, that you can't quite get it to work properly? How can we help? What specifically is not working there.

Another question: should the guestbookResponse relationship be one to one and not many to one? (i.e. one file can have many requests, as can one user, but a guestbookresponse should only be associated with at most one file request no?)

@stevenmce
Copy link

HI Gustavo,

Re " should the guestbookResponse relationship be one to one and not many to one?"

FWIW, it seems to me that Dataverse should probably look at moving towards having both dataSET requests, as well as dataFILE requests. (Given the move to dataSET level tools in other areas) So the one-to-one relationship probably isn't something you would want, as it would inhibit this.

(We in fact had a discussion with one of our data owners last week about this - they were interested in dataSET level reporting, and thus trying to figure out how to remove all the repeated dataFILE level guestbookResponses related to a single request for access to the dataSET).

Cheers,
Steve

@mdmADA
Copy link
Contributor Author

mdmADA commented Oct 10, 2019

Hi @scolapasta - In light of @stevenmce's comments regarding dataset-level requests, do you recommend going forward with the datafile-level request and visit the dataset-level requests in a future iteration? Or should dataset-level requests be considered/designed for in my current code?

Thanks!

@scolapasta
Copy link
Contributor

Actually, it's an interesting point. When we had this all in one table, and you downloaded a zip file, we had to have repeated guestbook entries because we wanted to count each file in the zip as one download. But since we've now separated, in fact have one guestbook be associated with several downloads.

@stevenmce's point was about fileaccessrequests which could be similar. if I request several files at the same time they could all be tied to one guestbookresponse. But you would have to change the relationship from guestbookresponse and download, as outlined above, as well, or you woudn't have a guestbookresponse to "attach" to each download as they happened.

The other challenge their is how to handle downloads at separate times. For example, I request files A and B, and fill out guestbookresponse 1. I am granted access, so I go and download File A. That download is associated with guestbookresponse 1. We'd still need to track that guestbookresponse 1 is the guestbook response to associate with a future download of File B.

I'd be perfectly happy with (and prefer) this approach, though of course it's more work and probably still requires some more details in the design.

I'm also going to ping @sekmiller and @landreev as they've had more direct experience working with guestbooks and downloads to make sure I'm not missing anything.

@mreekie mreekie added the bk2211 label Nov 1, 2022
@pdurbin
Copy link
Member

pdurbin commented Jan 3, 2023

@mdmADA hi, I just noticed that your 2043-alter-fileaccessrequests-tbl branch has been deleted. Are you still interested in getting this pull request merged? If so, we'll probably need a new branch.

@qqmyers
Copy link
Member

qqmyers commented Jan 3, 2023

I'm not sure this is needed with #5863 merged (could be wrong). FWIW - I'm working on an update of #2043 for QDR.

@mreekie mreekie removed the bk2211 label Jan 11, 2023
@pdurbin
Copy link
Member

pdurbin commented Jul 20, 2023

Closing in favor of this new PR:

@pdurbin pdurbin closed this Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants