feat: add video upload support for facebook

superfaceai / station

Comlink metadata for self-integrating applications.

https://superface.ai/catalog

MIT License

22 stars 5 forks source link

feat: add video upload support for facebook #282

Closed tmladek closed 1 year ago

tmladek commented 1 year ago

Description

Adds support for Facebook video uploads

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] I have read the CONTRIBUTING document.
[x] I haven't repeated the code. (DRY)
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

tmladek commented 1 year ago

Known issues: attachments is at the moment always presumed to be a list of Video IDs, triggering special handling (updating the video with published=true instead of creating an actual post). There's nothing about the ID that could be used to tell whether it's a video or potentially a photo.

add a type param to MediaAttachment?
change attachments to videos (as that's the only use right now anyway?)
GET various endpoints to see which one returns without error, giving info about the type of the item?
?

tmladek commented 1 year ago

However, I am not sure how it will affect usage with Twitter (for images) and LinkedIn (for documents).

Right - well, I suppose we'd have to add (yet) another attribute for that (documents, images)... So I'd rather vote for adding a type field?

jnv commented 1 year ago

Right - well, I suppose we'd have to add (yet) another attribute for that (documents, images)... So I'd rather vote for adding a type field?

That'd make it harder to use. I vote for videos with type of [string] and if we need to support more types, backward compat would be trivial.

tmladek commented 1 year ago

Okay, one last thing I noticed that I want to point out before I change it - we already use attachments with types in station, even in the same category of profiles: https://github.com/superfaceai/station/blob/main/grid/social-media/posts/profile.supr#L78

jnv commented 1 year ago

Okay, one last thing I noticed that I want to point out before I change it - we already use attachments with types in station, even in the same category of profiles: https://github.com/superfaceai/station/blob/main/grid/social-media/posts/profile.supr#L78

That's good point, but I think it's a different use case (heh). That structure is for reading the data, here we optimize for writing / input. And you won't be feeding that structure from one profile to the other. In fact, having a similar structure under the same field name can be confusing.