1. 23 May, 2019 9 commits
  2. 22 May, 2019 11 commits
    • azul's avatar
      Merge branch '191-reduce-use-of-error-app' into 'staging' · 78b58ef1
      azul authored
      handle 404 ourself rather than with exception app
      
      See merge request !268
      78b58ef1
    • azul's avatar
      test: flatten assertion blocks for error responses · bcef262d
      azul authored
      We used to raise exceptions in the controllers
      and then handle them with exception apps.
      
      This was best tested with blocks of the form
      ```
        assert_not_found do
          get :show, id: :nonexisting
        end
      
      ```
      
      Since we handle the error cases directly now
      we can test for the response code instead.
      
      This allows a simpler style of assertions:
      ```
        get :show, id: :nonexisting
        assert_not_found
      ```
      
      This is more chronological, less lines and easier to understand.
      bcef262d
    • azul's avatar
      fix: render 403 ourselves · 7228758a
      azul authored
      This is mostly to handle all error responses consistently.
      Avoiding the exception app also has the benefit
      that we can search the logs for any exceptions raised
      and figure out why we did not catch them or rescue_from them.
      7228758a
    • azul's avatar
      fix: make all not founds look the same · e986605d
      azul authored
      There should be no difference between
      * a non existing context
      * a hidden group
      * a hidden user
      e986605d
    • azul's avatar
      fix: render 401 ourselves · 5f23d56a
      azul authored
      Just like 404s these were rendered by the exception app before.
      Since the exception app runs as a rake middleware
      after handling cookies and sessions
      cookies and sessions did not work on the error pages.
      
      This was a problem for 401s
      because we render them mostly when people need to login.
      However the lack of cookies and sessions broke the login form.
      5f23d56a
    • azul's avatar
      refactor: use separate page_search_navigation · 29b4ed2b
      azul authored
      Hooking into the global nav system meant it would try to render
      the navigation even for error messages such as 404.
      
      This way the navigation is linked closer to the template
      that is rendered and will not be rendered for other templates.
      29b4ed2b
    • azul's avatar
      cleanup: use raise_not_found consistently · 4fdde4a4
      azul authored
      * do not use render_not_found directly
      
      * do not use `raise ErrorNotFound, :file`
      * do not rescue from ErrorNotFound yourself.
        We do have an application wide handler for that.
      4fdde4a4
    • azul's avatar
      handle 404 ourself rather than with exception app · e55fd3b9
      azul authored
      The exceoption app middleware runs after the cookie handler.
      Therefore sessions are not available to it
      which results in the login form becoming invalid.
      
      In addition exceptions handled by the exception app will still be logged.
      This is cluttering our logs quite a bit.
      We do not need traces for everything that cannot be found or accessed.
      
      This commit brings back the render_not_found method in Controllers.
      
      For the Dispatch controllers it dispatches to the exceptions controller
      directly rather than using it via the exception app.
      
      Keep the assert_not_found helper with a block
      even though it is technically not needed anymore.
      Before this used assert_raises.
      Since we do not raise anymore this is not needed and we now use assert_response.
      Keeping the same style will allow us to roll this back more easliy
      and reduces the number of places we need to change.
      e55fd3b9
    • dgt's avatar
      Merge branch 'test-failing-when-all-run' into 'staging' · 5e5a2487
      dgt authored
      test: rename duplicate Wiki::VersionsControllerTest
      
      See merge request !270
      5e5a2487
    • dgt's avatar
      Merge branch 'update-staging' into 'staging' · 1c106aad
      dgt authored
      Update staging
      
      See merge request !266
      1c106aad
    • azul's avatar
      test: rename duplicate Wiki::VersionsControllerTest · b54f6167
      azul authored
      The name Wiki::VersionsControllerTest was used twice:
       * in the main test dir for testing the generic wiki functions
       * in the wiki page extension for testing wiki page specific functions.
      
      When both tests were run in the same test run
      this resulted in the main app test failing.
      
      The page specific test is now called Wiki::PageVersionsControllerTest.
      b54f6167
  3. 18 May, 2019 7 commits
  4. 15 May, 2019 5 commits
    • dgt's avatar
      Merge branch '282-fix-500-on-double-join' into 'master' · d27fda55
      dgt authored
      Resolve "500 when adding user twice to group"
      
      Closes #282
      
      See merge request !264
      d27fda55
    • azul's avatar
      Merge branch '19-email-notifications-are-broken' into 'master' · 823dd466
      azul authored
      Resolve "email notifications are broken"
      
      See merge request !262
      823dd466
    • azul's avatar
      fix: return 409 when approving redundant requests · 5e50d4f3
      azul authored
      We were responding with 500 and an error popup
      when a request was approved for an action that had already been performed.
      
      For example when approving the removal of a former member
      that had already left the group on their own
      we responded with a 500.
      
      This changes the response to 409 - conflict:
      `    This response is sent when a request conflicts with the current state of the server.`
      
      I was also considering 404 - especially for requests
      to remove a non-member.
      However a 404 for an update on a request
      would seem more like the request itself could not be found.
      
      This commit introduces the Request::PointlessAction exception.
      It will be raised by requests whos action has already been performed.
      
      It allows us to unify error handling on the controller level
      and detect the different errors in each request class
      and reraise them with a common more semantic error class.
      5e50d4f3
    • azul's avatar
      test: failing tests for #282 · 5247682d
      azul authored
      5247682d
    • azul's avatar
      test: actually trigger requests · c29a0c1f
      azul authored
      also turned instance vars into local vars
      as they were not reused anywhere.
      c29a0c1f
  5. 13 May, 2019 1 commit
  6. 10 May, 2019 7 commits