fix: Novel fire chapter fetch issue - #2099
Conversation
| .text() | ||
| .trim(); | ||
| novel.totalPages = Math.ceil(parseInt(totalChapters) / 100); | ||
| novel.totalPages = Math.ceil(parseInt(totalChapters) / 50); |
There was a problem hiding this comment.
Is there anyway to dynamically get this number of chapters per page instead of hard coding?
In case NovelFire changes it again in future, it would need a plugin update.
There was a problem hiding this comment.
const firstPage = await this.parsePage(novelPath, '1');
const chaptersPerPage = firstPage.chapters.length;
novel.totalPages = chaptersPerPage > 0
? Math.ceil(parseInt(totalChapters) / chaptersPerPage)
: 1;This is what I've got after a bit of looking around. But this requires an extra API call, so not sure if there are better solutions.
There was a problem hiding this comment.
Or if we want to have a fallback -
let chaptersPerPage = 50; // fallback
try {
const firstPage = await this.parsePage(novelPath, '1');
const count = firstPage.chapters.length;
if (count > 0) chaptersPerPage = count;
} catch (e) {
console.warn('[NovelFire] Failed to get chaptersPerPage, falling back to 50:', e);
}
novel.totalPages = Math.ceil(parseInt(totalChapters) / chaptersPerPage);There was a problem hiding this comment.
I don't think it's a bad idea to make the extra request first to get the correct number of pages. The getAllChaptersForce already makes a separate request for each page. The only other solution I have in mind would be to fetch the total amount of chapters from the main novel page and then keep iterating through pages until all the chapters are fetched. Either way is one more request. I'm not great at Javascript, so not sure what the exact implementation would look like, I just wanted to at least get the plugin into a working state.
Looking a little bit further in though, it looks like the page that the parsePage function fetches has the latest chapter number under the CSS selector: "header.container > p:nth-child(5) > a:nth-child(1)". I think it would be possible to just make the fetchPage function additionally return that value on top of what it returns already so we can have the total amount of chapters without making an extra request since we would always be grabbing the first page no matter what. Once we have that getAllChaptersForce would just have to be modified so it keeps fetching the next page until allChapters.length() is equal to the most recent chapter.
There was a problem hiding this comment.
Once we have that getAllChaptersForce would just have to be modified so it keeps fetching the next page until allChapters.length() is equal to the most recent chapter.
This might involve more changes than expected, as the API seems to be designed to do things in paralel based on the total number of pages passed along.
I am in favour of getting the fix in firstly to resolve the issue.
There was a problem hiding this comment.
I think that this might fix it for now but it should have a dynamic fix that contends to future updates and changes to the site. All in all, I am in favor of pushing this fix for now. It's feasible and will get the job done.
Also, I am a newbie at this kind of thing and would like your help in fixing it for myself temporarily because don't know when the issue will get fixed officially.
Thanks and good work, mate!
There was a problem hiding this comment.
I made this PR mostly as a hotfix to get things working again, I agree that it needs a minor revamp to prevent this issue in the future.
Also, I am a newbie at this kind of thing and would like your help in fixing it for myself temporarily because don't know when the issue will get fixed officially.
You should be able to fork the repo, make the one line change to the file, and then run "npm run publish:plugins -- --all-branches" on your local copy of the repo. I couldn't get the GitHub action working on my fork, but manually running the command worked. You can then add your repo as a plugin repo in the app.
|
please add the following totalChapters: string,
): Promise<Plugin.ChapterItem[]> {
const url = `${this.site}ajax/listChapterDataAjax`
const params = new URLSearchParams({
draw: "1",
"columns[0][data]": "n_sort",
"columns[0][name]": "cmm_posts_detail.n_sort",
"columns[0][searchable]": "true",
"columns[0][orderable]": "true",
"columns[0][search][value]": "",
"columns[0][search][regex]": "false",
"columns[1][data]": "bookmark_created_at",
"columns[1][name]": "bookmark_chapters.created_at",
"columns[1][searchable]": "false",
"columns[1][orderable]": "true",
"columns[1][search][value]": "",
"columns[1][search][regex]": "false",
"order[0][column]": "0",
"order[0][dir]": "asc",
"order[0][name]": "cmm_posts_detail.n_sort",
start: "0",
length: totalChapters,
"search[value]": "",
"search[regex]": "false",
post_id: post_id,
only_bookmark: "false",
_: Date.now().toString()
});
const result = await fetchApi(`${url}?${params.toString()}`); const post_id = $('#novel-report').attr('report-post_id') || '0';
const totalChapters = $('.header-stats i.icon-book-open').parent().text().trim();
try {
novel.chapters = await this.getAllChapters(novelPath, post_id, totalChapters);
} catch (error) { |
|
@K1ngfish3r Please see new commit. |
|
The ajax url has a Rather than getting totalChapters directly, am now trying to find a way to smuggle post_id to parsePage to keep this undercover |
|
smuggling not required! here's some code, (remove comments) webStorageUtilized = true; // enable this
draw = 0; if (pageNo === 1) {
this.novelList = [];
this.draw = 0; // reset this
} async parseNovel(
novelPath: string,
): Promise<Plugin.SourceNovel & { totalPages: number }> {
this.draw = 0; // reset here too const post_id = $('#novel-report').attr('report-post_id');
if (post_id) {
storage.set(`${this.id}_postid_${novelPath}`, post_id);
} async parsePage(novelPath: string, page: string): Promise<Plugin.SourcePage> {
const post_id = storage.get(`${this.id}_postid_${novelPath}`);
if (post_id && !isNaN(Number(post_id))) {
try {
const chapters = await this.getAllChapters(novelPath, post_id, page);
return { chapters };
} catch (e) {
// Fallback to scraping if AJAX fails
}
}
const url = `${this.site}${novelPath}/chapters?page=${page}`;
... async getAllChapters(
novelPath: string,
post_id: string,
page: string,
): Promise<Plugin.ChapterItem[]> {
const url = `${this.site}ajax/listChapterDataAjax`;
const start = (parseInt(page) - 1) * 50;
this.draw++;
const params = new URLSearchParams({
draw: this.draw.toString(),
'columns[0][data]': 'n_sort',
'columns[0][name]': 'cmm_posts_detail.n_sort',
'columns[0][searchable]': 'true',
'columns[0][orderable]': 'true',
'columns[0][search][value]': '',
'columns[0][search][regex]': 'false',
'columns[1][data]': 'bookmark_created_at',
'columns[1][name]': 'bookmark_chapters.created_at',
'columns[1][searchable]': 'false',
'columns[1][orderable]': 'true',
'columns[1][search][value]': '',
'columns[1][search][regex]': 'false',
'order[0][column]': '0',
'order[0][dir]': 'asc',
'order[0][name]': 'cmm_posts_detail.n_sort',
start: start.toString(),
length: '50',
'search[value]': '',
'search[regex]': 'false',
post_id: post_id,
only_bookmark: 'false',
_: Date.now().toString(),
});
const result = await fetchApi(`${url}?${params.toString()}`); |
|
this can also probably also be used for getAllChaptersForce, but I haven't written that bit |
|
@K1ngfish3r Should all of this be in a separate PR? I just wanted to get a PR in so the plugin at least functions. |
Your changes has already been added lol |
|
Sounds good, closing this then. |
4 participants
Looks like in the paged mode changing the total pages to divide by the number of chapters per page to 50 instead of 100 fixed the missing chapters. Tested in Plugin Playground and the app and it seems to be working.
I think the issue started because Novel Fire used to have 100 chapters per page and they changed it to 50.
Closes issue #2097
Checklist