Closed hkamran80 closed 10 months ago
Thanks so much for your pull request!
@essential-randomness asked me to reply because she's a fool who trusts me to maintain repos and wants me to practice. (I really don't know what she's thinking.)
What do you think about adding the chapter info to the existing getWork
function instead of creating a separate getWorkChapter
function? The thought is to update getWork(workId)
to accept parameters {workId, chapterId}
where chapterId is optional:
export const getWork = async ({
workId,
chapterId
}: {
workId: string;
chapterId?: string;
}): Promise<WorkSummary | LockedWorkSummary> => {
Then we would add a chapterInfo key to the WorkSummary interface which is null for works with one chapter and which contains chapter info for works with multiple chapters:
export interface WorkSummary {
// the rest of the interface
chapterInfo: {
id: string;
index: number;
name: string | null;
summary: string | null;
} | null;
}
Since a work can have both a summary for the entire work and one for each chapter, it should be fine to return both, if they both exist. This would allow the end user to decide whether they want to use Work.summary
or Work.chapterInfo.summary
in their use case.
I think you're doing a wonderful job at maintaining! That's a great idea! I'll implement it right now!
The reason I removed WorkSummary.summary
from ChapterWorkSummary
is because the first chapter of a work cannot have a chapter summary and a work summary to my knowledge. Do you know of any fics that have both?
The reason you don't see it often is that the first time you create a work the summary box automatically is the work summary, but if you go back to edit Chapter 1 after Chapter 2 is created, another blank summary is present for editing. I discovered this by accidentally creating summaries under Chapter 1 a couple of times and having to go back to delete them.
I'm not sure I've ever seen a fic with both intentionally, but it is possible.
The code just picks all the summaries with the .summary blockquote.userstuff
selector, so both should be selected in that case.
I updated the code to use your suggestions.
Pinging @enigmalea!
Let me know if you're busy and i'll take care of this :)
After @enigmalea mentioned that chapter one could have a work and chapter summary, I added a separate function (getChapterSummary
) which only gets the chapter's summary. I also modified the getWorkSummary
function to only get the work's summary by checking for the preface
class.
I'll update the function signature for loadWorkPage
and add tests in a bit.
Woops, sorry about that, clearly I looked at it too fast and didn't see the logic had been updated (it's been a long day)! Tests will help clarify :)
All good!
If this is ready to be merged, I recommend using a squash merge.
Happy to help!!
This PR adds chapter details, like the index, name, and summary, to the work summary object. It's exposed through the
getWorkChapter
function and theChapterWorkSummary
interface.Tests are also included, this time with handlers and mock data!