diff --git a/app/controllers/buckets_controller.rb b/app/controllers/buckets_controller.rb index 320882381..dfbf076e0 100644 --- a/app/controllers/buckets_controller.rb +++ b/app/controllers/buckets_controller.rb @@ -22,7 +22,7 @@ class BucketsController < ApplicationController def update @bucket.update! bucket_params - @bucket.accesses.revise granted: grantees, revoked: revokees + @bucket.accesses.revise(granted: grantees, revoked: revokees) unless @bucket.all_access? redirect_to bubbles_path(bucket_ids: [ @bucket ]) end @@ -38,7 +38,7 @@ class BucketsController < ApplicationController end def bucket_params - params.expect(bucket: [ :name ]) + params.expect(bucket: [ :name ]).merge(all_access: params[:bucket].fetch(:all_access, false)) end def grantees diff --git a/app/controllers/concerns/bucket_scoped.rb b/app/controllers/concerns/bucket_scoped.rb index c0dd1176f..be9c8b99b 100644 --- a/app/controllers/concerns/bucket_scoped.rb +++ b/app/controllers/concerns/bucket_scoped.rb @@ -7,6 +7,12 @@ module BucketScoped private def set_bucket - @bucket = Current.user.buckets.find(params[:bucket_id]) + bucket = Current.account.buckets.find params[:bucket_id] + + if bucket.visible_to? Current.user + @bucket = bucket + else + head :forbidden + end end end diff --git a/app/models/bucket/accessible.rb b/app/models/bucket/accessible.rb index a5b49e94f..63002d949 100644 --- a/app/models/bucket/accessible.rb +++ b/app/models/bucket/accessible.rb @@ -21,6 +21,18 @@ module Bucket::Accessible has_many :users, through: :accesses + scope :all_access, -> { where(all_access: true) } + after_create -> { accesses.grant_to creator } + after_save_commit :grant_access_to_everyone end + + def visible_to?(user) + (account == user.account && all_access?) || user.bucket_ids.include?(id) + end + + private + def grant_access_to_everyone + accesses.grant_to(account.users) if all_access_previously_changed?(to: true) + end end diff --git a/app/models/user.rb b/app/models/user.rb index 2baa83876..674df02c6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -21,6 +21,8 @@ class User < ApplicationRecord validates_presence_of :email_address normalizes :email_address, with: ->(value) { value.strip.downcase } + after_create_commit :grant_access_to_buckets + scope :active, -> { where(active: true) } scope :alphabetically, -> { order("LOWER(name)") } @@ -48,4 +50,8 @@ class User < ApplicationRecord def deactived_email_address email_address.sub(/@/, "-deactivated-#{SecureRandom.uuid}@") end + + def grant_access_to_buckets + Access.insert_all(account.buckets.all_access.pluck(:id).collect { |bucket_id| { bucket_id: bucket_id, user_id: id } }) + end end diff --git a/app/views/buckets/accesses/edit.html.erb b/app/views/buckets/accesses/edit.html.erb deleted file mode 100644 index 4a593b72f..000000000 --- a/app/views/buckets/accesses/edit.html.erb +++ /dev/null @@ -1,14 +0,0 @@ -

Users on <%= @bucket.name %>

- -<%= form_with url: bucket_access_path, method: :put do |form| %> - - - <%= form.submit %> -<% end %> diff --git a/app/views/buckets/edit.html.erb b/app/views/buckets/edit.html.erb index 48931ac1c..e03bd6f79 100644 --- a/app/views/buckets/edit.html.erb +++ b/app/views/buckets/edit.html.erb @@ -42,13 +42,11 @@ - <%= link_to nil, class: "btn--faux flex-inline", tabindex: "-1" do %> - - <% end %> + diff --git a/db/migrate/20241209223551_add_all_access_to_bucket.rb b/db/migrate/20241209223551_add_all_access_to_bucket.rb new file mode 100644 index 000000000..d57855ad1 --- /dev/null +++ b/db/migrate/20241209223551_add_all_access_to_bucket.rb @@ -0,0 +1,5 @@ +class AddAllAccessToBucket < ActiveRecord::Migration[8.1] + def change + add_column :buckets, :all_access, :boolean, default: false, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 9895b7729..7f3a3d962 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2024_11_20_222444) do +ActiveRecord::Schema[8.1].define(version: 2024_12_09_223551) do create_table "accesses", force: :cascade do |t| t.integer "bucket_id", null: false t.integer "user_id", null: false @@ -104,6 +104,7 @@ ActiveRecord::Schema[8.0].define(version: 2024_11_20_222444) do t.string "name", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "all_access", default: false, null: false t.index ["account_id"], name: "index_buckets_on_account_id" t.index ["creator_id"], name: "index_buckets_on_creator_id" end diff --git a/test/controllers/bubbles_controller_test.rb b/test/controllers/bubbles_controller_test.rb index d440a1dec..5ea294bfb 100644 --- a/test/controllers/bubbles_controller_test.rb +++ b/test/controllers/bubbles_controller_test.rb @@ -49,4 +49,17 @@ class BubblesControllerTest < ActionDispatch::IntegrationTest assert_equal "moon.jpg", bubble.image.filename.to_s assert_equal [ tags(:mobile) ], bubble.tags end + + test "users can only see bubbles in buckets they have access to" do + get bucket_bubble_url(buckets(:writebook), bubbles(:logo)) + assert_response :success + + buckets(:writebook).accesses.revoke_from(users(:kevin)) # bucket is all-access + get bucket_bubble_url(buckets(:writebook), bubbles(:logo)) + assert_response :success + + buckets(:writebook).update! all_access: false + get bucket_bubble_url(buckets(:writebook), bubbles(:logo)) + assert_response :forbidden + end end diff --git a/test/controllers/buckets_controller_test.rb b/test/controllers/buckets_controller_test.rb index 306d745e3..82af62f56 100644 --- a/test/controllers/buckets_controller_test.rb +++ b/test/controllers/buckets_controller_test.rb @@ -37,6 +37,19 @@ class BucketsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to bubbles_path(bucket_ids: [ buckets(:writebook) ]) assert_equal "Writebook bugs", buckets(:writebook).reload.name assert_equal users(:david, :jz), buckets(:writebook).users + assert_not buckets(:writebook).all_access? + end + + test "update all access" do + bucket = Current.set(session: sessions(:kevin)) do + accounts("37s").buckets.create! name: "New bucket", all_access: false + end + + patch bucket_url(bucket), params: { bucket: { name: "Bugs", all_access: true } } + + assert_redirected_to bubbles_path(bucket_ids: [ bucket ]) + assert bucket.reload.all_access? + assert_equal accounts("37s").users, bucket.users end test "destroy" do diff --git a/test/fixtures/buckets.yml b/test/fixtures/buckets.yml index 6a2bf897e..b9c31d24a 100644 --- a/test/fixtures/buckets.yml +++ b/test/fixtures/buckets.yml @@ -2,3 +2,4 @@ writebook: name: Writebook account: 37s creator: david + all_access: true diff --git a/test/fixtures/sessions.yml b/test/fixtures/sessions.yml index 397f75b57..1d5518dea 100644 --- a/test/fixtures/sessions.yml +++ b/test/fixtures/sessions.yml @@ -1,2 +1,5 @@ david: user: david + +kevin: + user: kevin diff --git a/test/models/bucket_test.rb b/test/models/bucket_test.rb index 64d3546f2..c2b885955 100644 --- a/test/models/bucket_test.rb +++ b/test/models/bucket_test.rb @@ -11,4 +11,30 @@ class BucketTest < ActiveSupport::TestCase buckets(:writebook).accesses.revoke_from users(:kevin) assert_not_includes buckets(:writebook).users.reload, users(:kevin) end + + test "grants access to everyone after creation" do + bucket = Current.set(session: sessions(:david)) do + accounts("37s").buckets.create! name: "New bucket", all_access: true + end + assert_equal accounts("37s").users, bucket.users + end + + test "grants access to everyone after update" do + bucket = Current.set(session: sessions(:david)) do + accounts("37s").buckets.create! name: "New bucket" + end + assert_equal [ users(:david) ], bucket.users + + bucket.update! all_access: true + assert_equal accounts("37s").users, bucket.users.reload + end + + test "visibility" do + assert_not buckets(:writebook).visible_to?(User.new) + assert buckets(:writebook).visible_to?(User.new(account: accounts("37s"))) + + buckets(:writebook).update! all_access: false + assert_not buckets(:writebook).visible_to?(User.new(account: accounts("37s"))) + assert buckets(:writebook).visible_to?(users(:kevin)) + end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 5cb384b75..28159b991 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -10,6 +10,7 @@ class UserTest < ActiveSupport::TestCase assert_equal accounts("37s"), user.account assert_equal user, User.authenticate_by(email_address: "victor@hey.com", password: "secret123456") + assert_equal [ buckets(:writebook) ], user.buckets end test "deactivate" do