-
Notifications
You must be signed in to change notification settings - Fork 41
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
#A03. As Admin, I would like to update existing Hackathon events. #106
#A03. As Admin, I would like to update existing Hackathon events. #106
Conversation
…ow selecting past date for start date
… or in progress hackathons
…rectly in edit form
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.
LGTM
@hebs87 please have a quick look at my comments and let me know when I can merge it.
hackathon/views.py
Outdated
|
||
if request.method == 'GET': | ||
|
||
template = "hackathon/create-event.html" |
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.
Don't think you need to declare the template, you can just pass it directly in the render
function
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.
This is just a personal preference, but if the preference here is to pass it in directly then that's cool. I've made the change, it'll be visible when I push it to the existing PR 👍
hackathon/views.py
Outdated
hackathon = get_object_or_404(Hackathon, pk=hackathon_id) | ||
|
||
if request.method == 'GET': | ||
|
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.
Please remove the unneeded blank line
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.
Done, this will be visible when I push the change to the existing PR.
hackathon/urls.py
Outdated
@@ -1,9 +1,10 @@ | |||
from django.urls import path | |||
|
|||
from .views import HackathonListView, create_hackathon, delete_hackathon | |||
from .views import HackathonListView, create_hackathon, update_hackathon, delete_hackathon |
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.
This line is too long as per PEP8 it should be less than 79 chars. Please make sure that all of the lines/files adhere to this.
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.
Done, this will be visible once I push the changes to the existing PR.
hackathon/views.py
Outdated
messages.success(request, f'Thanks, {hackathon.display_name} has been successfully updated!') | ||
return redirect("hackathon:hackathon-list") | ||
|
||
pass |
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.
Can you please remove the pass
(this is not required) and the unneeded blank line above?
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.
Done. This will be visible once I push the changes to the existing PR.
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.
LGTM
User Story ID:
Description of PR:
All changes made are outlined below:
Tests:
Manually tested all implemented functionality, which is all working as expected.
Validated all code in relevant validators and all passed - HTML validator threw errors for Django template tags, but no other errors.
Know bugs/errors:
No known bugs or errors after testing.
Screenshots:
Does this introduce a breaking change