-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
Add hackathon projects #5414
Add hackathon projects #5414
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5414 +/- ##
==========================================
+ Coverage 29.83% 30.36% +0.53%
==========================================
Files 241 242 +1
Lines 20433 21954 +1521
Branches 2926 3445 +519
==========================================
+ Hits 6096 6667 +571
- Misses 14086 14984 +898
- Partials 251 303 +52
Continue to review full report at Codecov.
|
looks great but the vidyard link doesn't work :( @octavioamu |
fixed, is the same video I posted in the squad channel https://share.vidyard.com/watch/h5d5nv916eRiAeYK5ErnHL? |
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.
Looks amazing! Really nice work - just a few suggestions and fixes
@@ -53,6 +53,11 @@ | |||
{% endif %} | |||
<span class="d-block font-title-lg"> | |||
{{ hackathon.start_date|date:"M j, Y" }} - {{ hackathon.end_date|date:"M j, Y" }} | |||
{% if hackathon.end_date|timeuntil >= "1 min" %} |
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.
Does the comparison here actually work? I don't think something like "8 hours" >= "1 min" works with string comparisons, but if im wrong I will be amazed at Django's usefulness :) If it turns out we need better comparison logic, according to the Django docs timeuntil
will return the string "0 minutes" if the end date here is already passed so we can just check {% if hackathon.end_date|timeuntil != "0 minutes" %}
https://docs.djangoproject.com/en/2.2/ref/templates/builtins/#timeuntil
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.
Actually it works pretty well I used also on hackathon onboard. Django magic do it with the actual dates not strings .
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.
thats crazy! magic django!
profile = request.user.profile if request.user.is_authenticated and hasattr(request.user, 'profile') else None | ||
try: | ||
bounty = Bounty.objects.current().get(id=bounty_id) | ||
projects = HackathonProject.objects.filter(bounty__standard_bounties_id=bounty.standard_bounties_id, profiles__id=profile.id).nocache() |
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.
For the filter query here, could we just do HackathonProject.objects.filter(bounty=bounty)
?
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.
That will work with current bounty? Because when submission the bounty id is a new one not associated with the hackathon project
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.
ah I see, makes sense!
'projects': projects_paginated, | ||
'order_by': order_by, | ||
'filters': filters, | ||
'query': q.split |
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.
do we wanna do a check if q
exists ?
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.
is already request.GET.get('q', '') and I use on the view if exist to show the terms is being query
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.
left a few comments but otherwise all chill :)
Description
Also:
required
select wasn't working/Project card
Project Modal add/edition
Multiple Projects handling
Refers/Fixes
Ref: #5322 #5420
Testing
Tested locally with a lot of diff situations, and multiple bounties, hackathon and projects
https://share.vidyard.com/watch/h5d5nv916eRiAeYK5ErnHL?