-
Notifications
You must be signed in to change notification settings - Fork 25
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
Suggest presentation form added #72
Suggest presentation form added #72
Conversation
Python 3 django 2 migration
… 3.6 added instead
@@ -112,6 +112,8 @@ | |||
'people', | |||
'presentations', | |||
'blog', | |||
|
|||
'suggestpresentation', |
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.
Bunu yeni bir app yerine presentations app'i altına alabiliriz.
@@ -0,0 +1,18 @@ | |||
from django.db import models | |||
|
|||
# Create your models here. |
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.
Nit: Bu yorumu uçurabiliriz.
|
||
# Create your models here. | ||
|
||
class suggest(models.Model): |
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.
suggest -> Suggest
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.
Suggestion
daha mi iyi olur ?
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.
Bence de Suggestion daha iyi.
# Create your models here. | ||
|
||
class suggest(models.Model): | ||
name = models.CharField( |
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.
Yeni kodda hanging indentation kullansak daha iyi olur bence:
name = models.CharField(
max_length=255,
verbose_name='Ad ve Soyad',
)
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.
Birde kod icinde turkce bisi kullanmayip translation ile mi cevirsek acep ya 😄Basima bisi gelmicek ise kod icinde turkce yazilmis bisi gormek hosuma gitmiyor.
verbose_name='Ad ve Soyad') | ||
email = models.EmailField( | ||
max_length=255,) | ||
content = models.TextField(verbose_name="Öneriniz") |
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.
Nit: Çift veya tek tırnak arasında bir tercihim yok ama bütün kodda ikisinden birini tutarlı olarak kullansak daha iyi olur.
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.
docstring
harici çift tırnak kullanmıyorum :)
from django.views.generic import CreateView | ||
from .models import suggest | ||
|
||
# Create your views here. |
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.
Nit: Bu yorumu da uçurabiliriz.
class CreateSuggestPresentation(CreateView): | ||
model = suggest | ||
form_class = suggestPresentationForm | ||
success_message = "Öneriniz başarıyla iletildi." |
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.
Buraya "X gün içerisinde geri dönüş yapılacak." gibi bir şey de eklesek mi acaba? @samet @halilkaya
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.
Olur. "3 gün içerisinde değerlendirilip sizinle irtibata geçilecektir." gibi bir sey diyebiliriz.
|
||
def form_valid(self, form): | ||
messages.success(self.request, self.success_message) | ||
return super(CreateSuggestPresentation, self).form_valid(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.
super().form_valid(form)
|
||
def get_success_url(self): | ||
return reverse("suggestpresentation:index") | ||
|
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.
Nit: Bu boş satıra gerek yok.
success_message = "Öneriniz başarıyla iletildi." | ||
|
||
def form_valid(self, form): | ||
messages.success(self.request, self.success_message) |
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.
Bu pattern'ı halihazırda başka yerlerde de kullanıyoruz. Bence bunu mixin haline getirip diğer applerde de kullanabiliriz. Bunu bu PR içinde yapmaya gerek yok. Ayrı bir issue açacağım.
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.
Açtım: #73
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.
Bu mixin eklendi: 6d02fc5
Bu branch'i rebase edip mixin'i kullanırsak güzel olur.
from django import forms | ||
from .models import suggest | ||
|
||
class suggestPresentationForm(forms.ModelForm): |
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.
Bu da SuggestPresentationForm
olsa daha iyi olur.
from django.apps import AppConfig | ||
|
||
|
||
class SuggestpresentationConfig(AppConfig): |
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.
Burasi da SuggestPresentationConfig
olsa daha iyi olur.
@@ -38,6 +38,7 @@ <h1 id="logo" class="title"> | |||
<li><a href="/hakkimizda/">nedir?</a></li> | |||
<li><a href="{% url "presentations:index" %}">sunumlar</a></li> | |||
<li><a href="{% url "people:index" %}">insanlar</a></li> | |||
<li><a href="{% url "suggestpresentation:index" %}">sunum önerisi</a></li> |
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.
Sunum onerisi icin ayri bir menu eklemek yerine sunumlar
sayfasinin icine bir button olarak eklesek daha iyi olmaz mi?
@@ -38,6 +38,7 @@ <h1 id="logo" class="title"> | |||
<li><a href="/hakkimizda/">nedir?</a></li> |
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.
hard-coded url kullanılmasa iyi olur
d14fcad
to
b3b2a9a
Compare
Sunum önerisi oluşturma formu eklendi.