pinax-ratings icon indicating copy to clipboard operation
pinax-ratings copied to clipboard

Add one-step user voting widget, other fixes

Open mx-moth opened this issue 13 years ago • 2 comments

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.

mx-moth avatar Jan 23 '12 23:01 mx-moth

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!

paltman avatar Jul 01 '12 04:07 paltman

@paltman Are you still planning on merging this?

ossanna16 avatar Jan 06 '16 18:01 ossanna16