Limit photo album visibility

.rubocop.yml

@@ -33,9 +33,6 @@ Metrics/BlockLength:
     - 'config/routes.rb'
     - 'spec/**/*'
 
-Layout/LineLength:
-  Max: 100
-
 Metrics/MethodLength:
   Exclude:
     - 'db/migrate/*'

PERMISSIONS.md

@@ -20,4 +20,4 @@ When the user has the read permission it is able to get all groups (as always).
 
 ### Activity/Article/Photo
 #### Unauthenticated and without permission
-When not logged in or when without permission it is possible to get activities, articles and photos which have the publicly visible property to true.
+When not logged in or when without permission it is possible to get activities, articles and photos which have the visibility property to everybody.

app/controllers/v1/photos_controller.rb

@@ -1,5 +1,4 @@
 class V1::PhotosController < V1::ApplicationController
-  before_action :doorkeeper_authorize!, except: %i[index show get_related_resources]
   before_action do
     doorkeeper_authorize! unless %w[index show].include?(action_name) ||
                                  (action_name == 'get_related_resources' &&

app/controllers/v1/users_controller.rb

@@ -84,7 +84,7 @@ def nextcloud
                    groups: nextcloud_groups }
   end
 
-  def batch_import # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
+  def batch_import # rubocop:disable Metrics/AbcSize
     authorize model_class
 
     file = decode_upload_file(params['file'])
@@ -93,9 +93,7 @@ def batch_import # rubocop:disable Metrics/MethodLength, Metrics/AbcSize
 
     import = Import::User.new(file, group)
 
-    unless import.valid?
-      return render json: { errors: import.errors }, status: :unprocessable_entity
-    end
+    return render json: { errors: import.errors }, status: :unprocessable_entity unless import.valid?
 
     import.save!(live_run)
     render json: { users: import.imported_users.to_json(except: excluded_display_properties),

app/jobs/mail_moderation_reminder_job.rb

@@ -10,8 +10,6 @@ def perform(stored_mail_id)
       MailModerationMailer.reminder_for_moderation_email(moderator, stored_mail).deliver_later
     end
 
-    unless Rails.env.development?
-      MailModerationReminderJob.set(wait: 24.hours).perform_later(stored_mail_id)
-    end
+    MailModerationReminderJob.set(wait: 24.hours).perform_later(stored_mail_id) unless Rails.env.development?
   end
 end

app/models/import/transaction.rb

@@ -22,9 +22,7 @@ def import!
 
     def valid?(file)
       headers = get_headers(file)
-      unless headers.include?('username')
-        @errors.add(:import_file, 'username field must be present')
-      end
+      @errors.add(:import_file, 'username field must be present') unless headers.include?('username')
       headers.include?('username')
     end
 

app/models/photo.rb

@@ -21,12 +21,15 @@ class Photo < ApplicationRecord
     joins(:comments).distinct
   }
 
-  scope :with_tags, lambda {
-    joins(:tags).distinct
+  scope :alumni_visible, lambda { |start_date, end_date|
+    joins(:photo_album)
+      .where(photo_album: { visibility: %w[alumni public] })
+      .or(where.not(photo_album: { date: nil }).where(photo_album: { date: start_date..end_date }))
+      .or(where(photo_album: { date: nil }).where(photo_album: { created_at: start_date..end_date }))
   }
 
-  scope :publicly_visible, lambda {
-    joins(:photo_album).where(photo_albums: { publicly_visible: true })
+  scope :with_tags, lambda {
+    joins(:tags).distinct
   }
 
   before_save :extract_exif

app/models/photo_album.rb

@@ -7,10 +7,16 @@ class PhotoAlbum < ApplicationRecord
   belongs_to :group, optional: true
 
   validates :title, presence: true
-  validates :publicly_visible, inclusion: [true, false]
-
-  scope :publicly_visible, -> { where(publicly_visible: true) }
+  validates :visibility, inclusion: { in: %w[public alumni members] }
 
+  scope :publicly_visible, lambda {
+    where({ visibility: 'public' })
+  }
+  scope :alumni_visible, lambda { |start_date, end_date|
+    where(visibility: %w[alumni public])
+      .or(where.not(date: nil).where(date: start_date..end_date))
+      .or(where(date: nil).where(created_at: start_date..end_date))
+  }
   scope :without_photo_tags, lambda {
     where.not(id: Photo.joins(:tags).select(:photo_album_id).distinct)
   }

app/models/photo_comment.rb

@@ -6,8 +6,10 @@ class PhotoComment < ApplicationRecord
 
   validates :content, presence: true, length: { minimum: 1, maximum: 500 }
 
-  scope :publicly_visible, lambda {
+  scope :alumni_visible, lambda { |start_date, end_date|
     joins(photo: :photo_album)
-      .where(photo_albums: { publicly_visible: true })
+      .where(photo_album: { visibility: %w[alumni public] })
+      .or(where.not(photo_album: { date: nil }).where(photo_album: { date: start_date..end_date }))
+      .or(where(photo_album: { date: nil }).where(photo_album: { created_at: start_date..end_date }))
   }
 end

app/policies/photo_album_policy.rb

@@ -1,8 +1,14 @@
 class PhotoAlbumPolicy < ApplicationPolicy
   class Scope < ApplicationPolicy::Scope
-    def resolve
+    def resolve # rubocop:disable Metrics/AbcSize
       if user_can_read?
-        scope
+        membership = user.memberships.joins(:group).where(groups: { name: 'Leden' }).first
+        return scope.publicly_visible if membership.nil?
+
+        scope.alumni_visible(
+          membership.start_date&.advance(months: -18),
+          membership.end_date&.advance(months: 6)
+        )
       else
         scope.publicly_visible
       end

app/policies/photo_comment_policy.rb

@@ -1,18 +1,20 @@
 class PhotoCommentPolicy < ApplicationPolicy
   class Scope < ApplicationPolicy::Scope
-    def resolve
+    def resolve # rubocop:disable Metrics/AbcSize
       if user_can_read?
-        scope
+        membership = user.memberships.joins(:group).where(groups: { name: 'Leden' }).first
+        return scope.none if membership.nil?
+
+        scope.alumni_visible(
+          membership.start_date&.advance(months: -18),
+          membership.end_date&.advance(months: 6)
+        )
       else
-        scope.publicly_visible
+        scope.none
       end
     end
   end
 
-  def index?
-    true
-  end
-
   def show?
     scope.exists?(id: record.id)
   end

app/policies/photo_policy.rb

@@ -1,23 +1,25 @@
 class PhotoPolicy < ApplicationPolicy
   class Scope < ApplicationPolicy::Scope
-    def resolve
+    def resolve # rubocop:disable Metrics/AbcSize
       if user_can_read?
-        scope
+        membership = user.memberships.joins(:group).where(groups: { name: 'Leden' }).first
+        return scope.none if membership.nil?
+
+        scope.alumni_visible(
+          membership.start_date&.advance(months: -18),
+          membership.end_date&.advance(months: 6)
+        )
       else
-        scope.publicly_visible
+        scope.none
       end
     end
   end
 
-  def index?
-    true
+  def show?
+    scope.exists?(id: record.id)
   end
 
   def get_related_resources?
-    index?
-  end
-
-  def show?
-    scope.exists?(id: record.id)
+    user&.permission?(:read, record)
   end
 end

app/resources/v1/application_resource.rb

@@ -43,7 +43,7 @@ def self.search(records, value)
     value.each do |val|
       val.split.each do |word|
         records = records.where(
-          searchable_fields.map { |field| arel[field].lower.matches("%#{word.downcase}%") }.inject(:or) # rubocop:disable Layout/LineLength
+          searchable_fields.map { |field| arel[field].lower.matches("%#{word.downcase}%") }.inject(:or)
         )
       end
     end

app/resources/v1/form/form_resource.rb

@@ -20,9 +20,7 @@ def current_user_response_completed
   end
 
   def self.records(options = {})
-    if options[:context][:action] == 'index'
-      options[:includes] = %i[responses open_questions closed_questions]
-    end
+    options[:includes] = %i[responses open_questions closed_questions] if options[:context][:action] == 'index'
     super
   end
 

app/resources/v1/form/response_resource.rb

@@ -8,9 +8,7 @@ class V1::Form::ResponseResource < V1::ApplicationResource
   has_many :closed_question_answers, always_include_linkage_data: true
 
   def self.records(options = {})
-    if options[:context][:action] == 'index'
-      options[:includes] = %i[open_question_answers closed_question_answers]
-    end
+    options[:includes] = %i[open_question_answers closed_question_answers] if options[:context][:action] == 'index'
     super
   end
 

app/resources/v1/photo_album_resource.rb

@@ -1,5 +1,5 @@
 class V1::PhotoAlbumResource < V1::ApplicationResource
-  attributes :title, :date, :publicly_visible
+  attributes :title, :date, :visibility
 
   filter :without_photo_tags, apply: ->(records, _value, _options) { records.without_photo_tags }
 
@@ -8,7 +8,7 @@ class V1::PhotoAlbumResource < V1::ApplicationResource
   has_one :group, always_include_linkage_data: true
 
   def self.creatable_fields(_context)
-    %i[title date publicly_visible group]
+    %i[title date visibility group]
   end
 
   def self.searchable_fields

app/resources/v1/user_resource.rb

@@ -32,9 +32,7 @@ def avatar_thumb_url
     upcoming_birthdays = records.upcoming_birthdays
     records.find_each do |record|
       context[:model] = record
-      unless read_user_details?(context)
-        upcoming_birthdays = upcoming_birthdays.where.not(id: record.id)
-      end
+      upcoming_birthdays = upcoming_birthdays.where.not(id: record.id) unless read_user_details?(context)
     end
     upcoming_birthdays
   }

app/validators/not_renullable_validator.rb

@@ -4,8 +4,6 @@ def validate_each(record, attribute, _value)
 
     return unless changed
 
-    if !changed[0].nil? && changed[1].nil?
-      record.errors.add(attribute, 'changed from not-nil to nil')
-    end
+    record.errors.add(attribute, 'changed from not-nil to nil') if !changed[0].nil? && changed[1].nil?
   end
 end

db/migrate/20250216233318_remove_null_option_boolean.rb

@@ -1,5 +1,5 @@
 class RemoveNullOptionBoolean < ActiveRecord::Migration[7.0]
-  # rubocop:disable Rails/ReversibleMigration, Rails/BulkChangeTable, Metrics/AbcSize, Layout/LineLength
+  # rubocop:disable Rails/ReversibleMigration, Rails/BulkChangeTable, Metrics/AbcSize
   def change
     execute 'UPDATE static_pages SET publicly_visible = false WHERE publicly_visible IS NULL'
     execute 'UPDATE users SET sidekiq_access = false WHERE sidekiq_access IS NULL'
@@ -34,5 +34,5 @@ def change
     change_column_default :room_adverts, :publicly_visible, false
     change_column_null :room_adverts, :publicly_visible, false
   end
-  # rubocop:enable Rails/ReversibleMigration, Rails/BulkChangeTable, Metrics/AbcSize, Layout/LineLength
+  # rubocop:enable Rails/ReversibleMigration, Rails/BulkChangeTable, Metrics/AbcSize
 end

db/migrate/20250314221852_alumni_visibility.rb

@@ -0,0 +1,21 @@
+class AlumniVisibility < ActiveRecord::Migration[7.0]
+  def up
+    add_column :photo_albums, :visibility, :string, default: 'members', null: false
+
+    PhotoAlbum.find_each do |record|
+      record.update!(visibility: record.publicly_visible ? 'public' : 'members')
+    end
+
+    remove_column :photo_albums, :publicly_visible
+  end
+
+  def down
+    add_column :photo_albums, :publicly_visible, :boolean, default: false, null: false
+
+    PhotoAlbum.find_each do |record|
+      record.update!(publicly_visible: record.visibility == 'public')
+    end
+
+    remove_column :photo_albums, :visibility
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2025_02_19_195453) do
+ActiveRecord::Schema[7.0].define(version: 2025_03_14_221852) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -39,7 +39,7 @@
     t.string "content_type"
     t.text "metadata"
     t.bigint "byte_size", null: false
-    t.string "checksum", null: false
+    t.string "checksum"
     t.datetime "created_at", precision: nil, null: false
     t.string "service_name", null: false
     t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
@@ -411,9 +411,9 @@
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false
     t.datetime "deleted_at", precision: nil
-    t.boolean "publicly_visible", default: false, null: false
     t.bigint "author_id"
     t.bigint "group_id"
+    t.string "visibility", default: "members", null: false
     t.index ["author_id"], name: "index_photo_albums_on_author_id"
     t.index ["deleted_at"], name: "index_photo_albums_on_deleted_at"
     t.index ["group_id"], name: "index_photo_albums_on_group_id"

spec/factories/photo_albums.rb

@@ -1,10 +1,12 @@
 FactoryBot.define do
   factory :photo_album do
     title { Faker::Book.title }
-    publicly_visible { false }
+    visibility do
+      %w[public alumni members].sample
+    end
     author factory: %i[user]
     group
 
-    trait(:public) { publicly_visible { true } }
+    trait(:public) { visibility { 'public' } }
   end
 end

spec/factories/photo_comments.rb

@@ -6,7 +6,5 @@
     end
 
     photo
-
-    trait(:public) { photo factory: %i[photo public] }
   end
 end

spec/factories/photos.rb

@@ -9,7 +9,6 @@
     photo_album
     uploader factory: %i[user]
 
-    trait(:public) { photo_album factory: %i[photo_album public] }
     trait(:invalid) do
       image do
         Rack::Test::UploadedFile.new(

spec/models/photo_album_spec.rb

@@ -19,7 +19,7 @@
     end
 
     context 'when without public visibility' do
-      subject(:photo_album) { build_stubbed(:photo_album, publicly_visible: nil) }
+      subject(:photo_album) { build_stubbed(:photo_album, visibility: nil) }
 
       it { expect(photo_album).not_to be_valid }
     end
@@ -29,15 +29,16 @@
     it_behaves_like 'a model with group owners'
   end
 
-  describe '#publicly_visible' do
+  describe '#visibility' do
     before do
-      create(:photo_album, publicly_visible: true)
-      create(:photo_album, publicly_visible: true)
-      create(:photo_album, publicly_visible: false)
+      create(:photo_album, visibility: 'public')
+      create(:photo_album, visibility: 'alumni')
+      create(:photo_album, visibility: 'members')
     end
 
-    it { expect(described_class.publicly_visible.count).to be 2 }
-    it { expect(described_class.count - described_class.publicly_visible.count).to be 1 }
+    it { expect(described_class.publicly_visible.count).to be 1 }
+    it { expect(described_class.where(visibility: %w[alumni public]).count).to be 2 }
+    it { expect(described_class.where.not(visibility: %w[alumni public]).count).to be 1 }
   end
 
   describe '#to_zip' do