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

Added a journey page #201

Closed
wants to merge 1 commit into from
Closed

Added a journey page #201

wants to merge 1 commit into from

Conversation

anubhakushwaha
Copy link
Member

@anubhakushwaha anubhakushwaha commented Dec 18, 2017

Checklist

  • My branch is up-to-date with the upstream predev branch.
  • I have added necessary documentation (if appropriate).

Which issue does this PR fix?: fixes #100

If relevant, please include a screenshot.
screenshot from 2017-12-18 23-14-40


def journey(request):
journey_list = Journey.objects.all()
return render(request, 'journey.html' ,

Choose a reason for hiding this comment

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

whitespace before ','

@@ -38,3 +38,8 @@ def add_contest(request):

def submit_contest(request):
return render(request, 'contest_submission.html')

def journey(request):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1


def __str__(self):
return self.title

Choose a reason for hiding this comment

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

blank line at end of file

@@ -29,3 +29,11 @@ class Contest(models.Model):

def __str__(self):
return self.name

class Journey(models.Model):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('title', models.CharField(help_text='Journey title', max_length=128)),
('description', models.TextField(help_text='Session details', max_length=512)),

Choose a reason for hiding this comment

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

line too long (95 > 79 characters)

name='Journey',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('title', models.CharField(help_text='Journey title', max_length=128)),

Choose a reason for hiding this comment

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

line too long (87 > 79 characters)

migrations.CreateModel(
name='Journey',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),

Choose a reason for hiding this comment

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

line too long (114 > 79 characters)

admin.site.register(Contest, contestAdmin)

Choose a reason for hiding this comment

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

blank line at end of file



class chatSessionAdmin(admin.ModelAdmin):
list_display = ('title', 'start_date')

class journeyAdmin(admin.ModelAdmin):

Choose a reason for hiding this comment

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

expected 2 blank lines, found 1

@anubhakushwaha
Copy link
Member Author

@tapasweni-pathak @vaibhavsingh97 @jarifibrahim @nikhita Please review and suggestions on improving it would be great, thanks

start_date = models.DateField(null=True)

def __str__(self):
return self.title

Choose a reason for hiding this comment

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

no newline at end of file

('id', models.AutoField(auto_created=True,
primary_key=True, serialize=False, verbose_name='ID')),
('title', models.CharField(help_text='Journey title',
max_length=128)),

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

fields=[
('id', models.AutoField(auto_created=True,
primary_key=True, serialize=False, verbose_name='ID')),
('title', models.CharField(help_text='Journey title',

Choose a reason for hiding this comment

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

trailing whitespace

name='Journey',
fields=[
('id', models.AutoField(auto_created=True,
primary_key=True, serialize=False, verbose_name='ID')),

Choose a reason for hiding this comment

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

continuation line under-indented for visual indent

migrations.CreateModel(
name='Journey',
fields=[
('id', models.AutoField(auto_created=True,

Choose a reason for hiding this comment

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

trailing whitespace

@@ -26,4 +30,5 @@ def approve_contest(self, request, queryset):


admin.site.register(chatSession, chatSessionAdmin)
admin.site.register(Contest, contestAdmin)
admin.site.register(Journey, journeyAdmin)
admin.site.register(Contest, contestAdmin)

Choose a reason for hiding this comment

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

no newline at end of file


def __str__(self):
return self.title

Choose a reason for hiding this comment

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

blank line at end of file
blank line contains whitespace

class Journey(models.Model):
title = models.CharField(max_length=128, help_text="Journey title")
start_date = models.DateField(null=True)

Copy link
Member

Choose a reason for hiding this comment

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

Can we add a description field too?

Copy link
Member

Choose a reason for hiding this comment

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

We are good w/ these two.

def journey(request):
journey_list = Journey.objects.all()
return render(request, 'journey.html',
context={'Journey': journey_list})
Copy link
Member

@jarifibrahim jarifibrahim Dec 18, 2017

Choose a reason for hiding this comment

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

You might want to sort the journey_list by the start_date before sending it to the HTML page. This ensures that the data we send out of the view is chronologically ordered.
You can use Managers[0] to achieve this or you could do something like Journey.objects.all().filteryBy('start_date')

[0] - https://docs.djangoproject.com/en/2.0/topics/db/managers/

Copy link
Member

Choose a reason for hiding this comment

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

fixed w/ Journey.objects.order_by('start_date').

@@ -33,7 +33,8 @@
</ul>
</li>
<li><a href="https://github.com/OpenSourceHelpCommunity" target="_blank">Resources</a></li>
<li><a href="{% url 'contests' %}" target="_blank">Contests</a></li>
<li><a href="{% url 'contests' %}" target="_blank">Contests</a></li>
<li><a href="{% url 'journey' %}" target="_blank">Journey</a></li>
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should put target="_blank" here. The Journey page is part of the OSHC page.

Copy link
Member

@tapaswenipathak tapaswenipathak Dec 24, 2017

Choose a reason for hiding this comment

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

These needs to be removed for quite a few.

@jarifibrahim
Copy link
Member

@anubhakushwaha Is there any way we could make the timeline responsive?

@anubhakushwaha
Copy link
Member Author

@jarifibrahim Hey thanks for reviewing will do the changes, responsive as in? The current behavior allows new events to be added via the admin. Please help me understand this better.

@jarifibrahim
Copy link
Member

@anubhakushwaha By responsive I meant is the timeline mobile device friendly? As far as I know, the OSHC website is mobile friendly, so it would be great if we could make the timeline mobile friendly as well.

@vaibhavsingh97
Copy link
Member

@anubhakushwaha Can we have live demo link please

@anubhakushwaha
Copy link
Member Author

@vaibhavsingh97 Live demo for?

Copy link
Member

@tapaswenipathak tapaswenipathak 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 sending this through @anubhakushwaha! 🎉

I have fixed few minor reviews that you have got till now. We need to make this work even on a mobile sized/small screens.

@tapaswenipathak
Copy link
Member

tapaswenipathak commented Dec 24, 2017

This is merged after few fixes w/ 7922fee, live here.

@anubhakushwaha
Copy link
Member Author

@tapasweni-pathak Thank you for doing the changes.

We need to make this work even on a mobile sized/small screens.

I will update you on this.

@anubhakushwaha anubhakushwaha deleted the predev branch December 27, 2017 10:38
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.

Add a team page
5 participants