Fix viewing of file versions #1270
Conversation
Fixes the issue reported at nextcloud#1269 When trying to look at older versions of a pdf file, there was always an error as this.file was undefined. By adding a check for this to isDownloadable() this starts to work without an issue Signed-off-by: Lukas Obermann <[email protected]>
lobermann
force-pushed
the
fix_version_viewing
branch
from
0704284 to
337be57
Compare
|
/backport to stable32 |
|
/backport to stable31 |
|
I found an additional issue that I fixed in the second commit. |
… current version in the version history Signed-off-by: Lukas Obermann <[email protected]>
lobermann
force-pushed
the
fix_version_viewing
branch
from
99bfca2 to
2b898a1
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
There was a problem hiding this comment.
Sorry for the long delay, and thanks a lot for your contribution!
While reviewing it I noticed some further errors in the console when viewing file versions. Those errors are not caused by the changes here, but seem to be "unveiled" by them. I will investigate them further and see if I can fix them here too, or if they should be fixed later in another pull request.
Independently of that, I left some comments that it would be great if you could address. But if you prefer me to change that please let me know, that would be totally fine too! Thanks :-)
| isDownloadable() { | ||
| if (!this.file.shareAttributes) { | ||
| if (!this.file || !this.file.shareAttributes) { |
There was a problem hiding this comment.
I would suggest to use instead the optional chanining operator:
if (!this.file?.shareAttributes) {
as it is shorter and also consistent with other usages (see, for example, isEditable()).
The optional chaining operator should be applied too in hideDownload above.
| return downloadPermissions.value | ||
| } | ||
| return true | ||
| } catch (e) { |
There was a problem hiding this comment.
The try { } catch should enclose only the const shareAttributes = JSON.parse(this.file.shareAttributes) statement, as it is guarding against a parsing error, and the rest of statements should be safe to execute (at least if the function returns early in the catch like it does now).
3 participants
Fixes the issue reported at #1269
When trying to look at older versions of a pdf file, there was always an error as this.file was undefined.
By adding a check for this to isDownloadable() this starts to work without an issue