pinax / pinax-ratings

a ratings app for Django
MIT License
90 stars 28 forks source link

Add one-step user voting widget, other fixes #2

Open mx-moth opened 12 years ago

mx-moth commented 12 years ago

I was using this app for a project I am working on, and I noticed a few odd bits, as described below. Apologies for the one gigantic pull request, but each commit builds upon the previous one, and sending separate pull requests for each fix would not make much sense. I am happy to do more work on this to get the pull request up to scratch, but splitting it up would be more effort than it is worth.

Notable changes include:

One-step rating widget

A one-step user ratings widget that works without JavaScript, can be styled via CSS, and allows for more than one widget per page.

Fixing up of the category system

The old category system had some problems. The way of translating categories into an integer field was unstable, and would produce undefined effects if the order of categories was ever changed in the settings file. The categories.py file was confusing, and not documented. You could vote on an object with no category, even though it had categories defined, and this would still affect the overall rating in a strange manner.

Tempate error reporting threw errors

template.TemplateSyntaxError has a required argument. Not providing an argument throws an error in init. This caused the whole page to die in an unusual manner, bypassing even the Django debug error page.

Anonymous users broke the system.

Anonymous users visiting a page with a ratings object would break the page, instead of failing in a sensible manner.

paltman commented 12 years ago

Oh wow. Not sure how I missed the PR email notification about this. Just seeing it now for the first time and I must say, I am very impressed by all your notes both in your Pull Request as well as your commits. I am looking forward to spending some time with these changes to review and merge them. Thanks!

ossanna16 commented 8 years ago

@paltman Are you still planning on merging this?