Skip to content

Serialize work type ID#165

Merged
Neraste merged 4 commits intodevelopfrom
work-types-serialize-id
Mar 6, 2025
Merged

Serialize work type ID#165
Neraste merged 4 commits intodevelopfrom
work-types-serialize-id

Conversation

@odrling
Copy link
Contributor

@odrling odrling commented Jul 23, 2024

Serialize the ID of work types to be able to use the /api/library/work-types/{id} route, otherwise the ID has to be guessed.

Found while trying to implement feeding in Japan7/karaberus#38

Serialize the ID of work types to be able to use the
`/api/library/work-types/{id}` route, otherwise the ID has to be guessed.

Found while trying to implement feeding in Japan7/karaberus#38
@codecov
Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.61%. Comparing base (c7039ef) to head (922b137).
Report is 5 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #165   +/-   ##
========================================
  Coverage    98.61%   98.61%           
========================================
  Files           78       78           
  Lines         1876     1876           
========================================
  Hits          1850     1850           
  Misses          26       26           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

odrling added a commit to Japan7/karaberus that referenced this pull request Jul 25, 2024
Feed tags/medias/artists/karas to dakara on upload and on startup.
Define tag colors (hue) so we can set colors in karaberus and have it
change in dakara at the same time.

Could be obviously improved by checking locally for changes of karaokes,
medias and artists instead of just PUTing everything on sync. Probably
not worth it implement that for tags. We can't yet update work types
decently because dakara doesn't return their IDs.
(DakaraProject/dakara-server#165)

This assumes that video files will all be called `{id}.mkv`, so the
downloader will have to follow this naming scheme to work with dakara.
The files are not all matroska files, but it should not be a problem in
practice.

#40
odrling added a commit to Japan7/karaberus that referenced this pull request Jul 25, 2024
Feed tags/medias/artists/karas to dakara on upload and on startup.
Define tag colors (hue) so we can set colors in karaberus and have it
change in dakara at the same time.

Could be obviously improved by checking locally for changes of karaokes,
medias and artists instead of just PUTing everything on sync. Probably
not worth it implement that for tags. We can't yet update work types
decently because dakara doesn't return their IDs.
(DakaraProject/dakara-server#165)

This assumes that video files will all be called `{id}.mkv`, so the
downloader will have to follow this naming scheme to work with dakara.
The files are not all matroska files, but it should not be a problem in
practice.

#40
@Neraste
Copy link
Member

Neraste commented Jul 28, 2024

Please update test helper here.

@odrling
Copy link
Contributor Author

odrling commented Jul 28, 2024

I can't seem to get the tests to pass, id doesn't seem to be defined on WorkType and I couldn't understand why.

@Neraste
Copy link
Member

Neraste commented Jul 29, 2024

Ah, the helper is also used to assert work type obtained by other serializers (namely, WorkTypeForWorkSerializer) that don't include the ID. So either you check for the ID in the appropriate tests, or you tweak the helper to check the ID only when needed (the former would be simpler).

@Neraste
Copy link
Member

Neraste commented Mar 5, 2025

Ping

@odrling
Copy link
Contributor Author

odrling commented Mar 5, 2025

Oh my bad.

Ah, the helper is also used to assert work type obtained by other serializers (namely, WorkTypeForWorkSerializer) that don't include the ID. So either you check for the ID in the appropriate tests, or you tweak the helper to check the ID only when needed (the former would be simpler).

I think I’d rather add the ID to this serializer too, doesn’t make much sense to leave it out. And it would be trivial to update the helper then. Actually forgot I had already updated the helper so it just doesn’t have to be updated.

@Neraste
Copy link
Member

Neraste commented Mar 6, 2025

Good.

@Neraste Neraste merged commit 810487e into develop Mar 6, 2025
15 checks passed
@Neraste Neraste deleted the work-types-serialize-id branch March 6, 2025 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants