Cleanups in pootle_notifications views #54
Conversation
ghost
assigned 
|
Just note that some functionality of the pootle_notifications app is broken since we removed the 'Quick Links' feature in 77e4a79 as part of bug 2531 — |
| trans_proj = directory.translation_project | ||
| lang = trans_proj.language | ||
| proj = trans_proj.project | ||
| template_vars['language'] = lang |
There was a problem hiding this comment.
Might be easier to read:
template_vars.update({
'language': lang,
'project': proj,
})
If it is not working, because the 'Quick Links' were removed and won't be added again, IMHO then it makes sense to just remove that code and include a commit with those changes in this PR. I don't see the need for filing a bug. |
| # Take into account 'only active users' flag from the form. | ||
| if form.cleaned_data['restrict_to_active_users']: | ||
| to_list = to_list.exclude(submission=None).exclude(suggestion=None) \ | ||
| .exclude(suggester=None) |
There was a problem hiding this comment.
This three exclude calls perhaps should be changed, because I can't found none of this fields in the PootleProfile model
There was a problem hiding this comment.
No, those are foreign key references from the Submission and Suggestion models. If you remove that, you would be emailing all the users in the server with no exception — I doubt you want that.
|
Added some cleanups and changed others. Better review all commits. |
|
Updated whole PR:
Reviews appreciated. I would like to land the commits that are OK, possibly after squashing them. |
| if not check_profile_permission(person, 'view', directory): | ||
| continue | ||
|
|
||
| if person.user.email != '': |
There was a problem hiding this comment.
if person.user.email would suffice.
PEP8 cleanups. Put imports in the right order. Remove unnecessary or unused code. title template var is used on RSS so set it always. Rearrange code to make it more readable and efficient. Remove title in template since users can see its location in breadcrumbs.
Quoted from Julen comment: Just note that some functionality of the pootle_notifications app is broken since we removed the 'Quick Links' feature in 77e4a79 as part of bug 2531 — lang_filter and proj_filter won't have any effects.
No description provided.