make file expiry for guest links configurable when creating guest links

[Balk-Z]

This PR is trying to implement the feature request in #515 by allowing to configure file expiration for guest links.

To sum up what has been done:

• refactor existing expiration for guest links/files to differentiate between file
• add additional drop down to guest link creation screen and table header in overview
• add database column to keep track of configured expiry (as what ever the drop down defines e.g. 7 days)
• translate database entry to a time frame for upload of files
• adjusted test minimally
• added database migration script to update existing databases

I hope this suffices and is what you had in mind with "I'd accept a PR that implemented the functionality that allowed the admin to specify the expiration time for guest uploads.".

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[Balk-Z]

I have read the CLA Document and I hereby sign the CLA

[mtlynch]

Cool, thanks for adding this!

This is a good direction.

I think we need to add test coverage for the new functionality and reuse existing logic for expiration times, but we're on the right track.

[Balk-Z]

@mtlynch This is a version using ExpirationTime instead of string. Some issues you mentioned still persist i.e. when adding new options in guestLinksNewGet .
The database now stores what ever option was selected as delta t from now, so for the current options 1,7,30,365. (everything else is the never expire option) Relevant functions for the database read/write are formatFileExpirationTime and parseFileDatetime.
I am not sure if this is much better to be honest and would appreciate some advice. I have not added additional tests yet as I am waiting for your opinion. (Also I am unsure how to properly add tests as parseFileDatetime depends on the execution time...)

[Balk-Z]

Alrighty @mtlynch. I went ahead and reused FileLifetime to accomplish what's suggested above. Pretty sure we are pretty close.
Couple of things/issues:

• To implement your database suggestion I had to convert the NULL into 'NULL' as I kept getting: Scan error on column index 6, name "file_expiration_time": converting NULL to string is unsupported
• I still am unsure of how to implement the tests properly due to the moving target (the frontend is still sending a timestamp of the selection to ensure additions to guestLinksNewGet are respected and work)
• Files uploaded from a guest link which specifies never as the file expiration don't show the same on the files overview (see screenshot). I am not sure how to convert FileLifetimeInfinite to NeverExpire to be able to test for it in the format function of files (formatExpiration for fileIndex/Info/EditGet)

Also this is what the guest URL overview would look like, maybe also not optimal but I am trying to retain the style of the other functions:

Interested in more of your brilliant suggestions :)

[mtlynch]

Cool, nice progress! I added a few notes for things that I think might be getting in the way.

[Balk-Z]

Heya @mtlynch! I think it's finally in an acceptable state. I had to add a helper function for the upload test as I was clueless how else to make it work, hope that is alright. (see: convertExpirationTimeToFileLifetime)
Additionally I had to add the "Never" option to the file life time friendly name, I aligned that in the one location it was used.

The only remaining issue from my point of view is the display of the file expiration time in the fileIndex/fileInfo views when the FileLifetime is infinite, as per my last comment and screenshots. Not sure how we could extend the formatExpiration functions used with out checking for [is greater than absurd value].
Hope you agree with this point of view :)

[mtlynch]

Cool, this looks pretty close!

We're down to just a few naming things and small refactorings.

[Balk-Z]

@mtlynch I have implemented all of your suggestions (I believe). Had to add a FileLifetime to some of the GuestUploadTests as we now require a FileLifetime of at least 1 day.

My previous point about the filesIndex/fileInfo views still remains, maybe you have an idea for that.

Lastly, as there is quite a few commits now, would you like me to squash all of the commits down into one (commit msg: adding guest file expiry option when creating guest links) once we are done here?

[mtlynch]

Cool, this looks great! Thanks so much for implementing this.

I have implemented all of your suggestions (I believe). Had to add a FileLifetime to some of the GuestUploadTests as we now require a FileLifetime of at least 1 day.

Nice, sounds good.

My previous point about the filesIndex/fileInfo views still remains, maybe you have an idea for that.

Sorry, I can't find your question about that. The guest link index looks good to me as-is. Was there something you thought we should change?

Lastly, as there is quite a few commits now, would you like me to squash all of the commits down into one (commit msg: adding guest file expiry option when creating guest links) once we are done here?

No, Github has squash and merge built in, so you should never have to manually squash commits. Also, it's helpful if you can avoid force-pushing as well, as that makes the change a bit harder to review.