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
E2281. Reimplement waitlists #2485
base: main
Are you sure you want to change the base?
Conversation
E2281. Add migration to create waitlist_teams table
add migration to migrate waitlists to waitlist teams
add Controller files and views files(sign-up and student_task) to change compilation
Add migrations to change compilation
add helpers changes to change compilation
Add Model Files to change compilation
…rtiza-E2281 into change-compilation
Pr to push Change compilation to main
# Searches database for all waitlisted teams for specific assignment | ||
def self.find_waitlisted_teams_for_assignment(assignment_id, ip_address = nil) | ||
waitlisted_participants = WaitlistTeam.joins('INNER JOIN sign_up_topics ON waitlist_teams.topic_id = sign_up_topics.id') | ||
.select('waitlist_teams.id as id, sign_up_topics.id as topic_id, sign_up_topics.topic_name as name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't add SQL queries written on the code. You can use Rails' activerecord statements to achieve the same.
html += participant.user_name_placeholder | ||
if assignment.max_team_size > 1 | ||
html += '<a href="/sign_up_sheet/delete_signup_as_instructor/' + participant.team_id.to_s + '?topic_id=' + topic.id.to_s + '"">' | ||
html += '<img border="0" align="middle" src="/assets/delete_icon.png" title="Drop Student"></a>' | ||
end | ||
html += '<font color="red">(waitlisted)</font>' if participant.is_waitlisted | ||
html += '<font color="red">(waitlisted)</font>' if is_waitlisted |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Helpers, in my opinion, should be exclusively used for getting the data, and all the html rendering should be done in the .html.erb
files. Please refactor this as well.
# if a confirmed slot is deleted then push the first waiting list member to confirmed slot if someone is on the waitlist | ||
unless assignment.is_intelligent? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If any code is being removed, please add a code comment of what was removed, along with your team id (E2281 I suppose)
@@ -6,13 +6,9 @@ class SignedUpTeam < ApplicationRecord | |||
validates :topic_id, :team_id, presence: true | |||
scope :by_team_id, ->(team_id) { where('team_id = ?', team_id) } | |||
|
|||
def self.find_team_participants(assignment_id, ip_address = nil) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, add comments for code removed
@@ -2,25 +2,37 @@ | |||
|
|||
<table class="general"> | |||
<tr><%= render :partial => '/sign_up_sheet/table_header' %></tr> | |||
<% if !@selected_topics.nil? && @selected_topics.size != 0 %> | |||
<% if (!@selected_topics.nil? && @selected_topics.size != 0 )|| (!@waitlisted_topics.nil? && @waitlisted_topics.size != 0) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use unless instead of .nil
with !
let(:waitlist_team3) { build(:waitlist_team) } | ||
let(:signedupteam1) { build(:signed_up_team) } | ||
|
||
describe 'CRUD WaitlistTeam' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you be more specific on what the individual test-suite do instead of CRUD waitlistteam ?
@@ -0,0 +1,38 @@ | |||
describe WaitlistTeam do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would also like to see the functionality of how waitlisting works when team gets deleted
…rtiza-E2281 into Changes_since_demo
Suggested Changes
E2281. Refactor student_teams functionality
We ( @nagarajumadamshetti @naydt01 @RoninS28 @hss69017 ) have performed the following changes:
Wiki:
https://expertiza.csc.ncsu.edu/index.php/CSC/ECE_517_Fall_2022_-_E2281._Reimplement_Waitlists
Changes:
Created a migration to add a waitlist teams table.
Created a migration to migrate waitlisted sign-up teams to the waitlist team
Created functionality that:
Added Rspec Tests for:
@namanshrimali
@nagarajumadamshetti @naydt01 @RoninS28