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

Under improvement -Event page #236

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

Aarti002
Copy link

Lots of work still needs to be done , hope u will understand and please let me know if something is wrong or if i need to add on something! @garg3133
Thank you!

@Jagrati-Bot
Copy link
Collaborator

Congratulations for making your first Pull Request at JagratiWebApp!! 🎉 Someone from our team will review it soon.

Copy link
Owner

@garg3133 garg3133 left a comment

Choose a reason for hiding this comment

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

  • Put all the static files in static/events/ directory only.
  • Use the images directly from static/home/images, don't add any additional dummy images anywhere.
  • Indent all the files properly, they are currently not readable (use Alt + Shift + F in VSCode).
  • Revert back the changes in all the files which are not related to this PR.

First do all these changes so that your PR is readable. After that, I'll test it on my system and review it again.

@@ -8,7 +8,7 @@ class EventAdmin(admin.ModelAdmin):
list_display = ('title', 'schedule', 'venue')
search_fields = ('title', 'venue')
ordering = ('-schedule',)

Copy link
Owner

Choose a reason for hiding this comment

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

Revert back all the changes in this file. Remove this tab and an extra line inserted at the end of this file.

@@ -15,7 +15,7 @@ class Event(models.Model):
schedule = models.DateTimeField()
venue = models.CharField(max_length=50)
thumbnail = models.ImageField(
upload_to=event_thumbmail_path, null=True, blank=True)
upload_to='events/images', null=True, blank=True)
Copy link
Owner

Choose a reason for hiding this comment

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

It is already fetching the path where the thumbnail needs to be saved from the event_thumbnail_path function. You should revert back this change.

If you want to get the URL of the thumbnail, you can simply use <event>.get_thumbnail_url.

images = models.ImageField(upload_to = 'gallery/image',null=True ,blank=True)

def __str__(self):
return self.post
Copy link
Owner

Choose a reason for hiding this comment

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

Why a new Gallery model?? Gallery model is already present in the file (last model). Also, what do you mean by the post field here?

padding-left: 0,
padding-right: 0;
}
{% endblock %}
Copy link
Owner

Choose a reason for hiding this comment

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

Indent the CSS above properly, it is not readable. You may use Alt + Shift + F in VSCode to auto-indent everything.

@@ -1,9 +1,13 @@
from django.contrib import admin
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see admin being used anywhere.

from . import views

app_name = 'events'
urlpatterns = [
path('', views.index, name='index'),
path('add/', views.add_event, name='add_event'),
]
path('show_event/<int:myid>',views.show_event,name='show_event'),
Copy link
Owner

Choose a reason for hiding this comment

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

Use pk instead of myid to represent a primary key input.

from . import views

app_name = 'events'
urlpatterns = [
path('', views.index, name='index'),
path('add/', views.add_event, name='add_event'),
]
path('show_event/<int:myid>',views.show_event,name='show_event'),
]+static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)
Copy link
Owner

Choose a reason for hiding this comment

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

No need to add this here. It is already added in Jagrati.urls.py.

home/urls.py Outdated
@@ -8,4 +8,5 @@
path('dashboard/', views.dashboard, name='dashboard'),
path('dashboard/update_cwhw/', views.update_cwhw, name='update_cwhw'),
path('ajax/dashboard/', views.ajax_dashboard, name='ajax_dashboard'),

Copy link
Owner

Choose a reason for hiding this comment

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

Remove this extra line from here.

@@ -15,6 +17,8 @@
min-height: 100vh;
/* height: 100%; */
}
{% block style %}
{% endblock %}
Copy link
Owner

Choose a reason for hiding this comment

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

Don't add any additional CSS and make any changes in templates/layout.html directly. Doing this will break our all other pages.


left:0;
bottom: 0;
width:100%;
Copy link
Owner

Choose a reason for hiding this comment

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

Don't make any changes in some file which directly affects our all other pages.

@@ -15,7 +15,7 @@ class Event(models.Model):
schedule = models.DateTimeField()
venue = models.CharField(max_length=50)
thumbnail = models.ImageField(
upload_to='events/images', null=True, blank=True)
upload_to='<event>.get_thumbnail_url', null=True, blank=True)
Copy link
Owner

Choose a reason for hiding this comment

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

This isn't what I meant. Keep upload_to=event_thumbnail_path here.

I meant, if you want to get the URL of the thumbnail image anywhere in views or templates, you can just use the get_thumbnail_url property. For ex, if event is an instance of Event table, you can do event.get_thumbnail_url and it will return you the URL of thumbnail of that event.

@garg3133 garg3133 linked an issue May 1, 2021 that may be closed by this pull request
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.

[feature]: Adding event page
3 participants