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

coach - overly aggressive errors from page title logic as state is changing #11291

Open
thanksameeelian opened this issue Sep 22, 2023 · 12 comments
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) P2 - normal Priority: Nice to have

Comments

@thanksameeelian
Copy link
Contributor

Observed behavior

excessive errors in the console when user deletes a lesson in Coach - it appears that as user is automatically navigated away from the page of the lesson that the user just deleted, there are several calls to get page title using the details of the just-deleted lesson, leading to an error as that lesson no longer exists.

there do not appear to be any errors related to the actual page the user is automatically navigated to (the class's lesson list page) and its title displays correctly. unnecessary calls are potentially being made to get the previous page title as we are exiting it & subsequent errors are getting logged in the console during the transition away from a page that no longer exists.

Errors and logs

Failed to obtain page title. Ensure that this route's meta.titleParts are correctly configured.   |    useCoreCoach.js:64

image

image

failed-to-obtain-page-title.mov

Expected behavior

  • only make calls to determine page title on appropriate & necessary pages when navigating between pages

Steps to reproduce

  • Coach > [select facility &/or class] > Plan > Lessons > [create a temporary lesson if necessary] > Select lesson
  • on the lesson's Summary page, Options > Delete lesson & confirm choice in the modal
  • should be automatically navigated back to class's Plan > Lessons page
  • despite successful navigation & appropriate page title displayed, errors in console during transition between pages
@thanksameeelian thanksameeelian added the P2 - normal Priority: Nice to have label Sep 22, 2023
@thanksameeelian thanksameeelian added the APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) label Sep 22, 2023
@ShivangRawat30
Copy link
Contributor

I would love to look into this issue.

@thanksameeelian
Copy link
Contributor Author

awesome - thank you @ShivangRawat30! i've assigned you 🙂

@nick2432
Copy link
Contributor

can i work on this?

@MisRob
Copy link
Member

MisRob commented Jan 18, 2024

Hi @nick2432, I think you can give it a try, thank you. It may be helpful to read through #11338 and feedback provided there to know what to be careful about.

@nick2432
Copy link
Contributor

Hi @nick2432, I think you can give it a try, thank you. It may be helpful to read through #11338 and feedback provided there to know what to be careful about.

I will give it a try. Thank you, @MisRob !

@nick2432
Copy link
Contributor

Hey @thanksameeelian , I've spent two days on this issue and have some questions. The main problem is that when I navigate from the lesson page to any other page, I receive a SUMMARY from PLAN_LESSONS_ROOT and then PLAN_LESSONS_ROOT from SUMMARY in the console. This occurs because of the following line of code: return classSummary.lessonMap[params.lessonId].title. Interestingly, when I just return classSummary, it works fine, but using classSummary.lessonMap[params.lessonId] results in an unexpected behavior.

2024-01-22.17-36-14.mp4
2024-01-22.17-44-58.mp4

@MisRob
Copy link
Member

MisRob commented Jan 23, 2024

Hi @nick2432, thank you for looking into this! Seems you're onto something related.

I've spent two days on this issue and have some questions. The main problem is that when I navigate from the lesson page to any other page, I receive a SUMMARY from PLAN_LESSONS_ROOT and then PLAN_LESSONS_ROOT from SUMMARY in the console.

Can I understand right that the problem you're referring to can be seen in the first recording when you return from the lesson summary page back to the root page, but shortly before the navigation occurs, there are some calls to retrieve the summary page title despite we're navigating from it?

This occurs because of the following line of code: return classSummary.lessonMap[params.lessonId].title. Interestingly, when I just return classSummary, it works fine, but using classSummary.lessonMap[params.lessonId] results in an unexpected behavior.

Could you please send links to related code you're mentioning so that I know where to look at exactly? Alternatively, you can open a draft pull request and push your logging. We could then use pull request comments to talk about relevant places.

@nick2432
Copy link
Contributor

nick2432 commented Jan 24, 2024

Can I understand right that the problem you're referring to can be seen in the first recording when you return from the lesson summary page back to the root page, but shortly before the navigation occurs, there are some calls to retrieve the summary page title despite we're navigating from it?

yes

Could you please send links to related code you're mentioning so that I know where to look at exactly? Alternatively, you can open a draft pull request and push your logging. We could then use pull request comments to talk about relevant places.

https://github.com/learningequality/kolibri/blob/release-v0.16.x/kolibri/plugins/coach/assets/src/composables/useCoreCoach.js#L55

@MisRob
Copy link
Member

MisRob commented Jan 24, 2024

@nick2432 Alright, got it. One idea I had was that perhaps finding out why formatPageTitle is called in the first place would help to understand better what's actually the main problem. Since formatPageTitle is returned from the reactive computed here https://github.com/learningequality/kolibri/blob/release-v0.16.x/kolibri/plugins/coach/assets/src/composables/useCoreCoach.js#L13, it's possible that it's being triggered unexpectedly under certain conditions. You could try to track related functions calls as you navigate to create a mental picture of what's going on.

@nick2432
Copy link
Contributor

nick2432 commented Feb 1, 2024

@MisRob , I tried a lot, but I don't think I can do this. I'm sorry. Can you please unassign me from this issue? I have some ideas, but I doubt they will work for the original problem.

@MisRob
Copy link
Member

MisRob commented Feb 1, 2024

Alright @nick2432, thank you for your effort.

@MisRob
Copy link
Member

MisRob commented Apr 2, 2024

Depends on what's the problem but @rtibbles mentioned that https://vue-meta.nuxtjs.org/ may be handy if this shows to be a higher-level problem with the way we're approaching updating page metadata

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Coach Re: Coach App (lessons, quizzes, groups, reports, etc.) P2 - normal Priority: Nice to have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants