Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#5851] Review new framework defaults #8120

Merged
merged 22 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/controllers/comment_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ def new
flash[:notice] = _("Thank you for making an annotation!")

if params[:subscribe_to_request]
@track_thing = TrackThing.create_track_for_request(@info_request)
@existing_track = TrackThing.find_existing(@user, @track_thing)
if @user && @info_request.user == @user
# don't subscribe to own request!
Expand Down
2 changes: 0 additions & 2 deletions app/models/alaveteli_pro/draft_info_request_batch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ class AlaveteliPro::DraftInfoRequestBatch < ApplicationRecord
end
}, inverse_of: :draft_info_request_batches

validates_presence_of :user

after_initialize :set_default_body

strip_attributes only: %i[embargo_duration]
Expand Down
1 change: 0 additions & 1 deletion app/models/alaveteli_pro/embargo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class Embargo < ApplicationRecord
inverse_of: :embargoes,
through: :info_request

validates_presence_of :info_request
validates_presence_of :publish_at
validates_inclusion_of :embargo_duration,
in: ->(e) { e.allowed_durations },
Expand Down
1 change: 0 additions & 1 deletion app/models/alaveteli_pro/embargo_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ module AlaveteliPro
class EmbargoExtension < ApplicationRecord
belongs_to :embargo,
inverse_of: :embargo_extensions
validates_presence_of :embargo
validates_presence_of :extension_duration
validates_inclusion_of :extension_duration,
in: lambda { |_e| AlaveteliPro::Embargo.new.allowed_durations }
Expand Down
6 changes: 3 additions & 3 deletions app/models/alaveteli_pro/request_summary.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@
class AlaveteliPro::RequestSummary < ApplicationRecord
belongs_to :summarisable, polymorphic: true
belongs_to :user,
inverse_of: :request_summaries
inverse_of: :request_summaries,
optional: true
has_and_belongs_to_many :request_summary_categories,
class_name: "AlaveteliPro::RequestSummaryCategory",
inverse_of: :request_summaries

validates_presence_of :summarisable,
:request_created_at,
validates_presence_of :request_created_at,
:request_updated_at
validates_uniqueness_of :summarisable_id, scope: :summarisable_type

Expand Down
2 changes: 1 addition & 1 deletion app/models/announcement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class Announcement < ApplicationRecord
end
}

validates :content, :user,
validates :content,
presence: true
validates :visibility,
presence: true,
Expand Down
2 changes: 0 additions & 2 deletions app/models/announcement_dismissal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,4 @@ class AnnouncementDismissal < ApplicationRecord
inverse_of: :dismissals
belongs_to :user,
inverse_of: :announcement_dismissals

validates :announcement, :user, presence: true
end
6 changes: 4 additions & 2 deletions app/models/category.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@ class Category < ApplicationRecord
has_many :parent_relationships,
class_name: 'CategoryRelationship',
foreign_key: 'child_id',
dependent: :destroy
dependent: :destroy,
validate: false
has_many :parents,
through: :parent_relationships,
source: :parent

has_many :child_relationships,
class_name: 'CategoryRelationship',
foreign_key: 'parent_id',
dependent: :destroy
dependent: :destroy,
validate: false
has_many :children,
through: :child_relationships,
source: :child,
Expand Down
9 changes: 6 additions & 3 deletions app/models/censor_rule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ class CensorRule < ApplicationRecord
].freeze

belongs_to :info_request,
inverse_of: :censor_rules
inverse_of: :censor_rules,
optional: true
belongs_to :user,
inverse_of: :censor_rules
inverse_of: :censor_rules,
optional: true
belongs_to :public_body,
inverse_of: :censor_rules
inverse_of: :censor_rules,
optional: true

validate :require_valid_regexp, if: proc { |rule| rule.regexp? == true }

Expand Down
5 changes: 2 additions & 3 deletions app/models/citation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,9 @@ class Citation < ApplicationRecord
belongs_to :user, inverse_of: :citations
belongs_to :citable, polymorphic: true

belongs_to :info_request, via: :citable
belongs_to :info_request_batch, via: :citable
belongs_to :info_request, via: :citable, optional: true
belongs_to :info_request_batch, via: :citable, optional: true

validates :user, :citable, presence: true
validates :citable_type, inclusion: { in: %w(InfoRequest InfoRequestBatch) }
validates :source_url, length: { maximum: 255,
message: _('Source URL is too long') },
Expand Down
7 changes: 4 additions & 3 deletions app/models/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,17 @@ class Comment < ApplicationRecord

belongs_to :user,
inverse_of: :comments,
counter_cache: true
counter_cache: true,
optional: true # has to be optional for controller action to work

belongs_to :info_request,
inverse_of: :comments
inverse_of: :comments,
optional: true

