Moves RosterEntry update to background job

[shaunakpp]

What

Fixes #2210

• Setup AddStudentsToRosterChannel to send progress
• channel is called from AddStudentsToRosterJob
• progress via action cable is handled by add_students_to_roster.js
• removes the old create_entries method from model

TODO:

• [x] Add specs
• [x] Iterate on UIUX

UI: update with a few students(< 100)

UI: update with many students(> 100)

[d12]

Curious how large of a CSV would need to be uploaded to see delays. RosterEntry's are very lightweight AR objects, so I'm surprised we're noticing any issues here.

[shaunakpp]

I agree, we don't see any noticeable delays with 15-20 objects, but I did notice delays with around 100 entries. Also, since everything is in a transaction if the teacher cancels, we do a rollback which takes significant time, especially with large number of students.

In most cases(adding a few students), teachers wouldn't even notice the progress bar, but It can be useful if a large CSV is uploaded.

[shaunakpp]

cc: @femmebot I've added a progress bar for the update, but I'm not fully confident about how it looks, or it's placement. I'd love your inputs on how we could add it to this page.

Personally, I think I could improve on:

• Adding a flash message to describe that the update is in progress(already working on it)
• Displaying information about the identifiers which were not added
• Moving this to the modal itself?
• If we move to a modal, display a message to teacher that the update is in progress, if the page refreshes and/or when the modal is not displayed.

[femmebot]

One quick suggestion: In the tabs, could we add the total count of all students and unlinked students. It's one quick way teachers can confirm whether they have all students accounted for.

[shaunakpp]

One quick suggestion: In the tabs, could we add the total count of all students and unlinked students. It's one quick way teachers can confirm whether they have all students accounted for.

Added this change:

[stephaniegiang]

@d12 This job is actually great especially with duplicate roster entries. With bigger classes, doing those duplicate checks will take a really long time.

[shaunakpp]

Updated after suggestions:

• The progress bar is now shown in the modal
• Made the request an ajax request to avoid re-direct and hence avoid inconsistency in communication with action cable
• Disables text_area and file upload button while the request is pending
• Message on roster update shows how many entries were added

[femmebot]

I think I'm confused by the existing UI 😂(perhaps not directly from this PR). If there are only 3 unlinked GitHub accounts in the Unlinked GitHub accounts, why do all the students in the All students say "unlinked user"

[stephaniegiang]

@femmebot Could you do a screenshot? Not sure if I'm seeing this

[femmebot]

@femmebot Could you do a screenshot? Not sure if I'm seeing this

ah, I was referring to the gif shown here

[stephaniegiang]

Ohhh 🤦‍♀ That is because the roster entries are not linked to a GitHub user yet. But we want to display all the roster entries. Maybe we can change this in the future? Maybe rename the tab or something?

[femmebot]

Wait … so the users in Unlinked GitHub accounts is not the same as unlinked user? Also, is the teacher expected to manually link all the students shortly after they add the roster?

[stephaniegiang]

Yea they are not the same. Teachers could manually link all the students to the roster, but they usually don't. When a student accepts an assignment, they are presented with the screen to click their roster identifier. That is how most roster entries get linked to a GitHub account.

[shaunakpp]

Hi @jeffrafter it looks like I can't accept the change because of permissions, so @stephaniegiang can you help me out?

[stephaniegiang]

@shaunakpp accepted the changes. Let me know if you need more help 😄

[shaunakpp]

I've resolved the conflicts in #2402 as I cannot directly resolve conflicts in this branch. We can merge #2402 directly as it is updating this branch and does not affect master

[stephaniegiang]

@shaunakpp sounds like a good plan! Are you able to do this yourself?

[shaunakpp]

@stephaniegiang yup, conflicts are resolved and all specs pass, you'll just need to merge #2402 😅

[stephaniegiang]

@shaunakpp Done 😄I'll do another round of testing on this PR before we merge

[shaunakpp]

@stephaniegiang thank you so much! ❤️

[stephaniegiang]

Looks really good 😄 Just tested it and ran smoothly