fix: Allow disabling file name sanitization #1586
Conversation
taratatach
force-pushed
the
feat/add-file-creation-option-to-disable-sanitization
branch
from
409dce9 to
194c053
Compare
Some clients, like Cozy Desktop, do their own file name sanitization and don't expect `cozy-client` to do it for them. In fact, this sanitization would lead to issues down the road. To prevent these, we add a `sanitizeName` option to `FileCollection`'s methods to disable `cozy-client`'s sanitization. The option's value will be `true` by default so other clients don't have anything to do.
taratatach
force-pushed
the
feat/add-file-creation-option-to-disable-sanitization
branch
from
194c053 to
c35efeb
Compare
| * @returns {Promise} - Resolves when references have been removed | ||
| * and file has been sent to trash | ||
| */ | ||
| async destroy(file, { ifMatch = '' } = {}) { | ||
| const { _id, relationships } = file | ||
| const fileId = file.id || file._id |
There was a problem hiding this comment.
quite a nit, but _id is the preferred format, so I would reverse: file._id || file.id , so file._id is evaluated first (and skip file._id if it's true)
There was a problem hiding this comment.
Good catch. I can't remember why this change is part of this PR. It doesn't seem necessary.
I'll see if I can remove it otherwise I'll make the change you suggested.
| ? undefined | ||
| : sanitizeName | ||
| ? sanitizeAndValidateFileName(name) | ||
| : name, |
There was a problem hiding this comment.
I'm highly convinced that nested ternaries are a reading and understanding hell 😄
There was a problem hiding this comment.
Ah. I quite like them as you can see 😅. I'll go with 2 assignments then.
There was a problem hiding this comment.
We all have deviant sides, who am I to judge...
| * @returns {Promise<object>} - A promise that returns the copied file if resolved. | ||
| * @throws {FetchError} | ||
| * | ||
| */ | ||
| async copy(id, name, dirId) { | ||
| async copy(id, name, dirId, { sanitizeName = true } = {}) { |
There was a problem hiding this comment.
An alternative would have been to pass a function rather than a bool, like:
{ sanitizeFct = sanitizeAndValidateFileName } = {}
And then:
if (sanitizeFct) { sanitizeFct(name) }
Not a big deal but I find it a bit more elegant than using a boolean
There was a problem hiding this comment.
It would open up some use cases but I don't find copy(id, name, dirId, { sanitizeFct: undefined }) that elegant.
There was a problem hiding this comment.
Maybe sanitizeFct: () => {} would be better ? 😄
Anyways, I'm ok with both solutions, it's a matter of preference, so not a requirement to me
Some clients, like Cozy Desktop, do their own file name sanitization
and don't expect
cozy-clientto do it for them. In fact, thissanitization would lead to issues down the road.
To prevent these, we add a
sanitizeNameoption toFileCollection'smethods to disable
cozy-client's sanitization.The option's value will be
trueby default so other clients don'thave anything to do.