-
Notifications
You must be signed in to change notification settings - Fork 3
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
instructor links to ratemyprofessor.com #77
Comments
I was going to implement this feature, but I don't know the best way to do it since there there might be multiple professors with the same last name. That means you can't simply link to a URL with the professor's review, so the link would have to bring up a search query with possible choices. Maybe clicking on the name will bring up a dialog box with the results of said query, and the user can select which professor seems correct. I wrote some javascript that will only show UC, Irvine professors when you do a query. var table = document.getElementById('rmp_table');
var length = table.rows.length;
for (var i=0; i<length; i++) {
if (table.rows[i].innerHTML.indexOf('sid=1074') === -1) {
table.deleteRow(i);
length--; // one less row to check
i--; // check same row next iteration
}
} That can be injected at the end of a request to |
I went ahead and implemented this feature and sent a pull request. |
Checked out the pull request really briefly since I'm knee deep in work lately and looks awesome! I know early on I toyed around with the RMP site but like you said, it has a pretty questionable architecture that makes it pretty hard to get results easily. I'll take a look this weekend and see if I can work with the click problem then get everything merged into the main branch. |
Cool! Also, maybe you can checkout the memcache part. I noticed you used memcache for the other requests but when I tried imitating how you wrote your code in the other functions, the request stopped working properly. Maybe there is something I'm missing. Let me know if there is another issue you want me to work on. I was thinking about cleaning up instructions/documentation and error messages that were never implemented. |
Got the click handling fixed up. Ran into some minor issues with some hover events on the school side so fixing that now. Then once I get the memcached working and tweak some interface elements, a push is in store! |
Sounds good. I'll be interested to see what you did to get the click handling fixed since I don't have a lot of experience with JQuery. |
It actually didn't require too much. I put in a "return false" at the end of the professor click handler which prevents propogation of all parent elements' click handlers (so the course row's click handler gets canceled out). I think event.stopPropogation() works as well but I've never used it so not sure if there's any side effects. Another thing, I modified main.js courses that have 2 professors listed would only send one AJAX call. One thing I noticed though is that on IE 7, courses with 2 professors don't show any results. I'm going to investigate this regression later this week before I push live. I have the latest code on staging here: http://release-0-9-8-3.antplanner.appspot.com/ |
So that's the purpose of return false at the end of those click handlers. I was wondering about that. One thing I noticed is that my AJAX error function was never formatted properly. It should probably look something like: profTable.html('<tr><th colspan="6">An error occured!</th></tr><tr><td colspan="6">'+textStatus+' - '+errorThrown+'</td></tr>'); |
So I fixed the IE bug, the problem was that somehow IE was converting an element into uppercase when code was expecting lowercase. But...there's another problem, the professor links are hammering the servers hard!! I've been getting some pretty crazy numbers for amount of CPU used - around 23800cpu ms average, which may not sit well with quota reductions from the new AppEngine pricing scheme Google announced yesterday. Somehow we'll need to find another way around fetching pages from RMP on every request. I'm thinking it might be more practical to periodically start a workorder to index RMP professor ids and save them to the datastore. Then the professors could just link to the pages. Either that or more optimizations will be needed. :( |
I noticed the cpu usage as well. I think setting up a cron job that runs once a month could collect all the RMP ratings and then store them in a database. That way, user queries are against the database and not scraping the RMP website every time. It would be a little outdated but I think memcache could make results outdated as well so thats not a huge difference. Another option is to run the cron job each day but every day grabs a different letter. For example the first day you scrape Also, from what I could tell the problem was not so much cpu usage as it was cpu time. Meaning RMP is just slow so it has to wait for the response before doing any processing and Google doesn't want you hanging up their servers. I don't know if there is any optimizations that can be done. I hope that helps. |
No description provided.
The text was updated successfully, but these errors were encountered: