Fix #19816 Change where headerText and subheaderText value are set and fix their display on frontend

[Sepremo]

Overview

1. This PR fixes [BUG]: Console error in chapter editor page. #19816.
2. This PR does the following: Relocated the updates for the headerText and subheaderText values from the Chapter Editor component (story-node-editor.component.ts) to the component responsible for the navigation of the Story Editor (in story-editor-navigation.service.ts), like it is done for the updates of SubTopic values in the component for routing of the Topic Editor (in topic-editor-routing.service.ts)
3. The original bug occurred because: The headerText (along with subheaderText) property undergoes updates after Angular's change detection cycle has concluded. This behavior stems from the component managing the new Chapter Editor Tab. The issue arises because it alters a bound value (as bound in the top-navigation-bar.component.html) within a new Tab (Chapter Editor), albeit within the same page (Story Editor).
   This error came from the code migration in Migration of story-editor-page #17968.

Essential Checklist

Please follow the instructions for making a code change.

• I have checked the "Files Changed" tab and confirmed that the changes are what I want to make.
• I have written tests for my code.
• The PR title starts with "Fix #bugnum: " or "Fix part of #bugnum: ...", followed by a short, clear summary of the changes.
• I have assigned the correct reviewers to this PR (or will leave a comment with the phrase "@{{reviewer_username}} PTAL" if I can't assign them directly).
• The linter/Karma presubmit checks pass on my local machine, and my PR follows the coding style guide).
• "Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)

Proof that changes are correct

19816.Fixed.mp4

[oppiabot]

Assigning @Lawful2002 for the first pass review of this PR. Thanks!

[Sepremo]

@seanlip PTAL

[seanlip]

Thanks @Sepremo! It looks good to me in general, just had one comment.

Also I have a question -- there are a bunch of issues filed for the ExpressionChangedAfterItHasBeenCheckedError -- see https://github.com/oppia/oppia/issues?q=is%3Aissue+is%3Aopen+expressionchangedafterithasbeencheckederror . Would you mind taking a look and seeing if your PR fixes any of these (or augmenting it to fix some of those if it's easy to do)? If it's not easy then perhaps leaving a comment on the issues you skip explaining what contributors should do might be helpful (but I think it would be quite nice if we could get rid of all of them in one go :P ).

Thanks!

[seanlip]

Bit concerned about this -- why is the empty string being used as the title?

(Also is the function name correct? It seems a bit odd to me that the index is null ... perhaps some jsdoc might be useful for navigateToChapterEditorWithId() explaining what the null case means, and we might need to update this function's name if it is not actually as general as it sounds.)

[Sepremo]

The comment on line 35 states:

// 'chapterIndex' is null when we are navigating to a chapter with its ID.

It appears that this function was solely utilized in the story-editor-page.component.ts file to open the Chapter Editor Tab when the page was refreshed or directly accessed via the page link, without needing to assign the chapterIndex value. This approach aimed to avoid waiting for the story object to load.

While refreshing the Chapter Editor page, I observed that it results in the chapter title not being updated in the NavigationBar, remaining as the Story Title. Since the chapter title can only be obtained from a node within the story object, I believe the only viable solution is to replace the usage of the navigateToChapterEditor() function with an asynchronous wait for the story to be loaded, followed by the utilization of the function navigateToChapterEditorWithId() with all the necessary values. Is this an appropriate solution, or would displaying the incorrect title while the story object loads and subsequently updating it to the correct one be preferred?

In my locally run server, implementing an asynchronous wait for the story seems to marginally slow down the page refresh by 0.5 seconds.

[seanlip]

Hi @Sepremo, thanks for digging! I think it is OK to impose a slight wait there, especially since the page isn't learner-accessible. Want to go ahead and do that? I think it'd be simpler to maintain things if we always used "correct" data throughout.

Thanks for digging into this!

[Sepremo]

Hey @seanlip,

I reviewed other issues with similar errors and found that their fixes likely won't be as straightforward. Some of those issues involve specific image assets, and another pertains to modifying the isEditable value, although its exact location is unclear. These issues appear to require different solutions and I don't think I would be able to fix them that easily. I'll leave informative comments where I can.

Additionally, issue #18573 seems to be identical to this one and should be closed once this issue is resolved.

I'll continue to try to fix the tests that are having errors.
Let me know if you need any further adjustments!

[seanlip]

Thanks for pointing out the duplicate, I closed that issue!

OK re the other issues, no worries about that. Let's try to get this PR in!

Also I noticed your reply here is for the general comment but not the specific one in this thread. Do you want to respond to the latter as well?

[Ash-2k3]

Hi @Sepremo, just wanted to check, if there's any update on this ?

[Sepremo]

Hi @Ash-2k3, sorry, I've been a bit busy but I am going to see if it's easy to fix the other similar issues. In the meantime I'll add the changes to fix navigateToChapterEditor.

[Lawful2002]

Just one comment, also please look at the failing tests, thanks!

[Lawful2002]

Thanks @Sepremo, the changes look good to me! Please fix the failing tests though.

[oppiabot]

Unassigning @Lawful2002 since they have already approved the PR.