-
Notifications
You must be signed in to change notification settings - Fork 995
Fixed notifications and added posts to profile #236
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
base: develop
Are you sure you want to change the base?
Changes from 4 commits
784f0d3
6a1630d
91f3723
3b01f67
8f33e86
d26afac
dccced5
520c756
f170090
9d990c6
1ce14d7
3f4886c
b5c5d2f
658aa2b
a7686ef
72a176e
a276fa6
e9b1d82
39ed542
1ff460a
935531b
b75989d
d74f7ca
9398df6
58724e9
ed0a42e
79d9e3d
b0714eb
b67a5a3
87f5c6e
e791eec
a0c2b0e
8609db1
85d2b50
12075aa
4996a4e
e38e5c2
5b3f22b
315a52c
c722869
9aa0816
57b42dc
84d99d0
4142979
b86f76f
3a78664
361a3db
923ca80
aa70650
401aa5a
55bf1f5
538a9a4
ad30c6f
bbf28b3
c6dba0e
e9c55d8
c5777f4
59f5aa5
739763a
ca8230c
fbf5e7d
54dadda
386597c
6195b8c
36e5d44
5a2fd9d
b4c61ed
d8a05ec
7007148
79fee95
f13fcc4
cd8084f
de240ab
095df42
a2dfda6
5ca1a05
975795f
207350f
4ef5bc0
b7206a8
a67b41f
a152505
1c69c58
bbabd79
a13db4b
e8a0570
7cd634a
df2ad16
4965da8
5830d75
a48d506
7206a6c
aea8344
513cedb
e2adf9a
82b0d9f
ae6729e
059db93
5138b10
9a10ea9
dd31598
3911ed2
4283740
561458b
8bdf801
4521e63
0d89443
3429124
1dcb859
4f1c63d
1b053a9
1c68ff0
90b28a7
143e632
cd6bc7e
9ce3eca
d75052a
a1c3eb4
bee3238
b0874ee
06776f4
8a9cd16
51087cd
3633422
b128794
dd03e48
d24af92
822140e
81a117a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| {% extends 'base.html' %} | ||
| {% load static i18n humanize %} | ||
| {% load thumbnail %} | ||
|
|
||
| {% block head %} | ||
| <link href="{% static 'css/news.css' %}?v=1" rel="stylesheet"> | ||
| {% endblock head %} | ||
|
|
||
| {% block content %} | ||
| <div class="page-header"> | ||
| <h1>News</h1> | ||
| </div> | ||
| <ul class="stream"> | ||
| {% include 'news/news_single.html' with news=news %} | ||
| </ul> | ||
| {% endblock content %} | ||
|
|
||
| {% block modal %} | ||
| <script src="{% static 'js/jquery.waypoints.min.js' %}" type="text/javascript"></script> | ||
| <script src="{% static 'js/infinite.min.js' %}" type="text/javascript"></script> | ||
| <script src="{% static 'js/news.js' %}" type="text/javascript"></script> | ||
| {% endblock modal %} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <div class="infinite-container" style="margin-top: 20px"> | ||
| <ul class="stream"> | ||
| {% for news in user_activity %} | ||
| {% include 'news/news_single.html' with news=news %} | ||
| {% endfor %} | ||
| </ul> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,14 +7,21 @@ | |
| from django.views.generic import DetailView, ListView, RedirectView, UpdateView | ||
| from django.conf import settings as django_settings | ||
| from .models import User | ||
| from ..news.models import News | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This import is a bad pattern, it is preferable to use base project imports using absolute references. |
||
|
|
||
|
|
||
| class UserDetailView(LoginRequiredMixin, DetailView): | ||
| model = User | ||
| paginate_by = 20 | ||
gusbakker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| # These next two lines tell the view to index lookups by username | ||
| slug_field = "username" | ||
| slug_url_kwarg = "username" | ||
|
|
||
| def get_context_data(self, *args, **kwargs): | ||
| context = super().get_context_data(*args, **kwargs) | ||
| context['user_activity'] = News.objects.filter(user=context['user']) | ||
| return context | ||
|
|
||
|
|
||
| class UserRedirectView(LoginRequiredMixin, RedirectView): | ||
| permanent = False | ||
|
|
@@ -92,10 +99,10 @@ def upload_picture(request): | |
| im.thumbnail(new_size, Image.ANTIALIAS) | ||
| im.save(filename) | ||
|
|
||
| return redirect("/users/picture/?upload_picture=uploaded") | ||
| return redirect("/picture/?upload_picture=uploaded") | ||
|
|
||
| except Exception: | ||
| return redirect("/users/picture/") | ||
| return redirect("/picture/") | ||
|
|
||
|
|
||
| @login_required | ||
|
|
@@ -126,4 +133,4 @@ def save_uploaded_picture(request): | |
| except Exception: | ||
| pass | ||
|
|
||
| return redirect("/users/picture/") | ||
| return redirect("/picture/") | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,6 @@ | |
| # Django Admin, use {% url 'admin:index' %} | ||
| url(settings.ADMIN_URL, admin.site.urls), | ||
| # User management | ||
| url(r"^users/", include("bootcamp.users.urls", namespace="users")), | ||
| url(r"^accounts/", include("allauth.urls")), | ||
| # Third party apps here | ||
| url(r"^comments/", include("django_comments.urls")), | ||
|
|
@@ -33,6 +32,7 @@ | |
| url(r"^messages/", include("bootcamp.messager.urls", namespace="messager")), | ||
| url(r"^qa/", include("bootcamp.qa.urls", namespace="qa")), | ||
| url(r"^search/", include("bootcamp.search.urls", namespace="search")), | ||
| url("", include("bootcamp.users.urls", namespace="users")), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't remove the line and add it in a different place unless necessary, this changes brakes the traceability for future references.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would you do this? So users URL follow the pattern domain/user instead of domain/users/user. The line still there. I just replaced the first parameter "users/" by ""
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh no, I'm actually referring to you deleting the URL from line 20 and creating it again in line 35. I actually understand why you want to remove the |
||
| ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) | ||
|
|
||
| if settings.DEBUG: | ||
|
|
||
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.
What is the purpose of this URL pattern? It looks like it is to show individual news on a different page, which in time render useless the original feature. If that's the intent, what is the logic behind it? And if it only makes a call to a single element, is it necessary to make a pattern that complex? It looks like the simpler pattern added with
path()will suffice.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.
since in this version we are dealing with UUID instead of simple id, this pattern allows to show that uuid. But maybe could be simplified
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.
I haven't had the time of migrating all the URL from the old pattern using
url()to the new method given in Django2 usingpath(), which actually has a<uuid>identifying structure.