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

Updates events page. #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

paurushkg
Copy link

…nts to organise from the end user

@sumit4613 sumit4613 changed the title solved issue no.14 and opened a portal to recieve request for new eve… Updates events page. Aug 7, 2020
@sumit4613
Copy link
Member

@paurushofficial please list the changes here, also make the title as concise as you can.
Other details should be in description.

Copy link
Member

@sumit4613 sumit4613 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, but you should fix the suggested changes.

events/forms.py Outdated


class ParticipantForm(ModelForm):
class Meta:
model = EventParticipant
fields = ['title', 'student_name',
'email_id', 'mobile_number', 'roll_no', 'branch']

class requestEventForm(ModelForm):
Copy link
Member

Choose a reason for hiding this comment

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

Issues here :

  • You're not following PEP8 conventions here.
  • According to PEP8 there should be 2 blank lines after a class.
  • Whenever you create class, it should be in caps. eg. class RequestEventForm(ModelForm)
  • Also try to include docstrings in your class/methods.

events/models.py Outdated
@@ -53,3 +53,13 @@ class EventParticipant(TimeStampedModel):

def __str__(self):
return self.student_name

class requestEvent(models.Model):
Copy link
Member

Choose a reason for hiding this comment

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

  • Here also, In classes, you should follow CamelCase notation.
  • Try to add docstrings to explain what the class/method is doing.
  • You can also add docstrings for existing classes/methods.

events/views.py Outdated
Comment on lines 12 to 14
#numberOfEvents = len(events)
#if (numberOfEvents) == 0:
# events = 0
Copy link
Member

Choose a reason for hiding this comment

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

Why did you left commented code here. You should remove the unused code.

events/views.py Outdated
Comment on lines 52 to 64
def requestEvent(request):
if request.method == 'POST':
form = requestEventForm(request.POST)
if form.is_valid():
form.save()
your_name = form.cleaned_data.get('your_name')
title = form.cleaned_data.get('title')
messages.success(
request, f'Thank you { your_name } for requesting to organise { title }, you will be contacted shortly!')
return redirect('homepage')
else:
form = requestEventForm()
return render(request, 'event_register.html', {'form': form})
Copy link
Member

Choose a reason for hiding this comment

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

While creating methods, you should not write like this.
Use this notation, def request_event(request):

@lgtm-com
Copy link

lgtm-com bot commented Aug 7, 2020

This pull request introduces 1 alert when merging d0fe5cf into a8904e0 - view on LGTM.com

new alerts:

  • 1 for Unused import

@sumit4613
Copy link
Member

@paurushofficial Sorry for being this late on replying to your PR. Could you please share some screenshots so that I can see it's working as expected.

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