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

Can't edit series metadata #1491

Closed
5 tasks done
luevano opened this issue May 8, 2024 · 27 comments
Closed
5 tasks done

Can't edit series metadata #1491

luevano opened this issue May 8, 2024 · 27 comments

Comments

@luevano
Copy link

luevano commented May 8, 2024

Steps to reproduce

  1. Import new manga with valid series.json metadata.
  2. Try to modify a field, or add alternative titles.
  3. Nothing happens, no error messages.

Expected behavior

I should be able to add or modify metadata for a series. I'm able to do this for series that are imported with incorrect series.json that are populated by the first book metadata.

Actual behavior

Nothing happens.

Logs

I don't see any error messages, only some warnings such as:

WARN 2869 --- [o-25600-exec-26] o.s.web.servlet.PageNotFound : No mapping for GET /series/0FQEEQJRH6MEQ

Where 0FQEEQJRH6MEQ is the id for the series I'm trying to modify.

Komga version

1.11.1-master

Operating system

Arch linux 6.8.8-arch1-1

Installation method

other (please provide more details below)

Other details

I'm using the AUR package.

Acknowledgements

  • I have searched the existing issues (open AND closed) and this is a new ticket, NOT a duplicate or related to another open issue.
  • I have written a short but informative title.
  • I have checked the FAQ.
  • I have updated the app to the latest version.
  • I will fill out all of the requested information in this form.
@luevano luevano added the triage label May 8, 2024
@gotson
Copy link
Owner

gotson commented May 8, 2024

You can enable DEBUG logs to see what's happening. Alternatively, you can check what API call the browser is sending to the backend (in the network dev tools).

@luevano
Copy link
Author

luevano commented May 8, 2024

Disabling cache and restarting the page right on the series. I see the following (in order):

Error on console, without doing anything yet:

Firefox can’t establish a connection to the server at https://***/sse/v1/events.

While trying to add an alternative title I see the following logs, then nothing happens:

DEBUG 3385171 --- [        task-10] o.g.komga.interfaces.sse.SseController   : Publish SSE: 'ThumbnailSeriesAdded':ThumbnailSeriesSseDto(seriesId=0FQEEQJRH6MEQ, selected=true)

Any time that I hit "save changes" I see a PUT request with code 202.

If I try to modify a series that didn't import a series.json correctly it does update instantly and I see the following logs:

DEBUG 3385171 --- [        task-29] o.g.komga.interfaces.sse.SseController   : Publish SSE: 'ThumbnailSeriesAdded':ThumbnailSeriesSseDto(seriesId=0FSWNAG8SPR4S, selected=true)
DEBUG 3385171 --- [        task-28] o.g.komga.interfaces.sse.SseController   : Publish SSE: 'SeriesChanged':SeriesSseDto(seriesId=0FSWNAG8SPR4S, libraryId=0CFS1Z717CK2Z)

And I also see a PUT (202) request followed by a PATCH (204). This is the expected behavior.

Permissions are the same for both series I tried it on, only difference is that the series that was able to import the series.json doesn't let me update any field, that's the only thing I've noticed as of now.

@gotson
Copy link
Owner

gotson commented May 9, 2024

If you can't get SSE connectivity the webui will not get server updates, probably an incorrect reverse proxy configuration.

Any time that I hit "save changes" I see a PUT request with code 202.

is the API call successful ?

@luevano
Copy link
Author

luevano commented May 9, 2024

If you can't get SSE connectivity the webui will not get server updates, probably an incorrect reverse proxy configuration.

Any time that I hit "save changes" I see a PUT request with code 202.

is the API call successful ?

But as I stated, it works for other series. And I guess it's successful as I'm receiving the 202 status code, I don't see anything in the request itself, let me know what you're looking for here.

What would be a correct proxy configuration? I'm using nginx:

server {
    server_name ***;

    location / {
        proxy_pass http://127.0.0.1:25600;
        # proxy_http_version 1.1; # I commented this out to test things out as I don't use it for all my reverse proxies, but same thing on both cases

        proxy_set_header Host $host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Proto $scheme;

        proxy_read_timeout 600s;
        proxy_send_timeout 600s;
    }
    include /etc/nginx/ssl_cert.conf;
}

