search

editor-js / attaches

File attachments Block for the Editor.js
MIT License
64 stars 66 forks source link

Backend response fields not optional #15

Closed msrdic closed 2 years ago

msrdic commented 4 years ago

In the section about backend response format, the text says

Can contain data you want to store.

I've implemented a simple upload handler which returns only { success : 1 } response, but the component fails with the message 

File upload failed

The same thing happens when I add an empty file: { } section to response.

If I add the url field inside file object, the error changes to

Cannot read property 'split' of undefined

The message is the same if I add the title field inside the file object.

Once I added the name field into the file object, everything works as expected.

Note that in all these instances, the actual upload works – the file is properly stored inside the dedicated uploads folder.

I am using the latest dist version downloaded from the master branch today.

The way I see it, either docs need to reflect that name is a required response field inside the file object, or it needs to default to some name if one isn't returned.

Happy to supply more details if needed and thank you for all your work on this.

msrdic commented 4 years ago

I actually did investigate and found where the split() is being called. It's in a place where extension is being extracted from the name that is returned in the response. I forked the repository, but since I see no tests and I am not a JS developer, I don't feel confident enough submitting the pull request or making a design decision regarding this.

polyrainbow commented 3 years ago

In addition to this, optional fields from the backend response are not saved at all.

The attach plugin only takes the fields url, name and size into account, but not any others [1]. In comparison, the image plugin copies the whole backend response object instead [2].

[1] https://github.com/editor-js/attaches/blob/0e246b8f6899b7bd4380147a1ec73df22be1dff1/src/index.js#L256 [2] https://github.com/editor-js/image/blob/4ccb865d876d87edfa60f620136984eff73ebc12/src/index.js#L346

MakotoTatsumi commented 3 years ago

I am facing the same problem and it would be helpful if fix it

polyrainbow commented 3 years ago

I have fixed this in my fork along with some other stuff to make this plugin a bit more aligned with @editorjs/image. But I'm not sure if it meets the quality standards of this codebase. Anyway, feel free to use this to create a PR or merge if you like.