Rename existing files #450
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ectly Will now rely on `FormData` methods for catching erroneous objects being passed in - it may be worth working towards a more robust mapper for future use, but for now, it is felt that this version is more than suitable for our needs.
Missed a log statement that is no longer required - this has been removed
This just makes my life easier - these are only useful for TS usage
- Added check for renaming in API endpoint as well as functionality to perform the rename
- Cleaned up some code within `DocumentComponent`, namely:
- made `el` property `readonly` - this was because I believed it was being overwritten somewhere (see next point)
- the `updateProgress` function wasn't firing correctly and couldn't find the element it was supposed to be linked to - this has been removed (although it now means the code isn't as DRY)
- Changed the `showerror` function in an attempt to make result consistent across calls
- Added input and button for document renaming and implemented logic
- as noted in the code - for some reason I wasn't able to modify the `original-name` data attribute within the component using `event.target` so I had to pass the button in directly - frankly, TypeScript appears to have dropped the ball somewhere on this - I would upgrade the version of TS, Babel, Webpack, etc. but I'm not insane enough to go down that rabbit-hole - this is deemed the lesser of two evils in my opinion.
- Updated `datum::File` to check file name for marking datum as changed - Removed rename within file post request and placed within a put request - Updated `DocumentComponent` as required - Removed debug statements - Modified `edit.tt` as required
|
Thanks - this broadly looks fine, although there is a lot of duplicated code between the new |
Closed
I'm not sure I can entirely see that—the only common code between the two seems to be where the file is created, and that's only (really) one statement. I will add that I may be completely wrong here, so I'll take another look anyway. I'll add the security checks and the checks for a "new" record for now (although, in theory, the fact it's a new file should be passed as a parameter in the same way the delete option is in the |
Also applied perltidy using current config and modified output accordingly
It appears that using `find_with_permission` on the `Fileval` gives a datum object. Due to the only reason you would be able to access the field in this manner for a rename would be if you already have permission, this is seen as an over-complication.
Also updated styling
Following feedback from UX testing, further changes complete and pushed
- Added old name to rename button event in order to revert the result on error - Exported `RenameEvent` to keep things DRY - Modified specifiers on multiple elements within `DocumentComponent` as events were being fired across the board rather than having required specificity - Removed error as this wasn't displaying correctly - Moved to async methods as IE11 is no longer in use to add clarity to code - Refactored out progress function and bound as required (DRY) - Discovered server is returning errors as strings, not as JSON objects (this will become a new ticket as a bug) so ensured that these are being parsed as required
- Added check for invalid characters on upload - Allowed specific special characters on upload (underscore and dash)
Any invalid characters will be replaced when the file is saved to the record, but it is felt that the error still remain regardless.
droberts-ctrlo
commented
Sep 24, 2024
src/frontend/components/form-group/input/lib/filenameChecker.ts
Outdated
Show resolved
Hide resolved
Also removed extra check that's not required on `Datum::File` after `set_value`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requires merging of #449 before merge
datum::Fileto check file name for marking datum as changedDocumentComponentas requirededit.ttas required