-
-
Notifications
You must be signed in to change notification settings - Fork 39
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
feat: add private download feature #359
Conversation
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.
Can you please add some tests? Thank you
And thanks for the PR!
:download="`/nova-vendor/nova-file-manager/files/download?path=${file?.path}`" | ||
:href="`/nova-vendor/nova-file-manager/files/download?path=${file?.path}`" |
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 move these to an attribute, e.g file.downloadUrl
should allow for a better centralization of data and easier refactoring/customization
* @param \Oneduo\NovaFileManager\Http\Requests\DownloadFileRequest $request | ||
* @return \Illuminate\Http\JsonResponse | ||
*/ | ||
public function download(DownloadFileRequest $request) |
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 add a type hit for the return of this function
$this->authorizationAttribute() => __('nova-file-manager::errors.authorization.unauthorized', | ||
['action' => $this->authorizationActionAttribute()]), | ||
$this->authorizationAttribute() => __( | ||
'nova-file-manager::errors.authorization.unauthorized', | ||
['action' => $this->authorizationActionAttribute()] | ||
), |
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.
unnecessary code style change, please keep PR feature-only
Thanks for the review! I will fix it later this week :) |
Bump! I'd like this feature. I needed to use a different file-manager package since previous one didn't support Nova 4... Do you need help @JulianMar ? Download works only if I move these files into |
Hi @JulianMar Thank you for this PR. However this has been rewrote in #420. Will be released soon. All the best. |
closes #358