Merge branch 'master' into dependabot/bundler/puma-4.3.3

Author: xsrust

.github/workflows/mysql.yml

@@ -10,7 +10,6 @@ jobs:
       DB_ADAPTER: mysql2
       MYSQL_PWD: root
       RAILS_ENV: test
-      WICKED_PDF_PATH: vendor/bundle/bin/wkhtmltopdf
 
     steps:
     # Checkout the repo
@@ -79,6 +78,10 @@ jobs:
       run: |
         yarn install
 
+    # Figure out where wkhtmltopdf is installed
+    - name: 'Determine wkhtmltopdf location'
+      run: echo ::set-env name=WICKED_PDF_PATH::$(echo `bundle exec which wkhtmltopdf`)
+
     # Setup the database
     - name: 'Setup Test DB'
       run: bundle exec rake db:setup RAILS_ENV=test

.github/workflows/postgres.yml

@@ -9,7 +9,13 @@ jobs:
     services:
       # Postgres installation
       db:
-        image: postgres:11
+        image: postgres
+        env:
+          # Latest version of Postgres has increased security. We can use the default
+          # user/password in this testing scenario though so use the following env
+          # variable to bypass this changes:
+          # https://github.com/docker-library/postgres/issues/681
+          POSTGRES_HOST_AUTH_METHOD: trust
         ports: ['5432:5432']
         options: >-
           --health-cmd pg_isready
@@ -84,6 +90,10 @@ jobs:
           ${{ runner.os }}-yarn-
           ${{ runner.os }}-
 
+    # Figure out where wkhtmltopdf is installed
+    - name: 'Determine wkhtmltopdf location'
+      run: echo ::set-env name=WICKED_PDF_PATH::$(echo `bundle exec which wkhtmltopdf`)
+
     # Install the JS dependencies
     - name: 'Yarn Install'
       run: |

app/models/plan.rb

@@ -344,19 +344,17 @@ def editable_by?(user_id)
   #
   # Returns Boolean
   def readable_by?(user_id)
+    return true if commentable_by?(user_id)
     current_user = User.find(user_id)
-    if current_user.present?
-      # If the user is a super admin and the config allows for supers to view plans
-      if current_user.can_super_admin? &&
-          Branding.fetch(:service_configuration, :plans, :super_admins_read_all)
-        true
-      # If the user is an org admin and the config allows for org admins to view plans
-      elsif current_user.can_org_admin? &&
-          Branding.fetch(:service_configuration, :plans, :org_admins_read_all)
-        owner_and_coowners.map(&:org_id).include?(current_user.org_id)
-      else
-        commentable_by?(user_id)
-      end
+    return false unless current_user.present?
+    # If the user is a super admin and the config allows for supers to view plans
+    if current_user.can_super_admin? &&
+        Branding.fetch(:service_configuration, :plans, :super_admins_read_all)
+      true
+    # If the user is an org admin and the config allows for org admins to view plans
+    elsif current_user.can_org_admin? &&
+        Branding.fetch(:service_configuration, :plans, :org_admins_read_all)
+      owner_and_coowners.map(&:org_id).include?(current_user.org_id)
     else
       false
     end
@@ -388,6 +386,7 @@ def administerable_by?(user_id)
   def reviewable_by?(user_id)
     reviewer = User.find(user_id)
     feedback_requested? &&
+    reviewer.present? &&
     reviewer.org_id == owner.org_id &&
     reviewer.can_review_plans?
   end

spec/models/plan_spec.rb

@@ -856,6 +856,36 @@
       end
     end
 
+    context "explicit sharing does not conflict with admin-viewing" do
+
+      it "super admins" do
+        Branding.expects(:fetch)
+                .with(:service_configuration, :plans, :super_admins_read_all)
+                .at_most_once
+                .returns(false)
+
+        user.perms << create(:perm, name: "add_organisations")
+        role = subject.roles.commenter.first
+        role.user_id = user.id
+        role.save!
+
+        expect(subject.readable_by?(user.id)).to eql(true)
+      end
+
+      it "org admins" do
+        Branding.expects(:fetch)
+                .with(:service_configuration, :plans, :org_admins_read_all)
+                .at_most_once
+                .returns(false)
+
+        user.perms << create(:perm, name: "modify_guidance")
+        role = subject.roles.commenter.first
+        role.user_id = user.id
+        role.save!
+
+        expect(subject.readable_by?(user.id)).to eql(true)
+      end
+    end
   end
 
   describe "#commentable_by?" do