Add problem points vote functionality #1645

lakshy-gupta · 2021-04-19T02:28:40Z

No description provided.

codecov-commenter · 2021-04-19T02:30:29Z

Codecov Report

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

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%)`	⬇️
judge/admin/problem.py	`56.64% <61.90%> (+0.15%)`	⬆️
judge/models/problem.py	`90.85% <91.17%> (+0.03%)`	⬆️
dmoj/settings.py	`97.72% <100.00%> (+0.03%)`	⬆️
judge/admin/__init__.py	`100.00% <100.00%> (ø)`
judge/forms.py	`46.03% <100.00%> (+1.46%)`	⬆️
... 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.

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

kiritofeng · 2021-04-19T02:35:48Z

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 · 2021-04-19T19:03:20Z

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

judge/views/problem.py

ehhthing · 2021-04-19T20:11:59Z

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

judge/admin/vote.py

judge/models/problem_points_vote.py

judge/models/profile.py

templates/problem/problem.html

judge/admin/__init__.py

judge/views/problem.py

templates/voting-stats.html

judge/models/profile.py

judge/views/problem.py

templates/lightbox.html

templates/problem/problem.html

judge/admin/vote.py

judge/views/problem.py

templates/voting-stats.html

fluix-dev

~~Also~~ Mostly just nits, based the existing code I've looked at:

Please leave a space after your // comments in JS.
Capitalize the first letter of your comments as you would a sentence.
- If it's a full sentence, add a period.
There's quite a few unnecessary comments, I've pointed out a few, please follow through with the rest.
Configure your editor to add newlines to the end of your files (you can see it as the ⛔ on GitHub)
Put commas between function parameters in JS.

judge/views/problem.py

judge/models/problem.py

templates/problem/voting-controls.html

templates/problem/voting-stats.html

judge/admin/vote.py

judge/models/problem.py

lakshy-gupta · 2021-07-27T19:37:18Z

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

dmoj/urls.py

judge/admin/vote.py

judge/models/problem.py

templates/problem/voting-stats.html

templates/problem/voting-controls.html

templates/problem/voting-stats.html

templates/problem/voting-controls.html

Ninjaclasher

Otherwise LGTM. Could you post some screenshots?

judge/migrations/0122_add_voting_functionality.py

judge/views/problem.py

templates/admin/judge/problem/change_form.html

templates/problem/voting-controls.html

templates/problem/voting-stats.html

judge/models/problem.py

judge/views/problem.py

templates/problem/voting-controls.html

Xyene · 2022-10-28T06:17:39Z

resources/problem-vote.scss

+    margin-right: 0.5em;
+}
+
+#problem-vote-form textarea {


Is there a reason we have both an id and a class for problem-vote-form?

Lack of imagination. It's probably ok to rename:

class="problem-vote-form" -> class="problem-vote-container"

.problem-vote-form -> .problem-vote-container

done, .problem-vote-form was renamed to .problem-vote-container

templates/problem/vote-ajax.html

int-y1 · 2022-10-28T07:25:23Z

judge/admin/problem.py

+
+
+class ProblemPointsVoteAdmin(admin.ModelAdmin):
+    list_display = ('points', 'voter', 'problem', 'vote_time')


Make problem link to the problem. See CommentAdmin.linked_page for a code example.

int-y1 · 2022-10-28T07:58:28Z

resources/problem-vote.js

+            },
+            error: function (data) {
+                if (data.message !== undefined) {
+                    alert(interpolate(gettext('Unable to cast vote: %s'), [data.message]));


data.message !== undefined -> 'message' in data.responseJSON (and I think you can move var errors upwards to simplify things)
[data.message] -> [data.responseJSON.message]

int-y1 · 2022-10-28T08:00:42Z

resources/problem-vote.js

+                $.featherlight.close();
+            },
+            error: function (data) {
+                alert(interpolate(gettext('Unable to delete vote: %s'), [data.message]));


data.message -> data.responseJSON.message

Is this necessary? The earlier code didn't use responseJSON when looking up points.

Yes, the error path no longer alerts. I think it's because DeleteProblemVote.post returns a JsonResponse.

int-y1 · 2022-10-28T08:15:56Z

resources/problem-vote.js

+
+                var errors = data.responseJSON;
+                $('#points-error').text(typeof errors === 'object' && 'points' in errors ? errors.points[0] : '');
+                $('#note-error').text(typeof errors === 'object' && 'note' in errors ? errors.note[0] : '');


I guess typeof errors === 'object' is always true, so this condition can be simplified.

int-y1 · 2022-12-12T07:30:11Z

All previous comments have been addressed.

kiritofeng · 2022-12-20T20:26:37Z

judge/migrations/0133_add_voting_functionality.py

+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('judge', '0132_no_self_vote'),


Please update

quantum5 · 2022-12-20T23:24:02Z

judge/models/problem.py

@@ -434,6 +447,32 @@ def save(self, *args, **kwargs):

    save.alters_data = True

+    def is_solved_by(self, user):
+        # Return true if a full AC submission to the problem from the user exists.
+        return self.submission_set.filter(user=user.profile, result='AC', points__gte=F('problem__points')).exists()


This is going to have issues with 9999/10000 in a 5 point problem or something, but I couldn't be bothered to care.

quantum5 · 2022-12-20T23:30:44Z

judge/models/problem.py

+    def can_view(self):
+        return self >= VotePermission.VIEW
+
+    def can_vote(self):
+        return self >= VotePermission.VOTE


If I cared I'd ask for this to be @property, but this has dragged on for long enough.

quantum5 · 2022-12-20T23:33:05Z

templates/admin/judge/problem/change_form.html

           href="{% url 'admin:judge_submission_changelist' %}?problem__code={{ original.code }}">
            <i class="fa fa-lg fa-search-plus"></i>
-            <span class="text">{% trans "View submissions" %}</span>
+            <span class="text">{{ _('View submissions') }}</span>


Why this change? I can't even remember if this is a Django template or Jinja2.

Consistency with the rest of /templates. The only uses of {% trans "Foo" %} are in /templates/admin.

Yeah that would be because templates/admin are rendered as Django and not Jinja2. I am surprised this even works.

kiritofeng

It's yolo time

lakshy-gupta force-pushed the master branch 5 times, most recently from df78858 to 5fdf25b Compare April 19, 2021 18:59

wesley-a-leung reviewed Apr 19, 2021

View reviewed changes

judge/views/problem.py Outdated Show resolved Hide resolved

judge/views/problem.py Outdated Show resolved Hide resolved

judge/views/problem.py Outdated Show resolved Hide resolved

judge/views/problem.py Outdated Show resolved Hide resolved

Ninjaclasher requested changes Apr 19, 2021

View reviewed changes

kiritofeng requested changes Apr 19, 2021

View reviewed changes

lakshy-gupta force-pushed the master branch 3 times, most recently from 572a1e1 to 047ea40 Compare April 20, 2021 17:30

kiritofeng requested changes Apr 20, 2021

View reviewed changes

judge/admin/vote.py Outdated Show resolved Hide resolved

judge/views/problem.py Outdated Show resolved Hide resolved

templates/voting-stats.html Outdated Show resolved Hide resolved

templates/voting-stats.html Outdated Show resolved Hide resolved

fluix-dev suggested changes Apr 22, 2021

View reviewed changes

Ninjaclasher requested changes Apr 22, 2021

View reviewed changes

judge/models/problem.py Outdated Show resolved Hide resolved

judge/models/problem.py Outdated Show resolved Hide resolved

lakshy-gupta force-pushed the master branch 3 times, most recently from 2750beb to 85ef913 Compare July 27, 2021 19:32

Ninjaclasher reviewed Jul 27, 2021

View reviewed changes

ehhthing reviewed Jul 28, 2021

View reviewed changes

templates/problem/voting-controls.html Outdated Show resolved Hide resolved

templates/problem/voting-controls.html Outdated Show resolved Hide resolved

Ninjaclasher reviewed Aug 7, 2021

View reviewed changes

lakshy-gupta force-pushed the master branch from 1708608 to 42c3419 Compare August 7, 2021 02:37

Ninjaclasher reviewed Aug 7, 2021

View reviewed changes

judge/views/problem.py Outdated Show resolved Hide resolved

Ninjaclasher reviewed Aug 7, 2021

View reviewed changes

templates/problem/voting-controls.html Outdated Show resolved Hide resolved

lakshy-gupta force-pushed the master branch from 54ce823 to c0f98d4 Compare September 1, 2021 21:33

lakshy-gupta force-pushed the master branch from 3931912 to 9e862fc Compare September 13, 2021 00:45

kiritofeng force-pushed the master branch 5 times, most recently from 42c261f to 3636788 Compare October 28, 2022 06:16

Xyene reviewed Oct 28, 2022

View reviewed changes

templates/problem/vote-ajax.html Outdated Show resolved Hide resolved

int-y1 reviewed Oct 28, 2022

View reviewed changes

kiritofeng force-pushed the master branch from f3ccd6d to 85e7e26 Compare November 6, 2022 19:48

int-y1 force-pushed the master branch 4 times, most recently from 2be6f9c to 9d0a910 Compare December 12, 2022 07:10

int-y1 force-pushed the master branch from 9d0a910 to 70d9c08 Compare December 17, 2022 08:23

int-y1 added a commit to int-y1/online-judge that referenced this pull request Dec 19, 2022

don't pr this, it conflicts with DMOJ#1645

c742bed

kiritofeng reviewed Dec 20, 2022

View reviewed changes

int-y1 added 5 commits December 20, 2022 17:59

Add backend for problem point voting

c803bf7

Add migration for problem point voting

d654bec

Add view for problem point voting

b73368f

Add HTML/CSS for problem point voting

c1bea23

Fix problem solved condition

c07b4ba

int-y1 force-pushed the master branch from 70d9c08 to c07b4ba Compare December 20, 2022 23:06

quantum5 approved these changes Dec 20, 2022

View reviewed changes

quantum5 requested a review from kiritofeng December 20, 2022 23:37

kiritofeng approved these changes Dec 20, 2022

View reviewed changes

quantum5 merged commit 7f62071 into DMOJ:master Dec 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add problem points vote functionality #1645

Add problem points vote functionality #1645

lakshy-gupta commented Apr 19, 2021

codecov-commenter commented Apr 19, 2021 •

edited

Loading

kiritofeng commented Apr 19, 2021

lakshy-gupta commented Apr 19, 2021

ehhthing commented Apr 19, 2021

fluix-dev left a comment

lakshy-gupta commented Jul 27, 2021

Ninjaclasher left a comment

Xyene Oct 28, 2022

int-y1 Oct 28, 2022

int-y1 Dec 11, 2022 •

edited

Loading

int-y1 Oct 28, 2022

int-y1 Dec 11, 2022

int-y1 Oct 28, 2022 •

edited

Loading

int-y1 Dec 11, 2022

int-y1 Oct 28, 2022

Xyene Oct 28, 2022

int-y1 Oct 28, 2022

int-y1 Dec 11, 2022

int-y1 Oct 28, 2022 •

edited

Loading

int-y1 Dec 11, 2022

int-y1 commented Dec 12, 2022

kiritofeng Dec 20, 2022

int-y1 Dec 20, 2022

quantum5 Dec 20, 2022

quantum5 Dec 20, 2022

quantum5 Dec 20, 2022

int-y1 Dec 20, 2022

quantum5 Dec 20, 2022

kiritofeng left a comment



		class ProblemPointsVoteAdmin(admin.ModelAdmin):
		list_display = ('points', 'voter', 'problem', 'vote_time')

Add problem points vote functionality #1645

Add problem points vote functionality #1645

Conversation

lakshy-gupta commented Apr 19, 2021

codecov-commenter commented Apr 19, 2021 • edited Loading

Codecov Report

kiritofeng commented Apr 19, 2021

lakshy-gupta commented Apr 19, 2021

ehhthing commented Apr 19, 2021

fluix-dev left a comment

Choose a reason for hiding this comment

lakshy-gupta commented Jul 27, 2021

Ninjaclasher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

int-y1 Dec 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

int-y1 Oct 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

int-y1 Oct 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

int-y1 commented Dec 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiritofeng left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 19, 2021 •

edited

Loading

int-y1 Dec 11, 2022 •

edited

Loading

int-y1 Oct 28, 2022 •

edited

Loading

int-y1 Oct 28, 2022 •

edited

Loading