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

Sunum başvuruları #60

Closed
wants to merge 2 commits into from
Closed

Conversation

efe
Copy link
Contributor

@efe efe commented Jul 14, 2017

Amerika'yı yeniden keşfetmemek adına, sitede hali hazırda iş ilanı eklemek için kullanılan formu kullanarak, sunum başvuruları yapılabilecek bir view oluşturmayı amaçladım.

Bunun için önce PresentationRequest isimli bir model oluşturdum. Issue'da yazan ve bunlara ek olarak is_approved ve is_reviewed fieldlarını ekledim.

Form'da uzunluğu fazla olan verbose_namelerden dolayı, bazı CSS parametrelerini değiştirdim.
CSS'de #github ile ilgili değişiklik, daha önceki PR'da oluşan sonradan fark ettiğim bir hatamı çözmek için. İstersen onu ayrıca da açabilirim.

Geri bildirimlere açığım! :)

- PresentationRequest model init and migration.
- PresentationRequestForm init.
- test_request_form is implemented.
@@ -40,3 +40,17 @@ def test_grouper(self):
response = self.client.get(reverse('presentations:index'))
for i in range(10, 15):
self.assertContains(response, "%s" % date % i)

def test_request_form(self):
Copy link
Member

Choose a reason for hiding this comment

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

Burada sadece redirect e bakmak disinda yaptigin request db ye kaydedilmis mi diye de bakman iyi olur.

Bide form error case inde de bi test yazarsan guzel olabilir.

presenter_twitter_username = models.CharField(verbose_name='Twitter kullanici adi', max_length=255)
presenter_github_username = models.CharField(verbose_name='Github kullanici adi', max_length=255)
presentation_title = models.CharField(verbose_name='Sunum Basligi', max_length=255)
presentation_type = models.CharField(verbose_name='Sunum Turu',choices=presentation_type_choices, max_length=255)
Copy link
Member

Choose a reason for hiding this comment

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

surada bi pep8 hatasi var galiba

messages.success(self.request, self.success_message)
return super(PresentationRequestView, self).form_valid(form)

def get_success_url(self):
Copy link
Member

Choose a reason for hiding this comment

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

success urli fonksiyon icine yazmak disinda PresentationRequestView icine success_url diye eklerek onu da reverse_lazy de yapabilirdin.

Bence get_success_url daha cok custom isler yapmak yada url icinde dinamik parametreler icin override edilmeli.

('soru-cevap', "Soru & Cevap"),
)

presenter_name = models.CharField(verbose_name='Ad, Soyad', max_length=255)
Copy link
Member

Choose a reason for hiding this comment

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

buradaki max_length ler sanki biraz fazla olmus.

@bahattincinic
Copy link
Member

Eline saglik @efe

Bu sunum basvuru sayfasinin linkini sitede nereye koyucaz @berkerpeksag ?

- Test iyileştirmesi.
- PEP8
- reverse_lazy()
@berkerpeksag
Copy link
Contributor

Teşekkürler Efe :) Yanlış hatırlamıyorsam @fatiherikli ile harici bir servis kullanmaya karar vermiştik bu iş için. Zira, ihtiyaçlarımızı görecek bir aracın organizasyon ekibindeki isimlere her başvuruyu e-posta olarak göndermesi gibi irili ufaklı bir sürü özelliği içermesi gerekiyor. Bunları teker teker kendi içimizde implemente etmek yerine hazır bir servisi kullanıp siteye entegre etmek daha iyi olur bence.

@efe
Copy link
Contributor Author

efe commented Jul 14, 2017

Anladım. Baktığım kadarı ile TypeForm'un HTML'e form gömme (embed) özelliği var. Böyle entegre edilebilir sanırım. Başvuruların yönetimi de TypeForm'dan yapılacağı için, elimi issue'dan çekiyorum. :)

Teşekkür ettim.

@efe efe closed this Jul 14, 2017
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.

3 participants