Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ryot import implementation #563

Merged
merged 11 commits into from
Jun 28, 2024
Merged

Ryot import implementation #563

merged 11 commits into from
Jun 28, 2024

Conversation

oPisiti
Copy link
Contributor

@oPisiti oPisiti commented Jun 25, 2024

Changes made

  1. Created a new button on /import/ page for Ryot
  2. Added Ryot svg logo
  3. Created processRyotFile() function to correctly import the data of the json ryot export

@IRHM IRHM added the enhancement New feature or request label Jun 26, 2024
@IRHM IRHM assigned IRHM and oPisiti Jun 26, 2024
@IRHM IRHM linked an issue Jun 26, 2024 that may be closed by this pull request
@IRHM
Copy link
Member

IRHM commented Jun 26, 2024

Hey @oPisiti, I glanced over the changes and they look great, thank you for implementing this!

I will hopefully get to test the changes tonight or tomorrow, then I can get this merged.

Thanks again!

@IRHM
Copy link
Member

IRHM commented Jun 27, 2024

Hey @oPisiti, looks like some of the code wasn't formatted, I can format it for you if you'd like (feel free to do so yourself if you want to, we use prettier, running npm run format should format everything automatically)?

Also, if it would be alright, do you think you could attach a ryot export example that I could test with? I tried using their demo instance to export, but it doesn't seem to be working.

@oPisiti
Copy link
Contributor Author

oPisiti commented Jun 27, 2024

Hi @IRHM,

Below is the custom export I tailored to test the import function.
It contains:

  • Movies with different and multiple categories, in order to test the status ranked on line 425 of src/routes/(app)/import/+page.svelte. This rank is important to define the most meaningful tag, as Ryot allows multiple.
  • Movies with reviews
  • Series with multiple episodes watched and different progress levels
  • Series with no tags (which means dropped)
  • Series with episode reviews, which are not supported in watcharr (should be dropped without errors)
  • Other types of media, which should all raise errors: anime, manga, audiobook, visual novel

P.S.: In Ryot, there is overlap between anime and tv shows. As the anime section yields weird results when searching, I only used the tv category for my watched anime and it worked flawlessly and I imagine most people do the same. Also, it uses anilist identifiers. Therefore, I disconsidered "anime" as a source

RyotMedia - min.json

@IRHM
Copy link
Member

IRHM commented Jun 28, 2024

Hey @oPisiti, thank you very much for doing this, I just made some small tweaks, but other than that I am very happy to get this merged in!

I mostly just fixed some type errors, I will add comments to the code in the commit so you can review the changed I've made.

Kind of irrelevant to what this PR was for, but I also changed the Stats styling to push the values down instead:
From
image
To
image

I think that looks nicer than what we had before?

Thanks again!

