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

#284 - Added the list all user organizations page #hacktoberfest #315

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

huogerac
Copy link

Hi,
I'm trying to contribute to the organization topic.
The idea is first having the list the user organizations #284 done.

Please let me know what is missing.

@luanfonceca
Copy link
Owner

luanfonceca commented Oct 16, 2017

@huogerac, this issue was assigned to @tulikavijay. You didnt read the comments on the issue?

Sorry but I can't accept this.

@huogerac
Copy link
Author

no problem @luanfonceca
It seemed for me the correct order (regarding organization) work on that list first...

In your view, what should be the first task I could work on?
Thanks

@luanfonceca
Copy link
Owner

Hi @huogerac, the person that was assigned has no time to work with this today, so i will reopen you pull request and review it ok?

<div class="panel-body">
<h3 class="panel-title event-title">{{ organization.name }}</h3>
<p class="event-metadata">
Created by <strong><a href="/profile/user/">{{ organization.created_by }}</a></strong>
Copy link
Owner

@luanfonceca luanfonceca Oct 17, 2017

Choose a reason for hiding this comment

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

You cannot put the hardcoded url like this, please use the {% url %}

Created by <strong><a href="/profile/user/">{{ organization.created_by }}</a></strong>
</p>
<div class="event-description">
<p>Link: /{{ organization.slug }}</p>
Copy link
Owner

Choose a reason for hiding this comment

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

This field has no sense and adds no information, can you please remove it?

</div>

<div class="panel-footer event-actions text-center">
<a href="{% url 'update_organization' organization.slug %}" class="btn-flat gray text-upper" data-toggle="tooltip" title="" data-original-title="Edit this organization">
Copy link
Owner

Choose a reason for hiding this comment

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

You need to use the {% trans %} in the tooltip text.

<p class="panel-title ">No organization created.</p>
<p>
<a href="{% url 'create_organization' %}" class="btn-flat success text-upper">
<i class="icon-plus"></i>Create my organization</a>
Copy link
Owner

Choose a reason for hiding this comment

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

You need to translate this text too.


{% empty %}

<p class="panel-title ">No organization created.</p>
Copy link
Owner

Choose a reason for hiding this comment

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

This text needs to be translated.

@@ -10,7 +10,11 @@
url(regex=r'/<slug:slug>/update/',
view=views.UpdateOrganization.as_view(),
name='update_organization'),
url(regex=r'/list/',
view=views.ListAllUserOrganizations.as_view(),
Copy link
Owner

@luanfonceca luanfonceca Oct 17, 2017

Choose a reason for hiding this comment

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

Can you rename this view to ListOrganizations and not use the /list/, leave the url empty as we do in the other urls, like deck/urls.py.

@@ -10,7 +10,11 @@
url(regex=r'/<slug:slug>/update/',
view=views.UpdateOrganization.as_view(),
name='update_organization'),
url(regex=r'/list/',
view=views.ListAllUserOrganizations.as_view(),
name='list_all_user_organizations'),
Copy link
Owner

@luanfonceca luanfonceca Oct 17, 2017

Choose a reason for hiding this comment

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

This should be list_organizations to follow the other urls in speakerfight.

@@ -33,6 +38,13 @@ def form_valid(self, form):
return self.success_redirect(_(u'Organization updated.'))


class ListAllUserOrganizations(BaseOrganizationView, ListView):
Copy link
Owner

Choose a reason for hiding this comment

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

Rename this View.

@luanfonceca
Copy link
Owner

luanfonceca commented Oct 17, 2017

And you need to add tests too.

If you have any questions, please join me on gitter: http://gitter.im/luanfonceca

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants