nuxt migration -file upload - WIP #271
Conversation
WalkthroughThe recent updates involve refactoring the file upload functionality across different Vue components by centralizing the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (4)
- client/components/forms/FileInput.vue (3 hunks)
- client/components/forms/ImageInput.vue (2 hunks)
- client/lib/file-uploads.js (1 hunks)
- client/lib/utils.js (1 hunks)
Additional comments: 7
client/lib/file-uploads.js (3)
4-15: For local uploads, the function uses
useOpnApiwith a hard-coded endpoint/upload-file. Ensure that this endpoint is correct and that the server-side API is expecting theContent-Typeto be set tomultipart/form-datawithout a boundary. Some server-side implementations might require the boundary to be specified.18-56: For S3 uploads, the function dynamically constructs the request options and headers. It's important to ensure that the
Hostheader is not included in the S3 pre-signed URL request, as it is removed on lines 33-35. This is a good security practice to prevent Host header attacks.43-52: The use of
axios.create()to remove default headers is a good practice to avoid sending unnecessary or sensitive information. However, ensure that thecancelTokenis properly implemented and tested, as it is crucial for the cancellation feature to work correctly.client/components/forms/FileInput.vue (2)
82-82: The import statement for
storeFileis correct and follows best practices for modularity by using a centralized file upload function.195-204: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [189-200]
The
uploadFileToServermethod has been refactored to use thestoreFilefunction. Ensure that theopnFetchcall on line 196 is correctly updated to handle the new file upload process, and that the server-side endpoint is expecting thebodystructure provided.client/components/forms/ImageInput.vue (2)
113-113: The import statement for
storeFileis correct and follows best practices for modularity by using a centralized file upload function.191-197: The
uploadFileToServermethod has been refactored to use thestoreFilefunction. Ensure that theopnFetchcall on line 193 is correctly updated to handle the new file upload process, and that the server-side endpoint is expecting thebodystructure provided.
client/lib/utils.js
Outdated
| export const asset = (path) => { | ||
| const assetUrl = import.meta.env.VITE_VAPOR_ASSET_URL ? import.meta.env.VITE_VAPOR_ASSET_URL : '' | ||
| return assetUrl + '/' + path | ||
| } |
There was a problem hiding this comment.
The asset function correctly constructs a URL using the VITE_VAPOR_ASSET_URL environment variable. However, it does not handle the case where the path parameter might already start with a /, which would result in a double slash in the URL. Consider trimming any leading slash from the path parameter before concatenation.
- return assetUrl + '/' + path
+ return assetUrl + '/' + path.replace(/^\//, '');Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| export const asset = (path) => { | |
| const assetUrl = import.meta.env.VITE_VAPOR_ASSET_URL ? import.meta.env.VITE_VAPOR_ASSET_URL : '' | |
| return assetUrl + '/' + path | |
| } | |
| export const asset = (path) => { | |
| const assetUrl = import.meta.env.VITE_VAPOR_ASSET_URL ? import.meta.env.VITE_VAPOR_ASSET_URL : '' | |
| return assetUrl + '/' + path.replace(/^\//, ''); | |
| } |
| export const storeFile = async (file, options = {}) => { | ||
| if(!opnformConfig.s3_enabled) { // If not s3 then upload to local temp | ||
| let formData = new FormData() | ||
| formData.append('file', file) | ||
| const response = await useOpnApi('/upload-file', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'multipart/form-data' | ||
| }, | ||
| body: formData | ||
| }) | ||
| response.data.extension = file.name.split('.').pop() | ||
| return response.data | ||
| } | ||
|
|
||
| const response = await useOpnApi(options.signedStorageUrl ? options.signedStorageUrl : '/vapor/signed-storage-url', { | ||
| method: 'POST', | ||
| body: options.data, | ||
| bucket: options.bucket || '', | ||
| content_type: options.contentType || file.type, | ||
| expires: options.expires || '', | ||
| visibility: options.visibility || '', | ||
| baseURL: options.baseURL || null, | ||
| headers: options.headers || {}, | ||
| ...options.options | ||
| }) | ||
| console.log("response.data",response.data) | ||
|
|
||
| const headers = response.data.headers | ||
|
|
||
| if ('Host' in headers) { | ||
| delete headers.Host | ||
| } | ||
|
|
||
| if (typeof options.progress === 'undefined') { | ||
| options.progress = () => {} | ||
| } | ||
|
|
||
| const cancelToken = options.cancelToken || '' | ||
|
|
||
| // Remove authorization headers | ||
| const cleanAxios = axios.create() | ||
| cleanAxios.defaults.headers.common = {} | ||
| await cleanAxios.put(response.data.url, file, { | ||
| cancelToken: cancelToken, | ||
| headers: headers, | ||
| onUploadProgress: (progressEvent) => { | ||
| options.progress(progressEvent.loaded / progressEvent.total) | ||
| } | ||
| }) | ||
|
|
||
| response.data.extension = file.name.split('.').pop() | ||
|
|
||
| return response.data | ||
| } |
There was a problem hiding this comment.
The storeFile function appears to handle both local and S3 uploads. However, there is a console log statement on line 29 that should be removed to prevent potential leakage of sensitive information in production logs.
- console.log("response.data",response.data)Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| export const storeFile = async (file, options = {}) => { | |
| if(!opnformConfig.s3_enabled) { // If not s3 then upload to local temp | |
| let formData = new FormData() | |
| formData.append('file', file) | |
| const response = await useOpnApi('/upload-file', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'multipart/form-data' | |
| }, | |
| body: formData | |
| }) | |
| response.data.extension = file.name.split('.').pop() | |
| return response.data | |
| } | |
| const response = await useOpnApi(options.signedStorageUrl ? options.signedStorageUrl : '/vapor/signed-storage-url', { | |
| method: 'POST', | |
| body: options.data, | |
| bucket: options.bucket || '', | |
| content_type: options.contentType || file.type, | |
| expires: options.expires || '', | |
| visibility: options.visibility || '', | |
| baseURL: options.baseURL || null, | |
| headers: options.headers || {}, | |
| ...options.options | |
| }) | |
| console.log("response.data",response.data) | |
| const headers = response.data.headers | |
| if ('Host' in headers) { | |
| delete headers.Host | |
| } | |
| if (typeof options.progress === 'undefined') { | |
| options.progress = () => {} | |
| } | |
| const cancelToken = options.cancelToken || '' | |
| // Remove authorization headers | |
| const cleanAxios = axios.create() | |
| cleanAxios.defaults.headers.common = {} | |
| await cleanAxios.put(response.data.url, file, { | |
| cancelToken: cancelToken, | |
| headers: headers, | |
| onUploadProgress: (progressEvent) => { | |
| options.progress(progressEvent.loaded / progressEvent.total) | |
| } | |
| }) | |
| response.data.extension = file.name.split('.').pop() | |
| return response.data | |
| } | |
| export const storeFile = async (file, options = {}) => { | |
| if(!opnformConfig.s3_enabled) { // If not s3 then upload to local temp | |
| let formData = new FormData() | |
| formData.append('file', file) | |
| const response = await useOpnApi('/upload-file', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'multipart/form-data' | |
| }, | |
| body: formData | |
| }) | |
| response.data.extension = file.name.split('.').pop() | |
| return response.data | |
| } | |
| const response = await useOpnApi(options.signedStorageUrl ? options.signedStorageUrl : '/vapor/signed-storage-url', { | |
| method: 'POST', | |
| body: options.data, | |
| bucket: options.bucket || '', | |
| content_type: options.contentType || file.type, | |
| expires: options.expires || '', | |
| visibility: options.visibility || '', | |
| baseURL: options.baseURL || null, | |
| headers: options.headers || {}, | |
| ...options.options | |
| }) | |
| const headers = response.data.headers | |
| if ('Host' in headers) { | |
| delete headers.Host | |
| } | |
| if (typeof options.progress === 'undefined') { | |
| options.progress = () => {} | |
| } | |
| const cancelToken = options.cancelToken || '' | |
| // Remove authorization headers | |
| const cleanAxios = axios.create() | |
| cleanAxios.defaults.headers.common = {} | |
| await cleanAxios.put(response.data.url, file, { | |
| cancelToken: cancelToken, | |
| headers: headers, | |
| onUploadProgress: (progressEvent) => { | |
| options.progress(progressEvent.loaded / progressEvent.total) | |
| } | |
| }) | |
| response.data.extension = file.name.split('.').pop() | |
| return response.data | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (2)
- client/components/forms/FileInput.vue (3 hunks)
- client/components/forms/ImageInput.vue (2 hunks)
Additional comments: 4
client/components/forms/FileInput.vue (2)
82-82: The import of
storeFilefrom~/lib/file-uploads.jsis correctly placed at the top of the script section, which is a common convention in Vue components.196-205: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [190-202]
The
uploadFileToServermethod has been refactored to use thestoreFilefunction. Ensure that thestoreFilefunction is thoroughly tested, especially since it's now a centralized method used across different components. Also, verify that theopnFetchmethod is correctly updated to handle the newbodystructure and that the backend endpoint is expecting this new payload format.client/components/forms/ImageInput.vue (2)
113-113: The import of
storeFilefrom~/lib/file-uploads.jsis correctly placed and follows the standard convention in Vue components.191-197: The
uploadFileToServermethod has been updated to use thestoreFilefunction. Similar toFileInput.vue, ensure thatstoreFileis well-tested and that the backend is prepared to handle the new request body format in theopnFetchcall.
| body: { | ||
| type: 'files', | ||
| url: file.name.split('.').slice(0, -1).join('.') + '_' + response.uuid + '.' + response.extension | ||
| } |
There was a problem hiding this comment.
The construction of the url property in the body object seems to be manually concatenating parts of the file name and the response from storeFile. Consider creating a utility function to handle this logic, which would improve maintainability and reduce the risk of errors in URL construction.
- url: file.name.split('.').slice(0, -1).join('.') + '_' + response.uuid + '.' + response.extension
+ url: constructFileUrl(file.name, response.uuid, response.extension)Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| body: { | |
| type: 'files', | |
| url: file.name.split('.').slice(0, -1).join('.') + '_' + response.uuid + '.' + response.extension | |
| } | |
| body: { | |
| type: 'files', | |
| url: constructFileUrl(file.name, response.uuid, response.extension) | |
| } |
| body: { | ||
| url: this.file.name.split('.').slice(0, -1).join('.') + '_' + response.uuid + '.' + response.extension | ||
| } |
There was a problem hiding this comment.
The manual construction of the url property in the body object is repeated here as in FileInput.vue. It would be beneficial to abstract this logic into a utility function for better code reuse and maintainability.
- url: this.file.name.split('.').slice(0, -1).join('.') + '_' + response.uuid + '.' + response.extension
+ url: constructFileUrl(this.file.name, response.uuid, response.extension)Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| body: { | |
| url: this.file.name.split('.').slice(0, -1).join('.') + '_' + response.uuid + '.' + response.extension | |
| } | |
| body: { | |
| url: constructFileUrl(this.file.name, response.uuid, response.extension) | |
| } |
* wip * Managed to load a page * Stuck at changing routes * Fixed the router, and editable div * WIP * Fix app loader * WIP * Fix check-auth middleware * Started to refactor input components * WIP * Added select input, v-click-outside for vselect * update vselect & phone input * Fixed the mixin * input component updates * Fix signature input import * input component updates in vue3 * image input in vue3 * small fixes * fix useFormInput watcher * scale input in vue3 * Vue3: migrating from vuex to Pinia (#249) * Vue3: migrating from vuex to Pinia * toggle input fixes * update configureCompat --------- Co-authored-by: Forms Dev <[email protected]> * support vue3 query builder * Refactor inpus * fix: Vue3 Query Builder - Logic Editor (#251) * support vue3 query builder * upgrade * remove local from middleware * Submission table pagination & migrate chart to vue3 (#254) * Submission table Pagination in background * migrate chart to vue3 * Form submissions pagination * Form submissions * Fix form starts * Fix openSelect key issue --------- Co-authored-by: Forms Dev <[email protected]> Co-authored-by: Julien Nahum <[email protected]> * Vue 3 better animation (#257) * vue-3-better-animation * Working on migration to vueuse/motion * Form sidebar animations * Clean code * Added animations for modal * Finished implementing better animations --------- Co-authored-by: Forms Dev <[email protected]> * Work in progress * Migrating amplitude and crisp plugin/composable * Started to refactor pages * WIP * vue3-scroll-shadow-fixes (#260) * WIP * WIP * WIP * Figured out auth & middlewares * WI * Refactoring stores and templates pages to comp. api * Finishing the templates pages * fix collapsible * Finish reworking most templates pages * Reworked workspaces store * Working on home page and modal * Fix dropdown * Fix modal * Fixed form creation * Fixed most of the form/show pages * Updated cors dependency * fix custom domain warning * NuxtLink migration (#262) Co-authored-by: Forms Dev <[email protected]> * Tiny fixes + start pre-rendering * migrate-to-nuxt-useappconfig (#263) * migrate-to-nuxt-useappconfig * defineAppConfig --------- Co-authored-by: Forms Dev <[email protected]> * Working on form/show and editor * Globally import form inputs to fix resolve * Remove vform - working on form public page * Remove initform mixin * Work in progress for form create guess user * Nuxt Migration notifications (#265) * Nuxt Migration notifications * @input to @update:model-value * change field type fixes * @update:model-value * Enable form-block-logic-editor * vue-confetti migration * PR request changes * useAlert in setup * Migrate to nuxt settings page AND remove axios (#266) * Settings pages migration * remove axios and use opnFetch * Make created form reactive (#267) * Remove verify pages and axios lib --------- Co-authored-by: Julien Nahum <[email protected]> * Fix alert styling + bug fixes and cleaning * Refactor notifications + add shadow * Fix vselect issue * Working on page pre-rendering * Created NotionPages store * Added sitemap on nuxt side * Sitemap done, working on aws amplify * Adding missing module * Remove axios and commit backend changes to sitemap * Fix notifications * fix guestpage editor (#269) Co-authored-by: Julien Nahum <[email protected]> * Remove appconfig in favor of runtimeconfig * Fixed amplitude bugs, and added staging environment * Added amplify file * Change basdirectory amplify * Fix loading bar position * Fix custom redirect (#273) * Dirty form handling - nuxt migration (#272) * SEO meta nuxt migration (#274) * SEO meta nuxt migration * Polish seo metas, add defaults for OG and twitter --------- Co-authored-by: Julien Nahum <[email protected]> * migrate to nuxt useClipboard (#268) * Set middleware on pages (#278) * Se middleware on pages * Se middleware on account page * add robots.txt (#276) * 404 page migration (#277) * Templates pages migration (#275) * NuxtImg Migration (#279) Co-authored-by: Julien Nahum <[email protected]> * Update package json * Fix build script * Add loglevel param * Disable page pre-rendering * Attempt to allow svgs * Fix SVGs with NuxtImage * Add .env file at AWS build time * tRGIGGER deploy * Fix issue * ANother attrempt * Fix typo * Fix env? * Attempt to simplify build * Enable swr caching instead of prerenderign * Better image compression * Last attempt at nuxt images efficiency * Improve image optimization again * Remove NuxtImg for non asset files * Restore templates pages cache * Remove useless images + fix templates show page * image optimization caching + fix hydratation issue form template page * URL generation (front&back) + fixed authJWT for SSR * Fix composable issue * Fix form share page * Embeddable form as a nuxt middleware * Fix URL for embeddable middleware * Debugging embeddable on amplify * Add custom domain support * No follow for non-production env * Fix sentry nuxt and custom domain redirect * remove api prefix from routes (#280) * remove api prefix from routes * PR changes --------- Co-authored-by: Julien Nahum <[email protected]> * nuxt migration -file upload - WIP (#271) Co-authored-by: Julien Nahum <[email protected]> * Fix local file upload * Fix file submissions preview * API redirect to back-end from nuxt * API redirect to back-end from nuxt * Remove old JS app, update deploy script * Fix tests, added gh action nuxt step * Updated package-lock.json * Setup node in GH Nuxt action * Setup client directory for GH workflow --------- Co-authored-by: Forms Dev <[email protected]> Co-authored-by: Chirag Chhatrala <[email protected]> Co-authored-by: Rishi Raj Jain <[email protected]> Co-authored-by: formsdev <[email protected]>
Summary by CodeRabbit
New Features
storeFilefunction for handling file uploads with support for local and S3 storage.Enhancements
storeFileupload functionality.assetutility function for generating asset URLs with environment-specific base paths.Refactor