Where the ssl_cert.conf contains:

listen 443 ssl;
# listen [::]:443 ssl http2;
http2 on;
ssl_certificate /etc/letsencrypt/live/***/fullchain.pem; # managed by Certbot
ssl_certificate_key /etc/letsencrypt/live/***/privkey.pem; # managed by Certbot
include /etc/letsencrypt/options-ssl-nginx.conf; # managed by Certbot
ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem; # managed by Certbot

@gotson
Copy link
Owner

gotson commented May 9, 2024

What would be a correct proxy configuration? I'm using nginx:

i don't use NGinx.

Please provide your DEBUG logs with successful series and unsuccessful series updates.

@luevano
Copy link
Author

luevano commented May 9, 2024

I already posted the only 2 logs that show when I try to add an alternate title. Here is the full log (komga.log) since the shutdown with DEBUG enabled.

0CHJ4GYN6SHDM is the series I'm able to edit, among others.
0FZJ9VTEVW0KY is failing to edit, among others. You can notice I clicked it 3 times.

@gotson
Copy link
Owner

gotson commented May 10, 2024

it seems the log file does not contain the DEBUG log statements when you edit a series metadata. You should see log statements of the form Apply metadata for series

@luevano
Copy link
Author

luevano commented May 10, 2024

it seems the log file does not contain the DEBUG log statements when you edit a series metadata. You should see log statements of the form Apply metadata for series

I'm using LOGGING_LEVEL_ORG_GOTSON_KOMGA=DEBUG in /etc/komga.conf, according to https://komga.org/docs/faq/#using-docker (not using docker, but the env var does enable debug). Not sure why those you're mentioning don't show.

@luevano
Copy link
Author

luevano commented May 10, 2024

Another thing I'm noticing is I can't modify metadata for ongoing series that have correctly imported a series.json, but if the series has ended then I can.

^Not really, tried modifying the series.json for Ended and afterwards I still can't modify. The last thing I can think of is that maybe it's because it's a book with the same name as another one I already deleted. This shouldn't be the case because I assume there are unique IDs assigned to each book regardless of the name and all the updates are done on the ID.

^Also not it, not sure really, there is not much in the logs

@luevano
Copy link
Author

luevano commented May 10, 2024

The issue was that the book count is 0 by default for ongoing series with correctly imported series.json. I only found this by doing some requests via the swagger-ui:

PATCH request on metadata for a series (updated only to add a language):
image

If I tried to add language to a series, it just doesn't update and doesn't show any error:
image

If I manually add the 0, it does show the error:
image

But if I manually remove the 0 from the book count, then I'm able to update. So this issue is only when there is an explicit 0 in the metadata.

The 0 comes from the tool I'm using (I have control over it, I'll update the app and make it so no 0 is used), so I think that there should be a check for this somewhere when importing that just removes the 0 from the imported series.json.

And this is the reason it worked for incorrectly imported series.json or completed series.

@luevano
Copy link
Author

luevano commented May 10, 2024

Just tried it, Komga fails to import series.json if total_issues is missing or is null, so it has to be 0 for ongoing series and has to be handled so that it doesn't import as 0 (or that 0 is ok).

@gotson
Copy link
Owner

gotson commented May 13, 2024

series.json is a Mylar schema. What tool are you using to generate those ?

@luevano
Copy link
Author

luevano commented May 13, 2024

series.json is a Mylar schema. What tool are you using to generate those ?

Yeah I know, I'm taking reference from version 1.02, and total_issues cannot be null and cannot be missed (even tried those 2 options and komga fails to import).

I'm using a tool I forked (derived from mangal). The metadata comes directly from Anilist, and when the manga is still publishing it always returns a 0:

https://github.com/luevano/libmangal/blob/edb79c0e2a052f8c2fe60b467b9ad154673e1a2d/anilist_manga.go#L158

So given that Anilist returns 0 for publishing mangas, the 0 case should be handled as the null case is handled in Komga.

@gotson
Copy link
Owner

gotson commented May 13, 2024

putting 0 is wrong, you should put a number that is at least the current number of the issue

@luevano
Copy link
Author

luevano commented May 13, 2024

