{"action":"created","issue":{"url":"https://api.github.com/repos/alphagov/e-petitions/issues/137","labels_url":"https://api.github.com/repos/alphagov/e-petitions/issues/137/labels{/name}","comments_url":"https://api.github.com/repos/alphagov/e-petitions/issues/137/comments","events_url":"https://api.github.com/repos/alphagov/e-petitions/issues/137/events","html_url":"https://github.com/alphagov/e-petitions/pull/137","id":87736410,"number":137,"title":"Convert SystemSetting model to Site model","user":{"login":"pixeltrix","id":6321,"avatar_url":"https://avatars.githubusercontent.com/u/6321?v=3","gravatar_id":"","url":"https://api.github.com/users/pixeltrix","html_url":"https://github.com/pixeltrix","followers_url":"https://api.github.com/users/pixeltrix/followers","following_url":"https://api.github.com/users/pixeltrix/following{/other_user}","gists_url":"https://api.github.com/users/pixeltrix/gists{/gist_id}","starred_url":"https://api.github.com/users/pixeltrix/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pixeltrix/subscriptions","organizations_url":"https://api.github.com/users/pixeltrix/orgs","repos_url":"https://api.github.com/users/pixeltrix/repos","events_url":"https://api.github.com/users/pixeltrix/events{/privacy}","received_events_url":"https://api.github.com/users/pixeltrix/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-06-12T13:53:22Z","updated_at":"2015-06-12T13:59:23Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/alphagov/e-petitions/pulls/137","html_url":"https://github.com/alphagov/e-petitions/pull/137","diff_url":"https://github.com/alphagov/e-petitions/pull/137.diff","patch_url":"https://github.com/alphagov/e-petitions/pull/137.patch"},"body":"Since we only have at most a handful of settings it makes more sense to store them in explicit columns than cater for an infinite set. This allows us to cache the model in a thread local for performance and then reload in the application controller to allow for admin updates, e.g:\r\n\r\n``` ruby\r\nclass ApplicationController < ActionController::Base\r\n  before_filter :reload_site\r\n\r\n  protected\r\n\r\n  def reload_site\r\n    Site.reload\r\n  end\r\nend\r\n```\r\n\r\nThis will also be useful for when we implement 'Russian Doll' caching - by incorporating the updated_at column in the cache key we can create a global reset that we can both hook up to an admin function or call programmatically in a deploy script if we need to blow away any existing cache:\r\n\r\n``` ruby\r\nclass Admin::SettingsController < AdminController\r\n  def reset_cache\r\n    Site.touch(:updated_at)\r\n  end\r\nend\r\n```\r\n\r\nSome notes on implementation:\r\n\r\n1.  The `before_remove_const` method is a hook that Rails calls when unloading classes before reloading them in development mode. This allows us to clear the cached instance in the thread local, otherwise we would end up with an instance that has a stale class definition.\r\n\r\n2.  By using `first_or_create` to load the instance we can automatically seed the database with the correct values by defining the appropriate defaults on the respective columns. Changing to this exposed some failures in our cucumber test setup - it was defaulting to a threshold level of 0 so I had to correct for that.\r\n\r\n3.  I've added explicit settings for the debate and response threshold to make clear what each one is for."},"comment":{"url":"https://api.github.com/repos/alphagov/e-petitions/issues/comments/111501506","html_url":"https://github.com/alphagov/e-petitions/pull/137#issuecomment-111501506","issue_url":"https://api.github.com/repos/alphagov/e-petitions/issues/137","id":111501506,"user":{"login":"h-lame","id":608,"avatar_url":"https://avatars.githubusercontent.com/u/608?v=3","gravatar_id":"","url":"https://api.github.com/users/h-lame","html_url":"https://github.com/h-lame","followers_url":"https://api.github.com/users/h-lame/followers","following_url":"https://api.github.com/users/h-lame/following{/other_user}","gists_url":"https://api.github.com/users/h-lame/gists{/gist_id}","starred_url":"https://api.github.com/users/h-lame/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/h-lame/subscriptions","organizations_url":"https://api.github.com/users/h-lame/orgs","repos_url":"https://api.github.com/users/h-lame/repos","events_url":"https://api.github.com/users/h-lame/events{/privacy}","received_events_url":"https://api.github.com/users/h-lame/received_events","type":"User","site_admin":false},"created_at":"2015-06-12T13:59:23Z","updated_at":"2015-06-12T13:59:23Z","body":"Reviewing this makes me think, why don't we just put these 2 numbers into application.yml? Or, why don't we take the threshold figures that are in application.yml out and put them in here?"}}