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

Closes #1451 Added user categories to profile and settings #1459

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

TimOsahenru
Copy link
Contributor

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.

1
2

project/accounts/forms.py Outdated Show resolved Hide resolved
@brylie
Copy link
Member

brylie commented Nov 2, 2022

This looks good so far. How does it look in the UI?

@TimOsahenru
Copy link
Contributor Author

same result sir, same result. I attached a screenshot.

Is it a UI issue or maybe I'm not rendering the right form choice, if the latter is the problem please how do I resolve this I've tried everything I could come up with. thanks
2

@brylie
Copy link
Member

brylie commented Nov 2, 2022

Double-check the form template to see what the code looks like.

@TimOsahenru
Copy link
Contributor Author

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

Comment on lines 48 to 56
<div class="divider"></div>
<div class="section-title">CATEGORIES</div>
{% for category in categories %}
<div class="about-me">{{ category.name }}</div>
{% endfor %}
Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Member

@brylie brylie Nov 5, 2022

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.

Copy link
Contributor Author

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

Copy link
Member

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

@@ -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()
Copy link

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.

@codeclimate
Copy link

codeclimate bot commented Nov 5, 2022

Code Climate has analyzed commit 610fde0 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Clarity 1

View more on Code Climate.

Comment on lines +64 to +67
<div class="col s12 m6">
{{form.categories.errors}}
{{form.categories.label_tag}}
{{form.categories}}
Copy link
Member

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.

https://stackoverflow.com

Copy link
Contributor Author

@TimOsahenru TimOsahenru Nov 6, 2022

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?

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

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?

Copy link
Member

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.

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