@@ -406,7 +407,7 @@
// Build toImport array
const toImport: ImportedList[] = [];
const fileText = await readFile(new FileReader(), file);
const jsonData = JSON.parse(fileText)["media"] as Watched[];
const jsonData = JSON.parse(fileText)["media"] as any[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to any array, the data in the ryot export didnt conform to the Watched[] type

@@ -422,7 +423,7 @@

// Define the main general status of the movie/show
// In Ryot, it can be marked as multiple of the following, so choose the most relevant
const statusRanks = [
const statusRanks: [string, WatchedStatus][] = [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gave this a "tuple array" type so that it knows the second value is a WatchedStatus, fixed a type error below where we add status to the ImportedList object.

@@ -448,23 +449,23 @@
thoughts: v.lot === "movie" && v.reviews.length ? v.reviews[0].review.text : "",

// Ryot does not support overall rating for shows
rating: v.lot === "movie" && v.reviews.length ? Number(v.reviews[0].rating) : null,
rating: v.lot === "movie" && v.reviews.length ? Number(v.reviews[0].rating) : undefined,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed null usage to undefined to conform to the rating property type.

Irrelevant, but maybe informative if you care:
I usually try to avoid using null and use undefined instead. Since JS has null and undefined, they usually tell us the same thing, so I prefer to, in most cases, strictly use undefined instead (can't think of any upsides to null other than if our logic needs to differentiate between the two for some reason). Anyways that's just what I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, that makes sense.

JS is not my main language, so I was going with knowledge from a few years ago when I decided on null.
I will keep that in mind for future PRs


// Ignore fields with fewer than 1 unit
if (tmp) ansString += `${tmp} ${c[0]}${tmp >= 2 ? "s, " : ", "}`;
m -= tmp * c[1];
m -= tmp * (c[1] as number);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casted these to be numbers, I could've done the tuple array thing here too but i didn't think of that until now and its too late!!!!!! AAAAAAA

@@ -34,6 +34,7 @@ export type Icon =
| "eye"
| "star"
| "movary"
| "ryot"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added ryot to the Icon type (every icons name is included in this type).

@IRHM IRHM linked an issue Jun 28, 2024 that may be closed by this pull request
@IRHM IRHM merged commit 855b872 into sbondCo:dev Jun 28, 2024
1 check passed
truecharts-admin referenced this pull request in truecharts/public Jun 28, 2024
….0@2229366 by renovate (#23905)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ghcr.io/sbondco/watcharr](https://togithub.com/sbondCo/Watcharr) |
minor | `v1.40.0` -> `v1.41.0` |

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>sbondCo/Watcharr (ghcr.io/sbondco/watcharr)</summary>

###
[`v1.41.0`](https://togithub.com/sbondCo/Watcharr/releases/tag/v1.41.0)

[Compare
Source](https://togithub.com/sbondCo/Watcharr/compare/v1.40.0...v1.41.0)

#### 🧠 New

- Search: Infinite scrolling and serverside content type filters by
[@&#8203;IRHM](https://togithub.com/IRHM) in
[https://github.com/sbondCo/Watcharr/pull/562](https://togithub.com/sbondCo/Watcharr/pull/562)
- Search now supports scrolling through multiple pages of results,
making it possible to find content that may have been hidden in the
past.
- The search filters (Movies, TV Shows, Games, People) have been
reworked to filter through the server, this leads to better results and
more of them.
- Running search requests will now be cancelled if you navigate away
from the page or if you change your search query (helps a lot on slower
connections).
- Ryot import implementation by
[@&#8203;oPisiti](https://togithub.com/oPisiti) in
[https://github.com/sbondCo/Watcharr/pull/563](https://togithub.com/sbondCo/Watcharr/pull/563)

#### 💯 Changed

- Time unit changes by [@&#8203;oPisiti](https://togithub.com/oPisiti)
in
[https://github.com/sbondCo/Watcharr/pull/563](https://togithub.com/sbondCo/Watcharr/pull/563)
    -   Movie/show runtimes are now printed as `24 min` instead of `24m`
- Time spent watching stats can now show `months, weeks, days, hours &
minutes`, instead of just the previous `hours and minutes`.

#### 🏗️ Fixed

- Fix server side rating validation for imports, watched episodes and
watched seasons in
sbondCo/Watcharr@7888e04

#### 🔨 Maintenance

- server: bump golang.org/x/crypto from 0.23.0 to 0.24.0 in /server by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/sbondCo/Watcharr/pull/539](https://togithub.com/sbondCo/Watcharr/pull/539)
- ui: bump prettier from 3.2.5 to 3.3.2 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/sbondCo/Watcharr/pull/549](https://togithub.com/sbondCo/Watcharr/pull/549)
- server: bump gorm.io/driver/sqlite from 1.5.5 to 1.5.6 in /server by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/sbondCo/Watcharr/pull/546](https://togithub.com/sbondCo/Watcharr/pull/546)
- workflow: bump docker/build-push-action from 5 to 6 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/sbondCo/Watcharr/pull/545](https://togithub.com/sbondCo/Watcharr/pull/545)
- ui: bump
[@&#8203;typescript-eslint/eslint-plugin](https://togithub.com/typescript-eslint/eslint-plugin)
from 7.12.0 to 7.14.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/sbondCo/Watcharr/pull/560](https://togithub.com/sbondCo/Watcharr/pull/560)
- ui: bump svelte-eslint-parser from 0.36.0 to 0.39.2 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/sbondCo/Watcharr/pull/559](https://togithub.com/sbondCo/Watcharr/pull/559)
- ui: bump eslint-plugin-svelte from 2.39.0 to 2.41.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/sbondCo/Watcharr/pull/558](https://togithub.com/sbondCo/Watcharr/pull/558)
- server: bump github.com/go-co-op/gocron/v2 from 2.5.0 to 2.7.0 in
/server by [@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/sbondCo/Watcharr/pull/557](https://togithub.com/sbondCo/Watcharr/pull/557)

#### 🥇 Credit

- Thanks to [@&#8203;oPisiti](https://togithub.com/oPisiti) for making
their first (and second) contribution in
[https://github.com/sbondCo/Watcharr/pull/563](https://togithub.com/sbondCo/Watcharr/pull/563)
- Thanks to [@&#8203;simonbcn](https://togithub.com/simonbcn) for
reporting an issue that led to the search improvements being made in
[https://github.com/sbondCo/Watcharr/issues/531](https://togithub.com/sbondCo/Watcharr/issues/531)

#### 🆘 Getting Help

If you need help, encounter an issue or find a bug please [create an
issue](https://togithub.com/sbondCo/Watcharr/issues/new/choose) or [join
our space on Matrix](https://matrix.to/#/#watcharr:matrix.org) for
support. Always happy to help!

I spent a lot of time trying to make sure no bugs were introduced with
the new search features, please don't hesitate to reach out if you think
someone is not working as intended!

Package:
https://github.com/orgs/sbondCo/packages/container/watcharr/236341139?tag=v1.41.0
or [on docker
hub](https://hub.docker.com/layers/sbondco/watcharr/v1.41.0/images/sha256-2229366989b906418c17df5a54c0c6d378f6044f45c591bba81a6cdd25e30a96?context=explore)
**Full Changelog**:
sbondCo/Watcharr@v1.40.0...v1.41.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MjAuMSIsInVwZGF0ZWRJblZlciI6IjM3LjQyMC4xIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbImF1dG9tZXJnZSIsInVwZGF0ZS9kb2NrZXIvZ2VuZXJhbC9ub24tbWFqb3IiXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Better units [Feature] Import from Ryot
2 participants