-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add new My Suggestions page #996
Conversation
A new page for users to find new projects to work on. Includes a very (very) rudimentary recommendation algorithm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks ready for general release to me. Minor weight/criteria tweaking can always be done later of course if needed.
Great new feature!
tools/proofers/my_suggestions.php
Outdated
FROM user_project_info | ||
WHERE | ||
t_latest_page_event > 0 | ||
AND username='$username' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we encode the username for the sql context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We 100% should. I've added a second commit which does this here and in My Projects (which was where I sourced the initial version of this at the very beginning).
ORDER BY count DESC | ||
LIMIT %d | ||
", | ||
$username, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here, though I suspect our username limitations are safe enough.
$round = $Round_for_project_state_[$project->state]; | ||
validate_user_against_project_reserve($user, $project, $round); | ||
$return_projects[] = $project_row; | ||
} catch(Exception $exception) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was originally wondering if any other exceptions can occur that we should not ignore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now this can only return ReservedForNewProofreadersException
but I thought catching any exception was a safer approach here as any exception very likely indicates that validation failed for some reason or another.
Usernames can't contain special SQL characters, and the usernames are validated by the pages at the very top, but we should always escape inputs into SQL queries (belt & suspenders).
A new page for users to find new projects to work on. Includes a very (very) rudimentary recommendation algorithm. The page was originally titled "My Recommendations" but "Suggestions" is both shorter and sounds a little less formal.
The algorithm -- and the bulk of the code -- is the same one currently in noncvs on PROD, however it is not the exact same as this one relies on a few recent PRs.
Code in the PR is testable in https://www.pgdp.org/~cpeel/c.branch/my-suggestions/