ShokoAnime / ShokoServer

Repository for Shoko Server.
http://shokoanime.com/shoko-server/
MIT License
389 stars 75 forks source link

Add file/episode duration and resume position #944

Closed revam closed 2 years ago

revam commented 2 years ago

Because we want to render the percentage some places in Web UI.

Example from @ElementalCrisis's mock-ups; image

revam commented 2 years ago

Will merge late Sunday (local time) unless somebody wants something changed.

revam commented 2 years ago

Tried to touch as little as possible in the new files while adding the server model (and removing any references to the non-server model) for the user records.

revam commented 2 years ago

I did have one breaking change in the last rebase; the ResumePosition for the File was changed from a long to a TimeSpan. But afaik then no current or previous client have used the field to date.

bigretromike commented 2 years ago

are there any v3 endpoints that got broken backward compatible or old code should work without issue ?

revam commented 2 years ago

@bigretromike see my last comment/message. That's the only breaking change.

revam commented 2 years ago

Is Nakamori still using api v2?

revam commented 2 years ago

Is Nakamori still using api v2?

It's not used in the v3 data models used in Nakamori it seems, so I still don't think the field have ever been used before.

bigretromike commented 2 years ago

Is Nakamori still using api v2?

It's not used in the v3 data models used in Nakamori it seems, so I still don't think the field have ever been used before.

we did v3 some time ago. If those changes break things we will need to rewrite some code, thats why im asking. Then again, why not make another v3 version like it was done before. like v3.1 its a good thing to not break compatible.

revam commented 2 years ago

And i'm saying i know what I broke (and when it was introduced) and I am not familiar with any current or previous clients using the field within the last ~2 years since it was introduced. Also i will rather revert the type to a long than create another api sub version.

ElementalCrisis commented 2 years ago

Even though APIv3 has been in the works for a while, it's still in dev and subject to breaking changes. As Nakamori is currently using it, maybe @revam you can tag @bigretromike anytime APIv3 is updated with a breaking change so he stays informed?

bigretromike commented 2 years ago

Even though APIv3 has been in the works for a while, it's still in dev and subject to breaking changes. As Nakamori is currently using it, maybe @revam you can tag @bigretromike anytime APIv3 is updated with a breaking change so he stays informed?

That would be awesome - that or just add changelog with information that function is not backward compatible anymore.

@ElementalCrisis it's been in work as was v2 before, Shoko API won't ever be finished its always something that will evolve with rest of the code. You guys introduce sub api versions, and imo thats best place to put anything that otherwise break compatible - but im not forcing that anyway.

da3dsoul commented 2 years ago

Sure things are constantly changing, but we've done a really good job of not breaking apiv2

Cazzar commented 2 years ago

Api sub versions are nice but for a small change like this it is entirely overkill and not worth adding it. When the discussion was made about the potentially breaking change, we decided to accept that risk as APIv3 is as @ElementalCrisis mentioned, still in its “active development/unfinished” state unlike apiv1/apiv2

revam commented 2 years ago

Anything else? Or is it ok to merge this now?

da3dsoul commented 2 years ago

I think it's fine