Why would it be wrong? it's what Anilist API gives, also it is parsed as 0 in Komga, but not recognized afterwards. If I put the current number of issues, I would have to update series.json on each chapter download.

@gotson
Copy link
Owner

gotson commented May 13, 2024

it's wrong because if there's at least 1 book, then the total number of issues cannot be 0…

@luevano
Copy link
Author

luevano commented May 13, 2024

it's wrong because if there's at least 1 book, then the total number of issues cannot be 0…

It's not about having 0 books, it's about signaling that the series is ongoing publication. Else Komga would fail while parsing the 0, no?

There are no checks for "current issues" > "total issues" currently

@gotson
Copy link
Owner

gotson commented May 13, 2024

0 means 0 books. The Mylar schema does not enforce a positive number on that.

@luevano
Copy link
Author

luevano commented May 13, 2024

0 means 0 books. The Mylar schema does not enforce a positive number on that.

What does null books mean? Komga allows null for the count, I can manually remove the count and it becomes null. Just a matter of allowing total_issues to be null from the beginning or parse 0 as null.

What do you mean about the "not enforcing a positive number"?

@gotson
Copy link
Owner

gotson commented May 13, 2024

null means the field is not set, same goes for all non-mandatory metadata.

Just a matter of allowing total_issues to be null from the beginning

this is not allowed by Mylar's schema.

or parse 0 as null.

the schema should allow null values in that case

What do you mean about the "not enforcing a positive number"?

the schema should probably enforce that field to be strictly positive or null. But that's something that Mylar needs to change, they own the schema.

@luevano
Copy link
Author

luevano commented May 13, 2024

Ok, sure. The schema allows 0 (and even negative numbers I guess), as you mentioned it doesn't enforce positive numbers. So why block editing other metadata when there is a 0, this is still the initial issue. This is what I'm referring to handling the 0 case, I just want to be able to leave the 0 alone and focus on adding other metadata.

Either allow setting the 0 (just like when parsing) and this way other metadata can be added or show the error as soon as the edit metadata window opens (there's no validation when just opening it).

@gotson
Copy link
Owner

gotson commented May 13, 2024

It's 2 different things, the Mylar metadata, and the input of metadata from Komga API.

@luevano
Copy link
Author

luevano commented May 13, 2024

It's 2 different things, the Mylar metadata, and the input of metadata from Komga API.

Yes, I know. But they're not matching in allowed values right now (Mylar allows 0 while Komga doesn't). Again, it's still an issue that it is imported successfully as a 0 but Komga doesn't actually allow this value. Either fail or allow it.

@gotson
Copy link
Owner

gotson commented May 14, 2024

Just don't put 0 🤷🏻‍♂️

@gotson gotson closed this as not planned Won't fix, can't repro, duplicate, stale May 14, 2024
@luevano
Copy link
Author

luevano commented May 14, 2024

Anilist returns 0. Just don't parse the 0 from the start and provide meaningful errors.

@kiriya-aoi
Copy link

FWIW Anilist returns null, not zero when its own volumes and chapters fields are unset, like for an ongoing series (examples with volume and chapter unset vs set). What your metadata tool does with null is a separate issue.

@luevano
Copy link
Author

luevano commented May 14, 2024

FWIW Anilist returns null, not zero when its own volumes and chapters fields are unset, like for an ongoing series (examples with volume and chapter unset vs set). What your metadata tool does with null is a separate issue.

Your spaces were replaced by "+", but I see what you mean, you're correct. The tool is translating nulls to the "unset" values for the type, which for int is a 0.

At this point it's not about that but to just give an error when there is a zero, either at parsing time or in the UI (because it does get parsed into zero but it's not allowed by Komga, took me a lot of time to even debug it).

Also the schema supports zeros anyways, and his argument was that Komga adheres to the series.json schema, but it does not. A simple change to discard anything bellow 1 (0 and negatives) and make them null would help (as null is "accepted" as a field).

Again, this whole issue was because I couldn't even tell what was the error when I was trying to add other metadata, not because I WANT to use zero as the field, this was an afterthought which would help and save a lot of headaches. The zero is already being used and it behaves like null (doesn't display the total books that there should be), it just doesn't let you add other metadata fields.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants