compserv / hkn-rails Goto Github PK

The Property.current_semester method returns the current semester, string encoded as YYYYS, with a four-digit year YYYY and one-digit semester S (1 for spring, 2 for summer, 3 for fall). (It's a getter method for the Property.semester attribute.)

It's tied into many major mechanisms:

• Elections: setting the semester attributes of new Committeeships
• Assistant officer / committee member additions: ditto
• Officer view pages: determining which semester to view
• Course reviews: separating them by semester
• Resume books: ditto
  ...

There are, however, a few things that we don't want tied to the current semester:

• Elections: we hold elections before the next semester
• Assistant officer / committee member: we want to be able to add them now or in the next semester

There are also a few things we might want to tie to the current semester:

• Dept tours: #193

There is one major issue with all of this: the current semester is rolled over manually, with the Ruby console, ssh'd into the apphost.

We'd like to:
a) Tie the current semester to the actual current time
b) Decouple certain things from using just the current semester (elected positions)
c) Tie the department tour closed notice to the current semester

There may be more things to do here, but I'll add them later.

Could change to "if !user.approved"

The course aggregate view https://github.com/compserv/hkn-rails/blob/master/config/routes.rb#L246 looks for effectiveness or worthwhileness questions https://github.com/compserv/hkn-rails/blob/master/app/controllers/coursesurveys_controller.rb#L119 and takes only the first one by the corresponding keyword https://github.com/compserv/hkn-rails/blob/master/app/models/survey_question.rb#L34 where it should be filtering instead e.g. https://github.com/compserv/hkn-rails/blob/master/app/controllers/coursesurveys_controller.rb#L139.

When calculating average ratings for a professor over all instances of the same course (or all undergraduate courses), it is not weighted by class size. Thus low ratings for small-enrollment courses (i.e. CS 47A/B/C) disproportionately lower ratings for otherwise effective professors in large classes.

Example: https://hkn.eecs.berkeley.edu/coursesurveys/instructor/Ranade,Gireeja

The offending issue is in this line of code, which naively calculates the unweighted average of all course effectiveness ratings:

As part of data cleanup, I've removed 0 ratings before for 0-response course surveys, but I think it's unwise to remove unusual (but non-zero) low course ratings, instead re-weighting them by their class size (which is usually small).

https://docs.travis-ci.com/user/languages/ruby/

I'm pretty sure we have no actual tests, but we do have a spec that can be run:

bundle exec rake spec

