Aggregate Wrapped user data

[NIDHI2023]

Summary

• Building on this PR, adds total time TAs helped students, a user's most visited OH id, and most visited TA id as part of the wrapped data
• Adds extra check when pushing to Wrapped collection that user must be active
  • Active is defined as all users need at least one OH visit, and TAs need at least one session where they answered a question.
• Also rounds some of the time-related numbers to ensure integer results. This meant while testing the numbers weren't exactly accurate, but they also weren't completely far off and were always integer.

Test Plan

• Inactive student/TAs are not added to wrapped collection
• Total time is an integer always, and no negative numbers or infinity
• Favorite OH and TA is accurate in test collections
• Appears that nothing breaks with undefined values and collection is populated

Notes

• In the test database, there were not that many visits (which makes sense) so the personality was Independent for everyone. However on the real data if this is also happening, perhaps the thresholds could be lowered so more users get a variety of personalities

Breaking Changes

None

• I have updated the documentation accordingly.
• My PR adds a @ts-ignore

[dti-github-bot]

[diff-counting] Significant lines: 524.

[rgu0114]

left a couple comments

[rgu0114]

Is this else if redundant?

[NIDHI2023]

I was thinking of the case where while iterating through questions, a user who is actually a ta comes up first as the asker, so they get an instance created without the timeHelpingStudents (as undefined). Then if they come up later as an answerer, indicating that they are a ta, the if statement only checks and gives all the fields if the ta doesn't already have an instance, so I think they wouldn't actually get the timeHelpingStudents field without this extra check.

[rgu0114]

oh i see

[rgu0114]

yea for this I think it's ok if not every session has TAs assigned

[rgu0114]

to answer your question, let's use most common answererId for a student as a proxy for their favorite TA since there's many courses that don't assign TAs to office hours in QMI and work backwards from there to determine favorite class and day of the week since class should correspond with favorite TA. We can get class pretty easily from sessionId, but maybe we run a separate script to label each session with a day of the week – we'll leave day of the week for later.

[rgu0114]

remember to change these dates to sp24 and fa24

[rgu0114]

At this point, now that the functionality is more or less there I think it'll be helpful to start thinking about code style and readability – which should also make verification a bit easier as we get into the last few weeks of development.

1. Separating concerns into helper functions, for example async functions handling processSessions(answererId, sessionId) and updateWrappedDocuments().
2. await can sometimes be inefficient when repeatedly called, and we should start thinking a bit more about performance for this script. It might be better to do something like the following: const sessionPromises = questionsSnapshot.docs.map(doc => sessionsRef.doc(doc.sessionId).get());
const sessionDocs = await Promise.all(sessionPromises);
   Of course, this is made difficult when we're (more intuitively) iterating through questions, which each have their own sessionId, so one possibility would be to do some pre-processing to group questions with the same sessionId together beforehand so we can do this type of batch processing.
3. Using a for...of loop is generally better for async operations. So for the loop at the end where we use userStats to update the Wrapped collection, you might do something like for (const [userId, stats] of Object.entries(userStats)) {.
4. For all error scenarios, especially since we'll be rolling this out to production soon, let's collect all userIds in a separate list and log those out at the end so we can investigate any issues more easily.

Try to refactor the code and make these changes incrementally, testing as you go along.

[rgu0114]

A couple small things to improve code maintainability

[rgu0114]

This is a confusing conditional. Let's make it something like:

const hasVisits = stats.numVisits > 0;
const isTaActive = stats.timeHelpingStudents !== undefined || (TAsessions[userId]?.length > 0);
const hasFavoriteTa = stats.favTaId !== "";

if (hasVisits && isTaActive && hasFavoriteTa) {

[rgu0114]

this might lead to a weird edge case error if officeHourSessions[askerId] is undefined since we're using optional chaining (?.). Let's make this:
officeHourSessions[askerId] = officeHourSessions[askerId] || []; if (!officeHourSessions[askerId].includes(sessionId)) { officeHourSessions[askerId].push(sessionId); }

[rgu0114]

this block of code getting the user's favorite class and title seems correct and precise to me, but is it a bit inefficient? For instance, if we've already identified the favorite TA, there should only be one class/title and we don't need to do all this ordering. Idk, there might just be some edge cases I haven't thought of.

[NIDHI2023]

I mainly included this extra if statement because I wasn't sure if it was possible for a user to encounter the same TA in two classes. But if we can confirm that a student always has one class in common with the favorite TA and is a guaranteed precondition, I definitely agree that this isn't needed.