Add problem points vote functionality

[lakshy-gupta]

[codecov-commenter]

Codecov Report

Base: 46.44% // Head: 46.64% // Increases project coverage by +0.20% :tada:

Coverage data is based on head (2506f09) compared to base (97f2c45). Patch coverage: 66.27% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1645      +/-   ##
==========================================
+ Coverage   46.44%   46.64%   +0.20%     
==========================================
  Files         236      237       +1     
  Lines       13072    13229     +157     
==========================================
+ Hits         6071     6171     +100     
- Misses       7001     7058      +57     

Impacted Files	Coverage Δ
dmoj/urls.py	89.47% <ø> (ø)
judge/admin/profile.py	62.22% <ø> (ø)
judge/models/__init__.py	100.00% <ø> (ø)
judge/models/submission.py	88.81% <ø> (ø)
judge/views/problem.py	24.14% <26.98%> (-0.26%)	:arrow_down:
judge/admin/problem.py	56.64% <61.90%> (+0.15%)	:arrow_up:
judge/models/problem.py	90.85% <91.17%> (+0.03%)	:arrow_up:
dmoj/settings.py	97.72% <100.00%> (+0.03%)	:arrow_up:
judge/admin/__init__.py	100.00% <100.00%> (ø)
judge/forms.py	46.03% <100.00%> (+1.46%)	:arrow_up:
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[kiritofeng]

Can you please read this tutorial on how to rebase and this one on rewriting git history so we can review the relevant changes without having to cross-reference what we already have in master.

[lakshy-gupta]

I've squashed the commits and fixed the rebase issue, this pull request should be ready for review.

[ehhthing]

Could you upload screenshots for how this looks on both desktop and mobile?

[lakshy-gupta]

We updated the request, it should be ready for another round of code review.

[Riolku]

Commits definitely need to be split up. Maybe one for the migration, another for the unit tests, and maybe find some sane division of the remaining work. Label them appropriately.

[Ninjaclasher]

Please make sure the overlay covers the entire screen for large screens.

[Riolku]

Yeah, just cursor: pointer should do.

[Riolku]

What is this error?

[Riolku]

I'm not sold on the wording here, maybe something like Invalid points value! Points must be integers in the range [1, 50]

[Riolku]

Same with this. I think standard wording is more like "Points must be at most 50" or "Points cannot be greater than 50" or something... "Ensure" is just strange

[Riolku]

Should I be able to vote without justification?

[Riolku]

+1 on general UI, the lightboxes and what not look pretty clean.

[Riolku]

Good, you use min and max on the integer field.

[lakshy-gupta]

With reference to the error messages on the voting field, I believe they are automatically generated by Django.

[Riolku]

Well unless explicitly overridden by Roger or someone above, I think that error message (especially "this field is required") needs to be fixed. Do other parts of the codebase do it differently? Why is this error message garbage?

[lakshy-gupta]

There are other parts of the codebase that do it like this: I'll try to see what the codebase does for non-admin users though.

[Riolku]

Based on my search, the codebase doesn't have this issue yet, for non-admins.

[Riolku]

FWIW, if we want to customize the error messages, we can: https://docs.djangoproject.com/en/4.0/ref/forms/fields/#error-messages

[Riolku]

I'm going to look in Django to figure out what's wrong with IntegerField.

[Riolku]

Worst-case, we can override the "required" error message with something very generic.

[Riolku]

I see the issue: in JS, if the integer is invalid, it sends an empty string, or rather, if I have a entered, the POST request sends an empty string: so not an issue with django. The question is: what do we want to do now? Can we find a non-terrible workaround? Do we validate on the client? Sounds kinda ugly. Do we override required? I'm not super fond of that either.

[Riolku]

I think for now, let's override required, with an explicit comment about why we do so. This avoids issues with translations.

[lakshy-gupta]

Should I be able to vote without justification?

Yes

[Riolku]

Should I be able to vote without justification?

Yes

Out of band discussion: users will probably just put in garbage if it's required, so let's not bother. Besides, will we read the justifications?

[Riolku]

Sure, whatever, keep it.

[Riolku]

Note: you are failing lint.

[int-y1]

All previous conversations are resolved. Ready for review.

[int-y1]

Up-to-date screenshots:

[Riolku]

Oh no, what happened. (Firefox, reproduced in Chromium)

The chart also seems to be messed up...