• Responded to actually does something (category sort)
• Deprel's additional comments (notes, etc.) sep. from requester's comments
• Sort requests by requested date
• In show and respond page: (CC's not added, email not sent)

Sean Farhat has requested we close department tours for the summer, with associated blurb.

The wording and layout will be a little tricky; we'd still like to allow people to schedule dept tours for after the summer.

• Currently only allows selection of people who have listed as available for the particular day/time, good in theory but that preference list is often in flux and then is impossible for tutoring officers to manually add someone unless they go and update their preferences

• When selecting a new officer it unselects previously chosen officers, a minor bug to fix
  This is more of a side-effect of how HTML select elements work, but it could be a lot better

• Seems to be non-deterministic for generating a schedule based on the (same, constant) input JSON file

  • It's a non-deterministic algorithm; it's to be expected for a NP-Complete problem.

In F22 the department started using survey questions that have a maximum score of 7, but we are still mapping them to questions that have a maximum of 5 (https://github.com/compserv/hkn-rails/blob/master/app/helpers/admin/csec_admin_helper.rb#L7). Because this is done at survey upload time, the data is already all converted over. It is relatively simple to create a new survey question (with a max of 7) and replace all existing f22 surveyquestions of that type with the new one, but we should also look into if mapping questions is even necessary at this point, with the new code changes to allow multiple types of effectiveness/worthwhile questions.
See https://hkn.eecs.berkeley.edu/coursesurveys/course/CS/61C/2022_Fall for an example of all the questions being out of 7 points instead.

The instructions to click on the bubble for the course guide page is not clear. Many people won't know to click on it.

The link to the new course map is also not clear.

Dates and times should be separate things - sp20 deprel
Time should be a different field?

Some officers have HKN emails that don't match their website usernames. This causes the emails on the /about/officers page to be incorrect.

This will require adjusting users' usernames, through the Person.change_username method in the Ruby shell.

Reported by jaymo.

On saving a shortlink edit, for instance, from a url like this:

https://hkn.eecs.berkeley.edu/shortlinks/243/edit

The page is redirected to the (ostensibly) shortlink detail view page:

https://hkn.eecs.berkeley.edu/shortlinks/243/

However, this page does not exist, so a 404 error is returned.

Instead, this page should redirect to the main shortlinks page (potentially with an announcement banner confirming success):

https://hkn.eecs.berkeley.edu/shortlinks/

Gemfile:

# TODO: Replace this gem with one that is maintained, like                  
# rack-cas or omniauth-cas
# This is used for course surveys so that professors and TAs/GSIs can log in
# and see their survey results using their Berkeley login
gem 'rubycas-client', git: 'https://github.com/rubycas/rubycas-client'

Currently auto-sorted by timestamp, which is in fixed order.

Suggested by Dan Garcia.

Review count matters as much as review rating (since a 4.5 rating by 1000 people is more reliable than a 5 rating by 2 people), so the course professor ratings should reflect this.

Related: #202 #181 #199

• Fix BOM (#202)
• Fix "Response Rate" column error (#202)
• Allow more departments (IEOR)
• (Optional) Remove spurious reviews with non-existant data (0, N/A, etc.) or for non-existant classes (?)

Update photo at https://github.com/jvperrin/hkn-rails/blob/master/app/assets/images/site/2017_hkn_members.jpg and css links at https://github.com/jvperrin/hkn-rails/blob/master/app/assets/stylesheets/home.css.erb#L82.

mowen's been getting two errors while uploading course survey data for fa18.

There are two separate issues here:

"Instructor save failed: #<Instructor id: 5950, last_name: "Lin", picture: "", title: "Teaching Assistant", phone_number: "", email: "[email protected]", home_page: "", interests: "", created_at: "2011-04-15 23:29:06", updated_at: "2011-04-15 23:29:06", private: false, office: "", first_name: "Kevin">"

There are two Kevin Lins in the database. This is somehow causing an error to be thrown at csec_admin_helper.rb#L133, because Instructors are only disambiguated by first,last name, and not id.

"Could not find survey question 'Response Rate' in database"

There is an extra data point in this semester's survey, 'Response Rate'. This extra SurveyQuestion is not an entry in the database, so the lookup fails and an error is thrown at csec_admin_helper.rb#L108

According to the Sass blogpost:

Ruby Sass Has Reached End-Of-Life
One year has passed since we announced the deprecation of Ruby Sass, and it has now officially reached its end-of-life.
If you use Ruby Sass as a command-line tool, the easiest way to migrate is to install Dart Sass as a command-line tool. It supports a similar interface to Ruby Sass, and you can run sass --help for a full explanation of its capabilities.
If you use Ruby Sass as a plugin for a Ruby web app, particularly if you define your own Sass functions in Ruby, the sassc gem provides access to LibSass from Ruby with a very similar API to Ruby Sass. In most cases, you can just replace the Sass module with the SassC module and your code will continue to work.
If you're using Rails, we particularly recommend using the sassc-rails gem, which wraps up the sassc gem and integrates it smoothly into the asset pipeline. Most of the time you won't even need to change any of your code.

And according to an earlier blogpost announcing its deprecation:

With the release of Dart Sass 1.0.0 stable last week, Ruby Sass was officially deprecated.

It appears that the original Ruby implementation of Sass will no longer be maintained; most reference implementation work will be going into the Dart rewrite reasons listed here, with alternative implementations in Ruby leveraged instead (sassc, sassc-rails).

Currently, in the Gemfile, sass-rails is specified as a dependency; version 5.0 depends on sass:

# Sass support for stylesheets
gem 'sass-rails', '~> 5.0'

Replacing this dependency should be as simple as upgrading to version '6.0' (rails/sass-rails#424):

# Sass support for stylesheets
gem 'sass-rails', '~> 6.0'

We should check for breakage, but this looks relatively painless.

Bug reproduction: either navigate to https://hkn.eecs.berkeley.edu/indrel/events?sort=companies.name&sort_direction=up, or navigate to https://hkn.eecs.berkeley.edu/indrel/events, and sort by 'Company'.

Reported on rollbar as error 71; error occurs around indrel_events_controller.rb, line 25.

Looks like the opts.merge! no longer supports joins: as a parameter?

Google Calendar for internal events only shown if given HKN credentials. Google Calendar cannot pass such credentials; current workaround is to download a copy of the calendar when logged in, then import the file.

It appears sunspot:solr:restart doesn't successful start solr again, and the sunspot:solr:start task must be explicitly run to do so:

$ RAILS_ENV=production bundle exec rake sunspot:solr:restart
/home/h/hk/hkn/hkn-rails/migrate/releases/20190107053052/app/controllers/admin/admin_controller.rb:12: warning: key :person is duplicated and overwritten on line 12
/home/h/hk/hkn/hkn-rails/migrate/releases/20190107053052/app/controllers/admin/admin_controller.rb:12: warning: key :person is duplicated and overwritten on line 12
Sending stop command to Solr running on port 8983 ... waiting 5 seconds to allow Jetty process 24944 to stop gracefully.
$ ps aux | grep solr
hkn      14220  0.0  0.0  12784   944 pts/4    S+   16:40   0:00 grep --color=auto solr
$ RAILS_ENV=production bundle exec rake sunspot:solr:start
/home/h/hk/hkn/hkn-rails/migrate/releases/20190107053052/app/controllers/admin/admin_controller.rb:12: warning: key :person is duplicated and overwritten on line 12
/home/h/hk/hkn/hkn-rails/migrate/releases/20190107053052/app/controllers/admin/admin_controller.rb:12: warning: key :person is duplicated and overwritten on line 12
Removing stale PID file at tmp/pids/sunspot-solr-production.pid
Successfully started Solr ...

https://prot-hkn.eecs.berkeley.edu/wiki/Computing_Services#Uploading_Course_Surveys

The recaptcha API keys are still in plaintext in the code:

https://github.com/jvperrin/hkn-rails/blob/master/config/initializers/recaptcha.rb

It appears this was committed in

Fixing this will involve moving this to the rails secrets storage: https://guides.rubyonrails.org/security.html#environmental-security

Github has recommended practices at https://help.github.com/en/articles/removing-sensitive-data-from-a-repository.

Following a rewrite of the git history to remove the commits with the recaptcha api keys, we should also rotate them. @jvperrin is this doable in the Google Admin console?

• Merge #188 to remove api secrets
• Rotate api key

I'm opting not to remove this from the history: the first commit to add these in plaintext was 0115bbe, over 8 years ago.

Specifically, the relevant commits are:

• 0115bbe
• 53ffa80
• 9c11f7d

Removing these commits would require rewriting nearly the entire history of the repo. Rotating the key and loading from secrets.yml should be sufficient.

This issue is blocking making the repo public (#182).

The shortlinks are hard to navigate/find; it'd be much easier if they were separated out by committee.

Current issue is that colors used for marking tutor proficiency with class (yellow, green, blue) are exceptionally similar in hue and brightness. For individuals with red-green colorblindness or other visual deficiencies, these colors are difficult to distinguish.

A better alternative is to additionally use border thickness / pattern (solid / dotted) to represent degree of familiarity with class.

Using bold / italics / underline does not quite establish the same hierarchy so is probably not the best option.

Refer to/review #216.
Point of contacts: @jameslzhu @GeoHutch.

https://medium.com/@derwiki/rails-ip-spoofing-attack-97da8ee1c191

According to this blog post, it appears that we can disable this kind of IP spoofing attack check, since we’re not using IP-based authentication.

For about/officers and for about/cmembers:

1. Make profile pictures larger (maybe custom sizing?)
2. Make profile pictures 3 in a row

Bridge may have additional requests!

Seen with Amanda's computer?

Rationale

There are several benefits to us for publicly releasing our code:

• Transferring ownership to compserv org: the org's education benefits appear to have expired, so the only repos we can host under the compserv umbrella can be public. This is desirable for managing access permissions and avoiding ownership issues due to graduation, lost contact, etc.
• Lower barrier to contribution for non-compserv members / officers: faulty code can be investigated / fixed independently without manual access grants.
• Security incentives: making our code public incentivizes us to make our code secure, especially practices such as removing API keys. This also preemptively secures our code in the case of a Github security breach. (This may require a git filter-branch to remove objects with secrets.)
• Cross-pollination: we frequently receive questions on how portions of our website work, especially the d3.js course map. As a non-profit intended to benefit students and the public, we have at least a de jure motivation to release our code to the benefit of the campus community.
• Relevance to hknweb: hknweb is developed openly, but hkn-rails data must be migrated over. This will require access to hkn-rails code, facilitated by the open-sourcing.

Anti-rationales

• Security-risks: Making our code public, unfortunately, also comes with the risk of making our code easily exploitable, which given the end-of-life quasi-status of hkn-rails amplifies this risk. This risk is nullified when hkn-rails is no longer used in production.
• Effort on maintenance-mode code: this may be better used on hknweb. Nullified if we have an excess of manpower relative to tasks (chances are remote, but possible).

Tasks

• Remove all secrets (#175)
• Select license for open-source release: BSD 2-clause selected in 2011 by richardxia: aed99f9
• Request written permission from major contributors
• Transfer ownership to compserv org

The OCF requires that we have a badge on our website, advertising that we are hosted by the OCF.

Relatively simple, involves either an image loaded from the OCF website (displayed as the code snippet in the link below) or adding the static SVG asset to the website.

The SVG can be found at
https://www.ocf.berkeley.edu/docs/services/vhost/badges/.

https://app.vagrantup.com/debian/boxes/contrib-stretch64

Allegedly, this is the default debian/stretch64 box, but with the vboxsf kernel module, enabling Virtualbox shared folder support in the kernel.

This should bring file autosyncing back without rsync, and generally make people's lives easier.

Bug reproduction: navigate to any url of the form:

• http://hkn.eecs.berkeley.edu/exams/course/
• http://hkn.eecs.berkeley.edu/calendar?month=%22%20&%20sleep%204%20&%20%22&year=2018
• http://hkn.eecs.berkeley.edu/tutor/calendar?month=qwioenurqiopwuneropquwejioruj&year=2017asdfasdfsdfasdf
• http://hkn.eecs.berkeley.edu/events/past?event_filter=mandatory%20for%20candidates&page=file://../../WEB-INF/web.xml&sort=start_time&sort_direction=up
• http://hkn.eecs.berkeley.edu/coursesurveys/rating/424076 (undefined method private for NilClass)
• http://hkn.eecs.berkeley.edu/events/past?event_filter=mandatory%20for%20candidates&page=-1839&sort=start_time&sort_direction=down (invalid page -1839)

These are available on Rollbar as error 68, 93, 95, 99, 110.

These all stem from poor url parsing, especially assumed Integer conversion successes (errors are uncaught).

Ex: error 68.

Traceback: models/exam.rb.

Root cause: department (decoded from url) assumed to be found (no nil checks), causing dept.name to throw 'undefined method 'name''.

Candidate list is currently only sortable by name or by email, which are not very useful.

• EE127 says it’s 0 units when it should be 4
• Add automatic links by parsing the markdown and adding links when it recognizes course codes
  • Pretty regular format: “CS___, EE___, EECS___”

TODO

• Check shared folder will be consistent
• Edit deploy.rb to restart hkn-rails.service, instead of hkn-rails-migrate.service
• Edit logrotate systemd files on apphost to restart hkn-rails.service
• systemctl --user daemon-reload
• Merge into master
• Stop hkn-rails-migrate.service
• Delete old 2.5.0 bundler gems (~/hkn-rails/prod/shared/bundle)
• Deploy prod with Capistrano
• Start hkn-rails.service
• Check if working
• Disable hkn-rails-migrate.service
• Enable hkn-rails.service

Background

When the OCF upgraded its machines from Debian 8 (jessie) to 9 (stretch), it had a transition period for users on the old apphost (werewolves.ocf.berkeley.edu) to migrate their apps to the new apphost (vampires.ocf.berkeley.edu).

The idea was that we would get hkn-rails running simultaneously on both werewolves (jessie) and vampires (stretch), so when the OCF re-routed web traffic from werewolves to vampires, there would be no downtime.

When @jvperrin and I migrated hkn-rails, we created a separate capistrano target migrate, which would target the new apphost vampires in a separate deploy folder ~/hkn-rails/migrate/. (The previous deploy folder was ~/hkn-rails/prod.)

OCF setup

We implemented several workarounds in response to various issues arising from our specific setup on the OCF:

• NFS (Network File System) sharing between werewolves and vampires, causing both to share the same files
• (Not really related, but useful) Unix socket file binding, where traffic to hkn.eecs.berkeley.edu is routed to the program bound to the socket file /srv/apps/hkn/hkn.sock (see the apphosting docs).
• Service starting / restarting management with systemd, which due to NFS also shares network files
• RVM, ruby version manager, which installs and compiles Ruby on the apphost in our user directory (~hkn)
• Our use of Solr, a Java indexing engine which runs as a separate subprocess from hkn-rails. We write its PID number to a file, which hkn-rails uses to know that Solr is running and which PID to connect to.

Past issues / workarounds

NFS, by itself, caused several issues:

• Incompatible Ruby binaries
  • The same Ruby binaries were present on werewolves and vampires. Because Debian stretch upgraded various system libraries, the Ruby compiled on werewolves (2.5.0) linked to shared libraries that were not present on vampires.
  • Solution: we created a Git branch 'migrate', in which we edited the Gemfile ruby version from ruby: '2.5.0' to ruby: '2.5.1'. We installed Ruby 2.5.1 on vampires with rvm, and added rvm version config in the Capfile to denote which version capistrano should use when deploying.
• Systemd unit file changes
  • The systemd unit file, which specifies the hkn-rails script to run at startup, runs only when the host is werewolves: ConditionHost: werewolves
  • Solution: in the migrate branch, the systemd unit file has the host changed to vampires. On the apphost, the service file (~/.config/systemd/user) has been renamed to hkn-rails-migrate.service (to avoid NFS collision with hkn-rails.service). hkn-rails.service was enabled on werewolves, and hkn-rails-migrate.service was enabled on vampires.
• Solr detection failure
  • uh idk @jvperrin do you know how we got around this
• Shared folder inconsistency
  • The deploy uses ~/hkn-rails/prod/shared to share files between releases, i.e. resumes, pid files, configuration. We don't want to lose access to this in the new deploy.
  • Solution: symlink the new shared folder to the old: ~/hkn-rails/migrate/shared -> ~/hkn-rails/prod/shared.

Current tasks

Production deployment today involves checking out the migrate git branch, then deploying to the migrate target with:

bundle exec cap migrate deploy

We would like to return to checking out the master git branch, and deploying to prod; this reduces confusion for new contributors, and reduces redundancy in our config. This will require merging all of the changes on migrate into master, as well as updating the server-side configuration through ssh:

• systemd unit renamings (hkn-rails-migrate -> hkn-rails)
• Double-checking shared/ folder consistency
• Making sure Solr connections still work
• Avoiding downtime (some will be required, to avoid simultaneous bindings to the socket file)