-
Notifications
You must be signed in to change notification settings - Fork 344
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
Closes #1451 Added user categories to profile and settings #1459
base: develop
Are you sure you want to change the base?
Conversation
This looks good so far. How does it look in the UI? |
Double-check the form template to see what the code looks like. |
Oh I did @brylie and I think the form is missing this tag I tried solving the issue but still couldn't I think it requires some UI skills(CSS) if there's a quick fix to this challenge please do let me know so I can work on this issue and have it closed for another one. Thanks |
<div class="divider"></div> | ||
<div class="section-title">CATEGORIES</div> | ||
{% for category in categories %} | ||
<div class="about-me">{{ category.name }}</div> | ||
{% endfor %} |
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.
We don't need to loop over all of the categories here, but would instead loop over the profile.categories
and list them horizontally.
Since div
is a block element, it causes the categories to stack. Instead of wrapping each category in its own div
display the categories in a paragraph or span of text, separated by commas.
{% for category in profile.categories %}
{{ category }},
{% endfor %}
Notice that the above code lists all the profile categories separated by commas. For bonus points, we can later figure out how to make it, so the last category in the list doesn't have a trailing comma.
References
Check out the following documents for further info related to this comment.
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.
Oh that's fine @brylie but, I already performed the logic in the backend
def get(self, request, username=None):
profile = get_object_or_404(Profile, user__username=username)
categories = profile.categories.all()
or will you'll prefer I do it using the Django templating syntax?
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.
Using the profile
object already available in the template would require less code since the categories property is already part of the profile
.
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.
Hello @brylie please I'm getting a server error after implementing this
here is the error message
TypeError: 'ManyRelatedManager' object is not iterable
any suggestions on what I'm missing out?
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.
That error seems common enough to find an answer via Google or Stack Overflow. Let me know what search results you get and what you tried to resolve the error. It would also help to see what the exact line of code is that produces the error.
It may be worth considering that the code I suggested during the code review won't work without modification since I was going from memory.
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.
Thanks @brylie I just pushed my code for review, I also helped fix the last trailing comma you mentioned, with the best logic I could come up with for now before we find something more permanent.
Also, I don't remember you mentioning anything about the issue I raised in the settings.html file
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'll add a comment in settings.html
7a2e9c7
to
ed37c54
Compare
@@ -178,12 +179,14 @@ class UserProfileView(LoginRequiredMixin, View): | |||
|
|||
def get(self, request, username=None): | |||
profile = get_object_or_404(Profile, user__username=username) | |||
# categories = profile.categories.all() |
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.
Remove this commented out code.
Code Climate has analyzed commit 610fde0 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
<div class="col s12 m6"> | ||
{{form.categories.errors}} | ||
{{form.categories.label_tag}} | ||
{{form.categories}} |
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.
This looks correct. I'm not sure why the field isn't rendering. One thing to try would be to post this code and the form definition to a Stack Overflow question to get help from the broader community.
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.
Alright I'll do just that. but, I think it's a UI issue, I made an observation as to this form section missing out this tag
<select>
<option></option>
</select>
could that be the issue?
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.
The only UI here is whatever we write. Since we are using a Django Form to render UI fields, we need to figure out the correct way to render a ModelMultiSelect form field. Hence, asking on StackOverflow to see if what we are trying is correct.
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.
thanks @brylie, I've asked on stackoverflow hoping to get answer soon.
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.
One thing that might help in this case would be to create an empty testing project to work directly with the form fields. Just create a new Django project with a single app. Define two models in the app with one ForeignKey field. Create a form/template/view and see if the form field renders correctly. That will isolate the problem to give us an idea of how it should work.
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.
Great thoughts @brylie. I'll do just that right away.
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.
Can I pick this up, if this issue has not been resolved?
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.
Sure thing. You may need to start a fresh branch, since this one has conflicts.
Users can now see their choiced category on their profile. but I'm having a bit of a challenge displaying the list of categories; the issue is, it just display's as a plain list instead of a clickable selective list, please I need assistance on how to execute this, I also tried using ModelMultipleChoiceField still got NO result, below is a screenshot of my code and a description of my challenge. Please I need your assistance.