Update contest scoreboard #123
Conversation
Previously students results would only be shown on the public scoreboard if equal to or better than the median score.
- Add two boolean fields to the database live_scoreboard: If true, show problem scores on the live scoreboard show_unofficial_competitors: Mark non highschool students on scoreboard - Add these to the contest edit form - Update the scoreboard page itself Also: - Removed the contest info tab because it seems useless - Set the default tab to scoreboard if logged out
Also fix unofficial contestant detection. - User must belong to a school - User must have not graduated yet - User must be from New Zealand
Users outside New Zealand are considered to be unofficial contestants for NZIC. However, students outside New Zealand were allowed to select a school. Requiring a blank school allows us to remove the country check when determining whether a user is an official contestant. A user is considered to be official by the presence of a school and school year from their contest relation (not their profile), since a user may change schools/countries after the contest.
If a user changes countries after starting a contest, this should not change their displayed country flag on the contest scoreboard.
|
@tom93 Thanks for the initial review, I'm fine with those changes. I was intending to squash, since the history is still visible in this PR. Some commits (e.g. bbb0a51, aca4daa) can be moved into separate PRs, and I don't mind merging #117, #118 in their own PRs if you want to preserve their history in master. Otherwise, if you prefer to merge, feel free to clean up the commit history of this PR and add your fixes. |
I'd like to keep some of the commits in the repo history. How about we keep working in this branch/PR, then create a new PR with the cleaned-up history and merge that. GitHub doesn't allow manually marking PRs as merged, so we'll have to can close the other PRs and add a comment saying they were merged as part of the cleaned-up PR. |
…n from code review) Deleted users were incorrectly shown. Also refactor.
Leave the default blank so the user can't forget to set it. (Presence isn't currently validated, but that will be done in a separate PR.)
…eflect current behaviour "Protected" and "Private" actually have the same behaviour at the moment (#107). Document the current behaviour until this is fixed.
Otherwise it results in "Live scoreboardShow the scoreboard during the contest. If unchecked, ..."
|
Perhaps the help text for only_rank_official_contestants should make it clear that viewers will still have the option to see unofficial contestants on the scoreboard? Currently it might give the impression that unofficial contestants will be removed from the scoreboard entirely. |
The text is now at the end of the parent <div> element, so the <br> elements are redundant.
There was a problem hiding this comment.
Perhaps the help text for only_rank_official_contestants should make it clear that viewers will still have the option to see unofficial contestants on the scoreboard? Currently it might give the impression that unofficial contestants will be removed from the scoreboard entirely.
Good point. How about appending "Users will have an option to show or hide unofficial contestants when they view the scoreboard."
Or just changing "ranking" to "ranking calculation", so the message would be "Only rank official contestants - Exclude contestants from ranking calculation if they are not enrolled in a NZ school."
app/views/contests/info.html.erb
Outdated
| <h2>Contest Settings</h2> | ||
| <b>Mode: </b>Closed book (you cannot access other problems on the site while competing)<br> | ||
| <b>Judge feedback: </b>Real-time<br> | ||
| <b>Scoreboard: </b>Live<br> |
There was a problem hiding this comment.
I agree that the "info" tab was pretty useless before, but I don't think we should delete it. (I'm fine with redirecting to the "scoreboard" tab by default instead of "info" for non-signed-in users though.)
- The line "Mode: Closed book" seems useless because the site doesn't support open-book contests, but I think we should keep it so first-time users know they won't have access to their previous submissions. I'm OK with removing this line though.
- The line "Judging feedback: Real-time" can be removed (judging feedback is always real-time on our site, and also almost everywhere else these days -- the only exception I can think of is Google Code Jam's hidden verdict test cases).
- "Scoreboard: Live" was previously always the case, but we are adding non-live scoreboards so this line becomes important. Without it, users can't tell whether past contests had live scoreboards or not. Also, it might help users during the contest, if they wonder why there is nobody else on the scoreboard (though they should have already seen the message "The scoreboard is hidden until the end of the contest" before starting). I think we should also add an explanation of hidden scoreboards here, e.g. "Scoreboard: Hidden (you can only see your own score until the end of the contest)".
- We might want to move some of the information from the rest of the page into the "info" tab, to take up less space on the other tabs. I'm think specifically of the "Owner" line and the finalize/un-finalize buttons, but we might also want to move the start time, end time, and duration (at first I thought they were too important, but then I realised the user probably already saw that information in the contest list).
There was a problem hiding this comment.
I don't mind restoring the info tab. I'm fine with keeping all the current lines and adding messages for hidden scoreboards.
The owner field can be moved. I feel like finalize should appear on the scoreboard tab (since it basically locks the state of the scoreboard), so I'd prefer to leave it or move it to appear only on the scoreboard tab. I'd prefer to keep the others visible on all tabs (users could forget or miss it, and they might have been linked directly to a contest page).
There was a problem hiding this comment.
I restored the info tab and added the explanation of hidden scoreboards.
I suggest that we separately remove the line "Judging Feedback: Real-time", move the "Owner" line into the info tab, and move the finalize/un-finalize buttons into the scoreboard tab.
There was a problem hiding this comment.
Sure, I'm fine with that.
This sounds good. |
The the element created by "f.label" already contains the text "Only rank official contestants", so parts of the previous message were redundant. With this commit, the full message will be "Only rank official contestants - Exclude contestants from ranking if they are not enrolled in a NZ school."
In the phrase "Exclude contestants from ranking", the word "ranking" is intended to mean the numeric positions (1st, 2nd, 3rd) but could be interpreted as referring to the entire scoreboard.
This reverts parts of commit 937df69 (Update contest scoreboard and edit page plus remove info tab., 2020-04-19). The "info" tab was indeed useless (all the information was hard-coded), but since we are adding hidden scoreboards it's going to be more useful. Keep the change to redirect to the "scoreboard" tab instead of "info" for users who can't view the problems. Discussion: #123 (comment)
Also add an explanation of hidden scoreboards.
Previously the markup for the "problems" tab on the contest page was
<p>
<table>
...
</table>
</p>
This is invalid because <table> is a block-level element and can't
appear inside a <p> element. It's actually parsed as
<p></p>
<table>
...
</table>
<p></p>
The effect is that it adds spacing before and after the problems
table. I don't think it's intentional (the other tables don't have
spacing at the end), so I'm removing the <p> tags.
Move problem set from bottom of contest page (on all tabs) to top of "problems" tab.
Add a class named "current-user" to the row of the current user so it can be used in the JavaScript code instead of the "emphasized" class.
This adds extra space between the last checkbox and the "Create Contest" button. There wasn't extra space before, and I don't think the extra space is necessary/consistent.
This fixes the error: Uncaught TypeError: document.getElementById(...) is null
Currently the explanation might give the impression that unofficial contestants will be removed from the scoreboard entirely. This makes it clear that viewers will still have the option to see unofficial contestants on the scoreboard.
…sers migration to abort on validation failure" This reverts commit 7310e63.
This reverts commit e4c35fe.
It has already been merged in #129.
- Show all results publicly on the contest scoreboard - Add field descriptions to the contest editing form - Add option to hide the public scoreboard until the contest has ended - Add option to remove unofficial contestants from the ranking calculation Co-authored-by: puqeko <[email protected]> Co-authored-by: Tom Levy <[email protected]> Co-authored-by: Eric Song <[email protected]>
|
Will deploy the current state of this PR in branch |
There was a problem hiding this comment.
Approved.
I've only noticed just now that unofficial contestants are very easy to miss because they are hidden by default and the checkbox to toggle their visibility is far on the right side of the screen. I'm not sure if this is desirable or not.
An idea to increase their visibility is to move the checkbox to the left side of the screen, next to the scoreboard heading:
(done by moving the <div> element with the checkbox to be after the <h2> element, then floating both the <h2> and the <div> to the left and adding a margin between them)
That does make the heading area a bit too "busy" though. Leaving it up to you to decide.
|
@puqeko For future reference, Rails database migrations can be generated, and the generator uses a specific naming convention ("AddColumnToTable"). For example creates a file called class AddLiveScoreboardToContests < ActiveRecord::Migration
def change
add_column :contests, :live_scoreboard, :boolean
end
endThis is just missing |
MERGED (#141)
Based on PR #103.
Notable changes: