Module-linked pages toggle

[Akshat-Kalra]

Description

Adds a toggle for ingesting only module-linked pages to the vector store; namely the chatbot context.

I suppose this was requested by an instructors who uses our Canvas Pages Integration.

Preview:

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update
• This requires a run of yarn install
• This change requires an addition/change to the production .env variables. These changes are below:
• This change requires developers to add new .env variables. The file and variables needed are below:
• This change requires a database query to update old data on production. This query is below:

TODO:

• Add the migration file

How Has This Been Tested?

Hv to test more; so not ready to merge yet!!!

Checklist:

• I have performed a code review of my own code (under the "Files Changed" tab on github) to ensure nothing is committed that shouldn't be (e.g. leftover console.logs, leftover unused logic, or anything else that was accidentally committed)
• I have commented my code where needed
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have checked that new and existing tests pass locally with my changes
• Any work that this PR is dependent on has been merged into the main branch
• Any UI changes have been checked to work on desktop, tablet, and mobile

[Akshat-Kalra]

@FabianLCH Just added you as reviewer rn; but this is definitely not ready to merge yet. There is some race condition or caching issue in play where sometime toggling the button off deletes all the pages 💀.

[AdamFipke]

Will this also be checking if the linked module is published/visible?

[FabianLCH]

Changes are looking good so far, left some comments. I noticed the following:

• Since the "Module-linked pages only" setting is updated separately from the selected resource types and does not trigger a re-sync, any 'unlinked' pages already synced will not get removed until (a) the next CRON job run or (b) the instructor manually forces synchronization. I'm wondering if it would be better to group the toggle with the resource selector so the instructor has to click 'Save & Resync' to save their preferences and automatically trigger a sync.
• The single document toggle sync endpoint doesn't appear to check for the moduleLinkedPagesOnly and a page's isModuleLinked setting to prevent force syncing an 'unlinked'. Not sure if this is intentional, but even if it is, given syncDocuments filters out unlinked pages from the LMS API response, the document would get automatically removed on the next bulk sync.

[Akshat-Kalra]

Will this also be checking if the linked module is published/visible?

Every document type is checked for if its published when its fetched; this would just filter on top of that.

[Akshat-Kalra]

@FabianLCH Just added you as reviewer rn; but this is definitely not ready to merge yet. There is some race condition or caching issue in play where sometime toggling the button off deletes all the pages 💀.

This should be fixed now, there was some caching issue in play.

[AdamFipke]

hmm, it would be cool if this message could somehow be communicated to the user, but that might require too much of a refactor to be worth it

[bhunt02]

Agreed on this - but a simple way to implement this is putting a message on the frontend when the toggle is enabled saying 'no pages were detected' if we attempt to retrieve pages, and find none due to their not being any module-linked ones.

[AdamFipke]

This might be a silly question, but is it only pages that can be linked into modules or can other resources be linked as well? And would it make sense to have the option to only synchronize module-linked resources (and not just pages)?

Also, I looked over #313 and I noticed it mentioned 'locked' modules. Is there a difference between locked and published in Canvas?

[Akshat-Kalra]

This might be a silly question, but is it only pages that can be linked into modules or can other resources be linked as well? And would it make sense to have the option to only synchronize module-linked resources (and not just pages)?

Any Canvas resource can be linked into modules, from what I've seen it is mostly used to organize pages but it is definitely used to organize all resources of the same module in a course.

I think for the pages toggle, it was an instructor requested feature that bridgette told me about.

I was initially thinking about adding a separate such toggle for every resource, but we could think about adding 1 for all resources.

maybe what we can do is, hv a small checkbox on top of every resource tab titled "sync from modules" or smth like that and then one button on the main resources selector that does it for every resource.

[Akshat-Kalra]

Also, I looked over #313 and I noticed it mentioned 'locked' modules. Is there a difference between locked and published in Canvas?

I think locked modules are published but are greyed out and not clickable by students until a criterion is met (eg. completing previous modules, maybe specific date, time set for when it unlocks).

[FabianLCH]

Gave this another review! Commented on something pretty minor, but changes look good overall. Also waiting on my comment re: unpublished module filtering

[bhunt02]

I apologize for how long it took me to leave a review on this PR. I took a look at the code and nothing jumped out to me as requiring significant manual testing.

Backend code looks solid. Since we're working off an external API it's difficult to verify the types I know, but I don't see any issue reviewing the Canvas LMS types for modules.

I very much appreciate the attention to detail on the frontend leaving warnings and messages for the professors who toggle this option!