Add queue randomization (#147)
Add queue randomization as suggested in #147.
I am having a bit of trouble setting up the server locally to test my changes, as I am getting
ActiveRecord::PendingMigrationError Migrations are pending. To resolve this issue, run: bin/rails db:migrate RAILS_ENV=development
but I figured I would open this PR to get some feedback, and to see if I could get help with testing and a couple questions I had. I am currently unsure about two changes:
-
I am not familiar at all with ruby, so I am guessing the way you shuffle the outstanding requests is by doing
@course_queue.outstanding_requests.shuffle(code here), but again I haven't been able to test this, and I am not sure if there are other metadata which also need to be shuffled around. -
I am not positive if calling
course_queue'sbroadcastInstructorMessagefrominstructor_panelwill work (code here). My intention is to send a notification to students whenever the queue is randomized.
Thanks for any help anyone can provide!
To fix the migrations error, I was able to start the container and then run
$ docker-compose run web bash
# bin/rails db:migrate RAILS_ENV=development
I think the shuffle also needs to be a little more complicated. It looks like the outstanding requests are stored as rows in the course_queue_entries table. They are then always displayed ordered by the created_at column (when the request was created).
To randomize these requests, I think we'd need to either rewrite all the created_at times (which isn't great) or add another column to the table to store a custom order.
Thanks Thomas! Got the queue running locally now. You're right -- the shuffling is not working, so there's more that needs to be changed there.
Also, what is the best way to test notifications to students? Currently I'm just opening the queue on multiple browsers and signing up as a student. However, my student browser is not receiving notifications even when the instructor broadcasts a message. Is there a better way to test multiple users (from the instructor's perspective as well as the student's perspective) at the same time other than with multiple browsers?
I've only been able to test with multiple browsers (or the same browser with one user in private browsing mode). For me, broadcasting from one browser/window does bring up an alert in other browsers/windows.
I've implemented a version of randomization that seems to work. More manual testing is required, and I haven't written any automated tests yet. I'd also like feedback on whether the way I implemented it seems like the best way to do it.
How it works:
- There is a new column in the
course_queue_entriestable,priority. - Requests are now sorted by
priorityand thencreated_at. - New requests have a priority of 0, so they are added to the bottom of the queue as usual.
- Clicking the "Randomize Queue" button randomly assigns priorities such that all unresolved requests are now in a random order.
- Users with unpinned requests are notified if their position in the unpinned section of the queue changes.
👋 thanks for the contribution! Sorry for the super long delay in reviewing this. 💭 on #216 as potential alternative?
I think the solution in #216 looks good too! It's a more systematic solution than the one in this PR.
Closing in favor of https://github.com/mterwill/office-hours-help-queue/pull/216. Thanks for your contribution!