has_many :info_request_events, # in practice only ever has one
inverse_of: :comment,
dependent: :destroy

# validates_presence_of :user # breaks during construction of new ones :(
validate :check_body_has_content,
:check_body_uses_mixed_capitals

Expand Down
2 changes: 1 addition & 1 deletion app/models/dataset/key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# resource
#
class Dataset::Key < ApplicationRecord
belongs_to :key_set, foreign_key: 'dataset_key_set_id'
belongs_to :key_set, foreign_key: 'dataset_key_set_id', optional: true
has_many :values, foreign_key: 'dataset_key_id', inverse_of: :key

default_scope -> { order(:order) }
Expand Down
1 change: 0 additions & 1 deletion app/models/dataset/key_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,5 @@ class Dataset::KeySet < ApplicationRecord
InfoRequestBatch
].freeze

validates :resource, presence: true
validates :resource_type, inclusion: { in: RESOURCE_TYPES }
end
3 changes: 1 addition & 2 deletions app/models/dataset/value_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# A dataset collection of values
#
class Dataset::ValueSet < ApplicationRecord
belongs_to :resource, polymorphic: true
belongs_to :resource, polymorphic: true, optional: true
belongs_to :key_set, foreign_key: 'dataset_key_set_id'
has_many :values, foreign_key: 'dataset_value_set_id', inverse_of: :value_set

Expand All @@ -27,7 +27,6 @@ class Dataset::ValueSet < ApplicationRecord
FoiAttachment
].freeze

validates :key_set, :values, presence: true
validates :resource_type, inclusion: { in: RESOURCE_TYPES }, if: :resource
validates_associated :values
validate :check_at_least_one_value_is_present
Expand Down
4 changes: 1 addition & 3 deletions app/models/draft_info_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,9 @@ class DraftInfoRequest < ApplicationRecord
include AlaveteliPro::RequestSummaries
include InfoRequest::DraftTitleValidation

validates_presence_of :user

belongs_to :user,
inverse_of: :draft_info_requests
belongs_to :public_body, inverse_of: :draft_info_requests
belongs_to :public_body, inverse_of: :draft_info_requests, optional: true

strip_attributes

Expand Down
2 changes: 1 addition & 1 deletion app/models/foi_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class FoiAttachment < ApplicationRecord

MissingAttachment = Class.new(StandardError)

belongs_to :incoming_message, inverse_of: :foi_attachments
belongs_to :incoming_message, inverse_of: :foi_attachments, optional: true
has_one :info_request, through: :incoming_message, source: :info_request
has_one :raw_email, through: :incoming_message, source: :raw_email

Expand Down
4 changes: 0 additions & 4 deletions app/models/incoming_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,6 @@ class IncomingMessage < ApplicationRecord
inverse_of: :incoming_messages,
counter_cache: true

validates_presence_of :info_request

validates_presence_of :raw_email

has_many :outgoing_message_followups,
inverse_of: :incoming_message_followup,
foreign_key: 'incoming_message_followup_id',
Expand Down
9 changes: 6 additions & 3 deletions app/models/info_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,18 @@ def self.admin_title

belongs_to :user,
inverse_of: :info_requests,
counter_cache: true
counter_cache: true,
optional: true

validate :must_be_internal_or_external

belongs_to :public_body,
inverse_of: :info_requests,
counter_cache: true
counter_cache: true,
validate: false
belongs_to :info_request_batch,
inverse_of: :info_requests
inverse_of: :info_requests,
optional: true

validates_presence_of :public_body, message: N_("Please select an authority")

Expand Down
1 change: 0 additions & 1 deletion app/models/info_request_batch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ class InfoRequestBatch < ApplicationRecord

attr_accessor :ignore_existing_batch

validates_presence_of :user
validates_presence_of :body
validates_absence_of :existing_batch,
unless: -> { ignore_existing_batch },
Expand Down
11 changes: 6 additions & 5 deletions app/models/info_request_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,15 @@ class InfoRequestEvent < ApplicationRecord
belongs_to :info_request,
inverse_of: :info_request_events

validates_presence_of :info_request

belongs_to :outgoing_message,
inverse_of: :info_request_events
inverse_of: :info_request_events,
optional: true
belongs_to :incoming_message,
inverse_of: :info_request_events
inverse_of: :info_request_events,
optional: true
belongs_to :comment,
inverse_of: :info_request_events
inverse_of: :info_request_events,
optional: true

has_one :request_classification,
inverse_of: :info_request_event
Expand Down
6 changes: 4 additions & 2 deletions app/models/mail_server_log.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ class MailServerLog < ApplicationRecord
serialize :delivery_status, DeliveryStatusSerializer

belongs_to :info_request,
inverse_of: :mail_server_logs
inverse_of: :mail_server_logs,
optional: true
belongs_to :mail_server_log_done,
inverse_of: :mail_server_logs
inverse_of: :mail_server_logs,
optional: true

before_create :calculate_delivery_status

Expand Down
2 changes: 1 addition & 1 deletion app/models/note.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class Note < ApplicationRecord
default: Note.default_style,
suffix: true

belongs_to :notable, polymorphic: true
belongs_to :notable, polymorphic: true, optional: true

validates :body, presence: true, if: ->(n) { n.original_style? }
validates :rich_body, presence: true, unless: ->(n) { n.original_style? }
Expand Down
2 changes: 1 addition & 1 deletion app/models/notification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class Notification < ApplicationRecord
DAILY = :daily
enum frequency: [ INSTANTLY, DAILY ]

validates_presence_of :info_request_event, :user, :frequency, :send_after
validates_presence_of :frequency, :send_after

before_validation :calculate_send_after

Expand Down
4 changes: 2 additions & 2 deletions app/models/outgoing_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ class OutgoingMessage < ApplicationRecord
attr_accessor :default_letter

before_validation :cache_from_name
validates_presence_of :info_request
validates_presence_of :from_name, unless: -> (m) { !m.info_request&.user }
validates_inclusion_of :status, in: STATUS_TYPES
validates_inclusion_of :message_type, in: MESSAGE_TYPES
Expand All @@ -56,7 +55,8 @@ class OutgoingMessage < ApplicationRecord
belongs_to :incoming_message_followup,
inverse_of: :outgoing_message_followups,
foreign_key: 'incoming_message_followup_id',
class_name: 'IncomingMessage'
class_name: 'IncomingMessage',
optional: true

has_one :user,
inverse_of: :outgoing_messages,
Expand Down
3 changes: 2 additions & 1 deletion app/models/post_redirect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class PostRedirect < ApplicationRecord

# Optional, does a login confirm before redirect for use in email links.
belongs_to :user,
inverse_of: :post_redirects
inverse_of: :post_redirects,
optional: true

validates :circumstance, inclusion: CIRCUMSTANCES

Expand Down
3 changes: 2 additions & 1 deletion app/models/profile_photo.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ class ProfilePhoto < ApplicationRecord
MAX_DRAFT = 500 # keep even pre-cropped images reasonably small

belongs_to :user,
inverse_of: :profile_photo
inverse_of: :profile_photo,
optional: true

validate :data_and_draft_checks

Expand Down
2 changes: 0 additions & 2 deletions app/models/project/membership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,4 @@ class Project::Membership < ApplicationRecord
belongs_to :project
belongs_to :user
belongs_to :role

validates :project, :user, :role, presence: true
end
2 changes: 0 additions & 2 deletions app/models/project/resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,4 @@
class Project::Resource < ApplicationRecord
belongs_to :project
belongs_to :resource, polymorphic: true

validates :project, :resource, presence: true
end
3 changes: 1 addition & 2 deletions app/models/project/submission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class Project::Submission < ApplicationRecord
belongs_to :project
belongs_to :user
belongs_to :info_request
belongs_to :resource, polymorphic: true
belongs_to :resource, polymorphic: true, optional: true

scope :classification, -> { where(resource_type: 'InfoRequestEvent') }
scope :extraction, -> { where(resource_type: 'Dataset::ValueSet') }
Expand All @@ -31,7 +31,6 @@ class Project::Submission < ApplicationRecord
Dataset::ValueSet
].freeze

validates :project, :user, :info_request, :resource, presence: true
validates :resource_type, inclusion: { in: RESOURCE_TYPES }
validates_associated :resource
end
3 changes: 0 additions & 3 deletions app/models/public_body_category_link.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@ class PublicBodyCategoryLink < ApplicationRecord
belongs_to :public_body_heading,
inverse_of: :public_body_category_links

validates_presence_of :public_body_category
validates_presence_of :public_body_heading

validates :category_display_order, numericality: {
only_integer: true, message: 'Display order must be a number'
}
Expand Down
6 changes: 4 additions & 2 deletions app/models/public_body_change_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
class PublicBodyChangeRequest < ApplicationRecord
belongs_to :user,
inverse_of: :public_body_change_requests,
counter_cache: true
counter_cache: true,
optional: true
belongs_to :public_body,
inverse_of: :public_body_change_requests
inverse_of: :public_body_change_requests,
optional: true

validates_presence_of :public_body_name,
message: N_("Please enter the name of the authority"),
Expand Down
3 changes: 2 additions & 1 deletion app/models/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ class Role < ApplicationRecord
inverse_of: :roles

belongs_to :resource,
polymorphic: true
polymorphic: true,
optional: true

validates :resource_type,
inclusion: { in: Rolify.resource_types },
Expand Down
Loading
Loading