Skip to content
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

WCA Live Result Admin #1: Submit and Updating Results #10776

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

FinnIckler
Copy link
Member

Simple Submitting/Updating Results and double checking them. Does not include calculating advancing/rankings/records. Those will be follow up PRs (which I have already done, but I want to add tests).

Have not checked if I deleted everything related to those features, will do after dinner.

app/controllers/live_controller.rb Outdated Show resolved Hide resolved
app/controllers/live_controller.rb Outdated Show resolved Hide resolved
# TODO: What is the best way to do this?
r = Result.build({ value1: results[0], value2: results[1], value3: results[2], value4: results[3] || 0, value5: results[4] || 0, event_id: round.event.id, round_type_id: round.round_type_id, format_id: round.format_id })

result.update(average: r.compute_correct_average, best: r.compute_correct_best, live_attempts: new_attempts, entered_at: Time.now.utc, entered_by: current_user)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we send a timestamp from the frontend when the query was fired?
If we're running this in a queue later, the "now" here in the backend could be significantly different from the "now" happening for the people in front of their screens.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updating is currently not done in a queue, maybe it should? But we aren't doing that for registrations for example (but there updating doesn't create anything new)

Comment on lines 42 to 52
new_attempts = results.map.with_index(1) do |r, i|
same_result = previous_attempts.find_by(result: r, attempt_number: i)
if same_result.present?
same_result
else
different_result = previous_attempts.find_by(attempt_number: i)
new_result = LiveAttempt.build(result: r, attempt_number: i)
different_result&.update(replaced_by_id: new_result.id)
new_result
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: I have a hunch there might be an even more efficient way to handle this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will play around later when I have more headspace.

app/controllers/live_controller.rb Outdated Show resolved Hide resolved
app/webpacker/components/Live/Admin/Results/index.jsx Outdated Show resolved Hide resolved
app/webpacker/components/Live/Admin/Results/index.jsx Outdated Show resolved Hide resolved
return {};
}
if (result.advancing) {
return { backgroundColor: 'rgb(0, 230, 118)' };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The green tone should probably be stored somewhere, especially since you're reusing it with transparency again below

Comment on lines +33 to +70
export const recordTagStyle = (tag) => {
const styles = {
display: 'block',
lineHeight: 1,
padding: '0.3em 0.4em',
borderRadius: '4px',
fontWeight: 600,
fontSize: '0.6em',
position: 'absolute',
top: '0px',
right: '0px',
transform: 'translate(110%, -40%)',
color: 'rgb(255, 255, 255)',
};

switch (tag) {
case 'WR': {
styles.backgroundColor = 'rgb(244, 67, 54)';
break;
}
case 'CR': {
styles.backgroundColor = 'rgb(255, 235, 59)';
break;
}
case 'NR': {
styles.backgroundColor = 'rgb(0, 230, 118)';
break;
}
case 'PR': {
styles.backgroundColor = 'rgb(66, 66, 66)';
break;
}
default: {
return {};
}
}
return styles;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of big custom styles at all. SemUI has a Label component, can that work?
For specific colors, I'll agree to some CSS styling but display: block and others should definitely be left to SemUI.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, this was a fairly lazy way of recreating the same thing as wca live

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants