Rama mapeveri #68
Rama mapeveri #68
Conversation
| ''' | ||
|
|
||
| id = models.AutoField(primary_key=True) | ||
| descrip = models.CharField(_("Category"), max_length=100, default="") |
There was a problem hiding this comment.
Can we use a full name for this? description
Also, maybe we want a name and description for each category, as well as a slug for linking it?
|
I did some review, comments inline. We use English for this kind of projects since we want to on board people from all around mozilla. Also, I miss the migrations files for the model changes. |
|
Ready the changes. |
|
For apply the migrations, in the terminal:
|
| @@ -219,3 +225,20 @@ def stats(request): | |||
|
|
|||
| return render_to_response( | |||
| 'stats/index.html', data, context_instance=RequestContext(request)) | |||
|
|
|||
| def events_category(request, id): | |||
There was a problem hiding this comment.
I think we should use here id and slug as we use for the detail page, to be more search engine friendly.
|
I Added slug to model category. |
| @@ -45,4 +45,12 @@ <h2 class="event-name">{{ event.name }}</h2> | |||
|
|
|||
| {% include "events/twitter-widget.html" %} | |||
|
|
|||
| <br> | |||
| <h3>{% trans 'Category' %}</h3> | |||
| {% for obj in categories %} | |||
There was a problem hiding this comment.
I would use a more meaningful variable name here, obj is too generic.
{% for cat in categories %}
|
Ready @nukeador |
| try: | ||
| events = CategoryEvent.objects.filter(category_id=id) | ||
| except CategoryEvent.DoesNotExist: | ||
| raise Http404("Selected category nonexistent") |
There was a problem hiding this comment.
Text should be localizable here.
There was a problem hiding this comment.
that you mean exactly with localizable?
There was a problem hiding this comment.
Using the localization string "_".
raise Http404(_("Selected category nonexistent"))
|
No, one categorie, and possible multiple tags. |
|
Well Then? |
|
Then we need just one category per event for now. The tag thing is not currently a priority. |
|
@nukeador ready ;) |
|
Seem OK for me. @mapeveri Have you run the tests to ensure there is no problem? @sebasmagri I don't have a lot of time this week, can you help with deploying this to production? |
|
@nukeador No, I did the tests |
|
@nukeador justo esta semana estoy en medio de unos shipings pero si me das los detalles puedo hacerle deploy. Saludos. |
|
@sebasmagri comprueba que no rompe nada y mergea. Cuando esté avisa y le doy en el server deploy, que mozevent solo tenemos en producción. |
|
|
||
| from django_countries.countries import COUNTRIES | ||
|
|
||
| class Category(models.Model): |
There was a problem hiding this comment.
@mapeveri could you please add a Meta nested class to Category and define its verbose_name and verbose_name_plural to avoid typos in the admin such as Categorys instead of Categories.
|
@mapeveri I've left some comments to the changes. Could you please add a database schema migration for your changes as well? Best Regards, |
|
@sebasmagri Have you attach migration by email ?. The .gitignore ignored the migrations folder. |
|
@sebasmagri Was the schema migration included? I can't find it in the file list :) |
|
@sebasmagri @nukeador I just made a commit with to migrations that I have. |
Este pull request esta relacionado a este issue: #64
Lo que se hizo fue: