Add one-step user voting widget, other fixes #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This widget is an improvement on the old widget for a number of reasons: 1. The widget is straight HTML, so JavaScript is not required for the plugin to function 2. This widget is stylable via CSS, instead of having hard coded images 3. Multiple widgets can appear on the same page without having ID conflicts 4. The widget HTML is overridable in the normal Django template way
Previously it was possible to rate ANY object. This is not a good plan, as malicious users could rate objects that should not be rated, such as adminstrative users. This could potentially allow users to see private data. Now, models that are to be rated need to be explicitly mentioned as such in the `settings.py`. This is explained in the updated documentation. Additionally, category choices were stored as integer choice fields. The integers for each choice were assigned sequentially as choices were processed. This would have led to unstable behaviour regarding choices, as choices could be added and removed in the future, disrupting the sequence of choices. This has been fixed by changing the `category` field to a CharField, instead of the previous integer field. Only predefined values from `settings.py` are ever inserted into this field, so injection attacks are not an issue here.
People submitting the form with no JavaScript will have the same experience as other users.
All JavaScript properties, methods and libraries work in camelCase, so we should respond in kind with camelCased JSON.
Guard against invalid parameter counts in a consistent manner. Throwing TemplateSyntaxErrors with no arguments causes the TemplateSyntaxError to throw an error.
You can output the overall rating in either absolute terms (e.g. 4.2), as a percentage (e.g. 84%), or as a decimal in the range [0, 1] (e.g. 0.84). This is useful when doing computations for display (e.g. the width of an element to display the stars in).
Anonymous users do not get the JavaScript widget loaded, so they use normal HTTP requests to vote on things. As the `rate` view requires user logins, this results in them being redirected to the log in page
If a user is redirected to a ratings page after logging in, they would be using GET requests. Instead of failing and showing an error page, the user is now shown a simple 'rate object' page. Upon submission, the user should be redirected back to the objects absolute url
|
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 Are you still planning on merging this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.pyfile 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.