diff --git a/.gitignore b/.gitignore index d0efb14..51cbf82 100644 --- a/.gitignore +++ b/.gitignore @@ -85,3 +85,4 @@ lib/assets/.eslintcache !.keep .byebug_history +lib/data_cleanup/rules/org/fix_blank_abbreviation.yml diff --git a/.travis.yml b/.travis.yml index 94cf431..fdba97a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: ruby rvm: - 2.4.4 -before_install: +before_install: - curl -sL https://deb.nodesource.com/setup_8.x | sudo -E bash - && sudo apt-get install -y nodejs before_script: - cd lib/assets && npm install && npm run bundle -- -p && cd - @@ -14,7 +14,8 @@ - bundle exec rake db:drop RAILS_ENV=test - bundle exec rake db:create RAILS_ENV=test - bundle exec rake db:schema:load RAILS_ENV=test - - bundle exec rake db:migrate RAILS_ENV=test + - bundle exec rake db:migrate RAILS_ENV=test script: - bundle exec rake test + - bundle exec rspec spec diff --git a/Gemfile b/Gemfile index ac94878..a4d7bf3 100644 --- a/Gemfile +++ b/Gemfile @@ -117,6 +117,8 @@ gem "rspec-rails" + gem "rspec-collection_matchers" + gem "factory_bot_rails" gem "faker" @@ -138,7 +140,9 @@ # Code coverage for Ruby 1.9+ with a powerful configuration library and automatic merging of coverage across test suites (http://github.com/colszowka/simplecov) gem 'simplecov', require: false - gem 'database_cleaner' + gem 'database_cleaner', require: false + + gem "shoulda", require: false end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index dd2b87e..0feaf02 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -214,6 +214,8 @@ json responders (2.3.0) railties (>= 4.2.0, < 5.1) + rspec-collection_matchers (1.1.3) + rspec-expectations (>= 2.99.0.beta1) rspec-core (3.7.1) rspec-support (~> 3.7.0) rspec-expectations (3.7.0) @@ -236,6 +238,12 @@ rubyzip (1.2.1) safe_yaml (1.0.4) sax-machine (1.3.2) + shoulda (3.6.0) + shoulda-context (~> 1.0, >= 1.0.1) + shoulda-matchers (~> 3.0) + shoulda-context (1.2.2) + shoulda-matchers (3.1.2) + activesupport (>= 4.0.0) simplecov (0.16.1) docile (~> 1.1) json (>= 1.8, < 3) @@ -309,8 +317,10 @@ railties recaptcha responders (~> 2.0) + rspec-collection_matchers rspec-rails ruby_dig + shoulda simplecov sqlite3 thin diff --git a/app/controllers/org_admin/phases_controller.rb b/app/controllers/org_admin/phases_controller.rb index aeadb9d..1b39b6e 100644 --- a/app/controllers/org_admin/phases_controller.rb +++ b/app/controllers/org_admin/phases_controller.rb @@ -127,7 +127,7 @@ def sort @phase = Phase.find(params[:id]) authorize @phase - Section.update_numbers!(*params.fetch(:sort_order, []), phase: @phase) + Section.update_numbers!(*params.fetch(:sort_order, []), parent: @phase) head :ok end diff --git a/app/models/annotation.rb b/app/models/annotation.rb index 98fad83..b8a6968 100644 --- a/app/models/annotation.rb +++ b/app/models/annotation.rb @@ -21,9 +21,14 @@ # class Annotation < ActiveRecord::Base + include ValidationMessages + enum type: [ :example_answer, :guidance] - ## - # Associations + + # ================ + # = Associations = + # ================ + belongs_to :org belongs_to :question has_one :section, through: :question @@ -34,16 +39,23 @@ # I liked type as the name for the enum so overriding inheritance column self.inheritance_column = nil - validates :question, :org, presence: {message: _("can't be blank")} + # =============== + # = Validations = + # =============== - ## - # returns the text from the annotation - # - # @return [String] the text from the annotation - def to_s - "#{text}" - end + validates :text, presence: { message: PRESENCE_MESSAGE } + validates :org, presence: { message: PRESENCE_MESSAGE } + + validates :question, presence: { message: PRESENCE_MESSAGE } + + validates :type, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE, + scope: [:question_id, :org_id] } + + # ================= + # = Class Methods = + # ================= ## # deep copy the given annotation and all it's associations @@ -56,6 +68,18 @@ return annotation_copy end + # =========================== + # = Public instance methods = + # =========================== + + ## + # returns the text from the annotation + # + # @return [String] the text from the annotation + def to_s + "#{text}" + end + def deep_copy(**options) copy = self.dup copy.question_id = options.fetch(:question_id, nil) diff --git a/app/models/answer.rb b/app/models/answer.rb index d39d68a..46d9aa8 100644 --- a/app/models/answer.rb +++ b/app/models/answer.rb @@ -23,131 +23,125 @@ # fk_rails_... (user_id => users.id) # -class Answer < ActiveRecord::Base - - after_save do |answer| - if answer.plan_id.present? - plan = answer.plan - complete = plan.no_questions_matches_no_answers? - if plan.complete != complete - plan.complete = complete - plan.save! - else - plan.touch # Force updated_at changes if nothing changed since save only saves if changes were made to the record - end - end - end - - ## - # Associations - belongs_to :question - belongs_to :user - belongs_to :plan - has_many :notes, dependent: :destroy - has_and_belongs_to_many :question_options, join_table: "answers_question_options" - - has_many :notes - - - ## - # Validations -# validates :user, :plan, :question, presence: true -# -# # Make sure there is only one answer per question! -# validates :question, uniqueness: {scope: [:plan], -# message: I18n.t('helpers.answer.only_one_per_question')} -# -# # The answer MUST have a text value if the question is NOT option based or a question_option if -# # it is option based. -# validates :text, presence: true, if: Proc.new{|a| -# (a.question.nil? ? false : !a.question.question_format.option_based?) -# } -# validates :question_options, presence: true, if: Proc.new{|a| -# (a.question.nil? ? false : a.question.question_format.option_based?) -# } -# -# # Make sure the plan and question are associated with the same template! -# validates :plan, :question, answer_for_correct_template: true - - ## - # deep copy the given answer - # - # @params [Answer] question_option to be deep copied - # @return [Answer] the saved, copied answer - def self.deep_copy(answer) - answer_copy = answer.dup - answer.question_options.each do |opt| - answer_copy.question_options << opt - end - answer_copy.save! - return answer_copy - end - - # This method helps to decide if an answer option (:radiobuttons, :checkbox, etc ) in form views should be checked or not - # Returns true if the given option_id is present in question_options, otherwise returns false - def has_question_option(option_id) - self.question_option_ids.include?(option_id) - end - - # Returns true if the answer is valid and false otherwise. If the answer's question is option_based, it is checked if exist - # any question_option selected. For non option_based (e.g. textarea or textfield), it is checked the presence of text - def is_valid? - if self.question.present? - if self.question.question_format.option_based? - return !self.question_options.empty? - else # (e.g. textarea or textfield question formats) - return self.text.present? - end - end - return false - end - # Returns answer notes whose archived is blank sorted by updated_at in descending order - def non_archived_notes - return notes.select{ |n| n.archived.blank? }.sort!{ |x,y| y.updated_at <=> x.updated_at } - end - - ## - # Returns True if answer text is blank, false otherwise - # specificly we want to remove empty hml tags and check - # - # @return [Boolean] is the answer's text blank - def is_blank? - if self.text.present? - return self.text.gsub(/<\/?p>/, '').gsub(//, '').chomp.blank? - end - # no text so blank - return true - end - - ## - # Returns the parsed JSON hash for the current answer object - # Generates a new hash if none exists for rda_questions - # - # @return [Hash] the parsed hash of the answer. - # Should have keys 'standards', 'text' - # 'standards' is a list of : pairs - # 'text' is the text from the comments box - def answer_hash - default = {'standards' => {}, 'text' => ''} - begin - h = self.text.nil? ? default : JSON.parse(self.text) - rescue JSON::ParserError => e - h = default - end - return h - end - - ## - # Given a hash of standards and a comment value, this updates answer - # text for rda_questions - # - # @param [standards] a hash of standards - # @param [text] option comment text - # nothing returned, but the status of the text field of the answer is changed - def update_answer_hash(standards={},text="") - h = {} - h['standards'] = standards - h['text'] = text - self.text = h.to_json - end -end +class Answer < ActiveRecord::Base + include ValidationMessages + + after_save do |answer| + if answer.plan_id.present? + plan = answer.plan + complete = plan.no_questions_matches_no_answers? + if plan.complete != complete + plan.complete = complete + plan.save! + else + plan.touch # Force updated_at changes if nothing changed since save only saves if changes were made to the record + end + end + end + + ## + # Associations + belongs_to :question + belongs_to :user + belongs_to :plan + has_many :notes, dependent: :destroy + has_and_belongs_to_many :question_options, join_table: "answers_question_options" + + has_many :notes + + + # =============== + # = Validations = + # =============== + + validates :plan, presence: { message: PRESENCE_MESSAGE } + + validates :user, presence: { message: PRESENCE_MESSAGE } + + validates :question, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE, + scope: :plan_id } + + ## + # deep copy the given answer + # + # @params [Answer] question_option to be deep copied + # @return [Answer] the saved, copied answer + def self.deep_copy(answer) + answer_copy = answer.dup + answer.question_options.each do |opt| + answer_copy.question_options << opt + end + answer_copy.save! + return answer_copy + end + + # This method helps to decide if an answer option (:radiobuttons, :checkbox, etc ) in form views should be checked or not + # Returns true if the given option_id is present in question_options, otherwise returns false + def has_question_option(option_id) + self.question_option_ids.include?(option_id) + end + + # Returns true if the answer is valid and false otherwise. If the answer's question is option_based, it is checked if exist + # any question_option selected. For non option_based (e.g. textarea or textfield), it is checked the presence of text + def is_valid? + if self.question.present? + if self.question.question_format.option_based? + return !self.question_options.empty? + else # (e.g. textarea or textfield question formats) + return self.text.present? + end + end + return false + end + + # Returns answer notes whose archived is blank sorted by updated_at in descending order + def non_archived_notes + return notes.select{ |n| n.archived.blank? }.sort!{ |x,y| y.updated_at <=> x.updated_at } + end + + ## + # Returns True if answer text is blank, false otherwise + # specificly we want to remove empty hml tags and check + # + # @return [Boolean] is the answer's text blank + def is_blank? + if self.text.present? + return self.text.gsub(/<\/?p>/, '').gsub(/<br\s?\/?>/, '').chomp.blank? + end + # no text so blank + return true + end + + ## + # Returns the parsed JSON hash for the current answer object + # Generates a new hash if none exists for rda_questions + # + # @return [Hash] the parsed hash of the answer. + # Should have keys 'standards', 'text' + # 'standards' is a list of <std_id>: <title> pairs + # 'text' is the text from the comments box + def answer_hash + default = {'standards' => {}, 'text' => ''} + begin + h = self.text.nil? ? default : JSON.parse(self.text) + rescue JSON::ParserError => e + h = default + end + return h + end + + ## + # Given a hash of standards and a comment value, this updates answer + # text for rda_questions + # + # @param [standards] a hash of standards + # @param [text] option comment text + # nothing returned, but the status of the text field of the answer is changed + def update_answer_hash(standards={},text="") + h = {} + h['standards'] = standards + h['text'] = text + self.text = h.to_json + end +end diff --git a/app/models/concerns/acts_as_sortable.rb b/app/models/concerns/acts_as_sortable.rb new file mode 100644 index 0000000..f5ebc6e --- /dev/null +++ b/app/models/concerns/acts_as_sortable.rb @@ -0,0 +1,44 @@ +module ActsAsSortable + extend ActiveSupport::Concern + + module ClassMethods + + def update_numbers!(*ids, parent:) + # Ensure only records belonging to this parent are included. + ids = ids.map(&:to_i) & parent.public_send("#{model_name.singular}_ids") + return if ids.empty? + case connection.adapter_name + when "PostgreSQL" then update_numbers_postgresql!(ids) + when "Mysql2" then update_numbers_mysql2!(ids) + else + update_numbers_sequentially!(ids) + end + end + + private + + def update_numbers_postgresql!(ids) + # Build an Array with each ID and its relative position in the Array + values = ids.each_with_index.map { |id, i| "(#{id}, #{i + 1})" }.join(",") + # Run a single UPDATE query for all records. + query = <<~SQL + UPDATE #{table_name} \ + SET number = svals.number \ + FROM (VALUES #{sanitize_sql(values)}) AS svals(id, number) \ + WHERE svals.id = #{table_name}.id; + SQL + connection.execute(query) + end + + def self.update_numbers_mysql2!(ids) + ids_string = ids.map { |id| "'#{id}'" }.join(",") + update_all(%Q{ number = FIELD(id, #{sanitize_sql(ids_string)}) }) + end + + def self.update_numbers_sequentially!(ids) + ids.each_with_index.map do |id, number| + find(id).update_attribute(:number, number + 1) + end + end + end +end diff --git a/app/models/concerns/validation_messages.rb b/app/models/concerns/validation_messages.rb new file mode 100644 index 0000000..f0f711f --- /dev/null +++ b/app/models/concerns/validation_messages.rb @@ -0,0 +1,10 @@ +module ValidationMessages + # frozen_string_literal: true + + PRESENCE_MESSAGE = _("can't be blank") + + UNIQUENESS_MESSAGE = _("must be unique") + + INCLUSION_MESSAGE = _("isn't a valid value") + +end diff --git a/app/models/concerns/validation_values.rb b/app/models/concerns/validation_values.rb new file mode 100644 index 0000000..dff74c3 --- /dev/null +++ b/app/models/concerns/validation_values.rb @@ -0,0 +1,3 @@ +module ValidationValues + BOOLEAN_VALUES = [true, false].freeze +end \ No newline at end of file diff --git a/app/models/exported_plan.rb b/app/models/exported_plan.rb index 24bfaac..24a854a 100644 --- a/app/models/exported_plan.rb +++ b/app/models/exported_plan.rb @@ -12,6 +12,7 @@ # class ExportedPlan < ActiveRecord::Base + include ValidationMessages include GlobalHelpers include SettingsTemplateHelper @@ -20,15 +21,9 @@ belongs_to :plan belongs_to :user - VALID_FORMATS = ['csv', 'html', 'pdf', 'text', 'docx'] + validates :plan, presence: { message: PRESENCE_MESSAGE } - validates :format, inclusion: { - in: VALID_FORMATS, - message: -> (object, data) do - _('%{value} is not a valid format') % { :value => data[:value] } - end - } - validates :plan, :format, presence: {message: _("can't be blank")} + validates :format, presence: { message: PRESENCE_MESSAGE } # Store settings with the exported plan so it can be recreated later # if necessary (otherwise the settings associated with the plan at a diff --git a/app/models/guidance.rb b/app/models/guidance.rb index c98c5cc..5ad8950 100644 --- a/app/models/guidance.rb +++ b/app/models/guidance.rb @@ -30,23 +30,30 @@ class Guidance < ActiveRecord::Base include GlobalHelpers + include ValidationMessages + include ValidationValues - ## - # Associations + # ================ + # = Associations = + # ================ belongs_to :guidance_group has_and_belongs_to_many :themes, join_table: "themes_in_guidance" -# depricated, but required for migration "single_group_for_guidance" - #has_and_belongs_to_many :guidance_groups, join_table: "guidance_in_group" - # EVALUATE CLASS AND INSTANCE METHODS BELOW # # What do they do? do they do it efficiently, and do we need them? + # =============== + # = Validations = + # =============== + validates :text, presence: { message: PRESENCE_MESSAGE } - validates :text, presence: {message: _("can't be blank")} + validates :guidance_group, presence: { message: PRESENCE_MESSAGE } + + validates :published, inclusion: { message: INCLUSION_MESSAGE, + in: BOOLEAN_VALUES} # Retrieves every guidance associated to an org scope :by_org, -> (org) { diff --git a/app/models/guidance_group.rb b/app/models/guidance_group.rb index 490c48f..f9520a2 100644 --- a/app/models/guidance_group.rb +++ b/app/models/guidance_group.rb @@ -21,6 +21,9 @@ class GuidanceGroup < ActiveRecord::Base include GlobalHelpers + include ValidationValues + include ValidationMessages + ## # Associations belongs_to :org @@ -30,8 +33,20 @@ # has_and_belongs_to_many :guidances, join_table: "guidance_in_group" + # =============== + # = Validations = + # =============== - validates :name, :org, presence: {message: _("can't be blank")} + validates :name, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE, scope: :org_id } + + validates :org, presence: { message: PRESENCE_MESSAGE } + + validates :optional_subset, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + validates :published, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } # EVALUATE CLASS AND INSTANCE METHODS BELOW diff --git a/app/models/identifier_scheme.rb b/app/models/identifier_scheme.rb index f96cd79..9243b11 100644 --- a/app/models/identifier_scheme.rb +++ b/app/models/identifier_scheme.rb @@ -13,8 +13,25 @@ # class IdentifierScheme < ActiveRecord::Base + include ValidationMessages + include ValidationValues + + ## + # The maximum length for a name + NAME_MAXIMUM_LENGTH = 30 + has_many :user_identifiers has_many :users, through: :user_identifiers - - validates :name, uniqueness: {message: _("must be unique")}, presence: {message: _("can't be blank")} + + # =============== + # = Validations = + # =============== + + validates :name, uniqueness: { message: UNIQUENESS_MESSAGE }, + presence: { message: PRESENCE_MESSAGE }, + length: { maximum: NAME_MAXIMUM_LENGTH } + + validates :active, inclusion: { message: INCLUSION_MESSAGE, + in: BOOLEAN_VALUES } + end diff --git a/app/models/language.rb b/app/models/language.rb index 7c52f72..6ee7e9b 100644 --- a/app/models/language.rb +++ b/app/models/language.rb @@ -9,19 +9,52 @@ # name :string # -class Language < ActiveRecord::Base - ## - # Associations - has_many :users - has_many :orgs - - ## - # Validations - # Cannot do FastGettext translations here because we constantize LANGUAGES in initializers/constants.rb - validates :abbreviation, presence: {message: "can't be blank"}, uniqueness: {message: "must be unique"} - - scope :sorted_by_abbreviation, -> { all.order(:abbreviation) } - scope :default, -> { where(default_language: true).first } - # Retrieves the id for a given abbreviation of a language - scope :id_for, -> (abbreviation) { where(abbreviation: abbreviation).pluck(:id).first } +class Language < ActiveRecord::Base + # frozen_string_literal: true + + include ValidationValues + + # ============= + # = Constants = + # ============= + + ABBREVIATION_MAXIMUM_LENGTH = 5 + + ABBREVIATION_FORMAT = /\A[a-z]{2}(\_[A-Z]{2})?\Z/ + + NAME_MAXIMUM_LENGTH = 20 + + # ================ + # = Associations = + # ================ + + has_many :users + + has_many :orgs + + + # =============== + # = Validations = + # =============== + + validates :name, presence: { message: "can't be blank" }, + length: { maximum: NAME_MAXIMUM_LENGTH } + + validates :abbreviation, presence: { message: "can't be blank" }, + uniqueness: { message: "must be unique" }, + length: { maximum: ABBREVIATION_MAXIMUM_LENGTH }, + format: { with: ABBREVIATION_FORMAT } + + validates :default_language, inclusion: { in: BOOLEAN_VALUES } + + # ========== + # = Scopes = + # ========== + + scope :sorted_by_abbreviation, -> { all.order(:abbreviation) } + scope :default, -> { where(default_language: true).first } + # Retrieves the id for a given abbreviation of a language + scope :id_for, -> (abbreviation) { + where(abbreviation: abbreviation).pluck(:id).first + } end diff --git a/app/models/note.rb b/app/models/note.rb index 17f86f0..a78dc00 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -22,12 +22,28 @@ # class Note < ActiveRecord::Base - ## - # Associations + include ValidationMessages + include ValidationValues + + # ================ + # = Associations = + # ================ + belongs_to :answer + belongs_to :user + # =============== + # = Validations = + # =============== - validates :text, :answer, :user, presence: {message: _("can't be blank")} + validates :text, presence: { message: PRESENCE_MESSAGE } + + validates :answer, presence: { message: PRESENCE_MESSAGE } + + validates :user, presence: { message: PRESENCE_MESSAGE } + + validates :archived, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } end diff --git a/app/models/notification.rb b/app/models/notification.rb index e6386e3..cfbe0cb 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -15,13 +15,44 @@ # class Notification < ActiveRecord::Base + include ValidationMessages + include ValidationValues + enum level: %i[info warning danger] enum notification_type: %i[global] - has_and_belongs_to_many :users, dependent: :destroy, join_table: 'notification_acknowledgements' + # ================ + # = Associations = + # ================ - validates :notification_type, :title, :level, :starts_at, :expires_at, :body, presence: true - validate :valid_dates + has_and_belongs_to_many :users, dependent: :destroy, + join_table: 'notification_acknowledgements' + + + # =============== + # = Validations = + # =============== + + validates :notification_type, presence: { message: PRESENCE_MESSAGE } + + validates :title, presence: { message: PRESENCE_MESSAGE } + + validates :level, presence: { message: PRESENCE_MESSAGE } + + validates :body, presence: { message: PRESENCE_MESSAGE } + + validates :dismissable, inclusion: { in: BOOLEAN_VALUES } + + validates :starts_at, presence: { message: PRESENCE_MESSAGE }, + after: { date: Date.today, on: :create } + + validates :expires_at, presence: { message: PRESENCE_MESSAGE }, + after: { date: Date.tomorrow, on: :create } + + + # ========== + # = Scopes = + # ========== scope :active, (lambda do where('starts_at <= :now and :now < expires_at', now: Time.now) @@ -43,6 +74,8 @@ users.include?(user) if user.present? && dismissable? end + private + # Validate Notification dates def valid_dates return false if starts_at.blank? || expires_at.blank? diff --git a/app/models/org.rb b/app/models/org.rb index 6f1360e..443c0e3 100644 --- a/app/models/org.rb +++ b/app/models/org.rb @@ -33,38 +33,82 @@ # class Org < ActiveRecord::Base + include ValidationMessages + include ValidationValues include GlobalHelpers include FlagShihTzu extend Dragonfly::Model::Validations validates_with OrgLinksValidator + LOGO_FORMATS = %w[jpeg png gif jpg bmp].freeze + # Stores links as an JSON object: { org: [{"link":"www.example.com","text":"foo"}, ...] } # The links are validated against custom validator allocated at validators/template_links_validator.rb serialize :links, JSON - ## - # Associations -# belongs_to :organisation_type # depricated, but cannot be removed until migration run + + # ================ + # = Associations = + # ================ + belongs_to :language - has_many :guidance_groups + + belongs_to :region + + has_many :guidance_groups, dependent: :destroy + has_many :templates + has_many :users + has_many :annotations has_and_belongs_to_many :token_permission_types, join_table: "org_token_permissions", unique: true has_many :org_identifiers + has_many :identifier_schemes, through: :org_identifiers - ## - # Validators - validates :name, presence: {message: _("can't be blank")}, uniqueness: {message: _("must be unique")} + + # =============== + # = Validations = + # =============== + + validates :name, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE } + + validates :abbreviation, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE } + + validates :is_other, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + validates :language, presence: { message: PRESENCE_MESSAGE } + + validates :contact_email, email: { allow_nil: true }, + presence: { message: PRESENCE_MESSAGE, + if: :feedback_enabled } + + validates :org_type, presence: { message: PRESENCE_MESSAGE } + + validates :feedback_enabled, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + validates :feedback_email_subject, presence: { message: PRESENCE_MESSAGE, + if: :feedback_enabled } + + validates :feedback_email_msg, presence: { message: PRESENCE_MESSAGE, + if: :feedback_enabled } + + validates_property :format, of: :logo, in: LOGO_FORMATS, + message: _("must be one of the following formats: jpeg, jpg, png, gif, bmp") + + validates_size_of :logo, maximum: 500.kilobytes, message: _("can't be larger than 500KB") + # allow validations for logo upload dragonfly_accessor :logo do after_assign :resize_image end - validates_property :format, of: :logo, in: ['jpeg', 'png', 'gif','jpg','bmp'], message: _("must be one of the following formats: jpeg, jpg, png, gif, bmp") - validates_size_of :logo, maximum: 500.kilobytes, message: _("can't be larger than 500KB") ## # Define Bit Field values @@ -178,20 +222,25 @@ end private - ## - # checks size of logo and resizes if necessary - # - def resize_image - unless logo.nil? - if logo.height != 100 - self.logo = logo.thumb('x100') # resize height and maintain aspect ratio - end + + ## + # checks size of logo and resizes if necessary + # + def resize_image + unless logo.nil? + if logo.height != 100 + self.logo = logo.thumb('x100') # resize height and maintain aspect ratio end end + end - # creates a dfefault Guidance Group on create on the Org - def create_guidance_group - GuidanceGroup.create(name: self.abbreviation? ? self.abbreviation : self.name , org_id: self.id) - end - + # creates a dfefault Guidance Group on create on the Org + def create_guidance_group + GuidanceGroup.create!({ + name: abbreviation? ? self.abbreviation : self.name , + org: self, + optional_subset: false, + published: false, + }) + end end diff --git a/app/models/org_identifier.rb b/app/models/org_identifier.rb index 4ad113a..618ed2a 100644 --- a/app/models/org_identifier.rb +++ b/app/models/org_identifier.rb @@ -17,14 +17,34 @@ # class OrgIdentifier < ActiveRecord::Base + include ValidationMessages + + # ================ + # = Associations = + # ================ + belongs_to :org belongs_to :identifier_scheme - + + # =============== + # = Validations = + # =============== + # Should only be able to have one identifier per scheme! - validates_uniqueness_of :identifier_scheme, scope: :org - - validates :identifier, :org, :identifier_scheme, presence: {message: _("can't be blank")} - + validates :identifier_scheme_id, uniqueness: { scope: :org_id, + message: UNIQUENESS_MESSAGE } + + validates :identifier, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE } + + validates :org, presence: { message: PRESENCE_MESSAGE } + + validates :identifier_scheme, presence: { message: PRESENCE_MESSAGE } + + # =========================== + # = Public instance methods = + # =========================== + def attrs=(hash) write_attribute(:attrs, (hash.is_a?(Hash) ? hash.to_json.to_s : '{}')) end diff --git a/app/models/perm.rb b/app/models/perm.rb index 1cc7c14..4dfde8c 100644 --- a/app/models/perm.rb +++ b/app/models/perm.rb @@ -9,18 +9,38 @@ # class Perm < ActiveRecord::Base - ## - # Associations - has_and_belongs_to_many :users, :join_table => :users_perms + include ValidationMessages - validates :name, presence: {message: _("can't be blank")}, uniqueness: {message: _("must be unique")} + # ================ + # = Associations = + # ================ - scope :add_orgs, -> {Perm.find_by(name: 'add_organisations')} - scope :change_affiliation, -> {Perm.find_by(name: 'change_org_affiliation')} - scope :grant_permissions, -> {Perm.find_by(name: 'grant_permissions')} - scope :modify_templates, -> {Perm.find_by(name: 'modify_templates')} - scope :modify_guidance, -> {Perm.find_by(name: 'modify_guidance')} - scope :use_api, -> {Perm.find_by(name: 'use_api')} - scope :change_org_details, -> {Perm.find_by(name: 'change_org_details')} - scope :grant_api, -> {Perm.find_by(name: 'grant_api_to_orgs')} + has_and_belongs_to_many :users, join_table: :users_perms + + # =============== + # = Validations = + # =============== + + validates :name, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE } + + # ========== + # = Scopes = + # ========== + + scope :add_orgs, -> { Perm.find_by(name: 'add_organisations') } + + scope :change_affiliation, -> { Perm.find_by(name: 'change_org_affiliation') } + + scope :grant_permissions, -> { Perm.find_by(name: 'grant_permissions') } + + scope :modify_templates, -> { Perm.find_by(name: 'modify_templates') } + + scope :modify_guidance, -> { Perm.find_by(name: 'modify_guidance') } + + scope :use_api, -> { Perm.find_by(name: 'use_api') } + + scope :change_org_details, -> { Perm.find_by(name: 'change_org_details') } + + scope :grant_api, -> { Perm.find_by(name: 'grant_api_to_orgs') } end diff --git a/app/models/phase.rb b/app/models/phase.rb index 68fcbd3..42312b4 100644 --- a/app/models/phase.rb +++ b/app/models/phase.rb @@ -27,12 +27,17 @@ # [+Created:+] 03/09/2014 # [+Copyright:+] Digital Curation Centre and University of California Curation Center class Phase < ActiveRecord::Base + include ValidationMessages + include ValidationValues + include ActsAsSortable + ## # Sort order: Number ASC default_scope { order(number: :asc) } - ## - # Associations + # ================ + # = Associations = + # ================ belongs_to :template has_one :prefix_section, -> (phase) { @@ -56,7 +61,24 @@ }, class_name: "Section" - validates :title, :number, :template, presence: { message: _("can't be blank") } + # =============== + # = Validations = + # =============== + + validates :title, presence: { message: PRESENCE_MESSAGE } + + validates :number, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE, + scope: :template_id } + + validates :template, presence: { message: PRESENCE_MESSAGE } + + validates :modifiable, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + # ========== + # = Scopes = + # ========== scope :titles, -> (template_id) { Phase.where(template_id: template_id).select(:id, :title) diff --git a/app/models/plan.rb b/app/models/plan.rb index 57f874c..7609c70 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -35,46 +35,93 @@ class Plan < ActiveRecord::Base include ConditionalUserMailer include ExportablePlan - before_validation :set_creation_defaults + include ValidationMessages + include ValidationValues + + # ============= + # = Constants = + # ============= ## - # Associations - belongs_to :template - has_many :phases, through: :template - has_many :sections, through: :phases - has_many :questions, through: :sections - has_many :themes, through: :questions - has_many :answers, dependent: :destroy - has_many :notes, through: :answers - has_many :roles, dependent: :destroy - has_many :users, through: :roles - has_and_belongs_to_many :guidance_groups, join_table: :plans_guidance_groups + # (in mm) + A4_PAGE_HEIGHT = 297 - accepts_nested_attributes_for :template - has_many :exported_plans + ## + # (in mm) + A4_PAGE_WIDTH = 210 - has_many :roles + ## + # Round estimate up to nearest 5% + ROUNDING = 5 -# COMMENTED OUT THE DIRECT CONNECTION HERE TO Users to prevent assignment of users without an access_level specified (currently defaults to creator) -# has_many :users, through: :roles + ## + # convert font point size to mm + FONT_HEIGHT_CONVERSION_FACTOR = 0.35278 + ## + # Assume glyph width averages 2/5 the height + FONT_WIDTH_HEIGHT_RATIO = 0.4 - accepts_nested_attributes_for :roles # public is a Ruby keyword so using publicly enum visibility: [:organisationally_visible, :publicly_visible, :is_test, :privately_visible] - #TODO: work out why this messes up plan creation : - # briley: Removed reliance on :users, its really on :roles (shouldn't have a plan without at least a creator right?) It should be ok like this though now -# validates :template, :title, presence: true + # ================ + # = Associations = + # ================ - ## - # Constants - A4_PAGE_HEIGHT = 297 #(in mm) - A4_PAGE_WIDTH = 210 #(in mm) - ROUNDING = 5 #round estimate up to nearest 5% - FONT_HEIGHT_CONVERSION_FACTOR = 0.35278 #convert font point size to mm - FONT_WIDTH_HEIGHT_RATIO = 0.4 #Assume glyph width averages 2/5 the height + belongs_to :template + + has_many :phases, through: :template + + has_many :sections, through: :phases + + has_many :questions, through: :sections + + has_many :themes, through: :questions + + has_many :answers, dependent: :destroy + + has_many :notes, through: :answers + + has_many :roles, dependent: :destroy + + has_many :users, through: :roles + + has_and_belongs_to_many :guidance_groups, join_table: :plans_guidance_groups + + has_many :exported_plans + + has_many :roles + + # ============== + # = Attributes = + # ============== + + accepts_nested_attributes_for :template + + accepts_nested_attributes_for :roles + + # =============== + # = Validations = + # =============== + + validates :title, presence: { message: PRESENCE_MESSAGE } + + validates :template, presence: { message: PRESENCE_MESSAGE } + + validates :feedback_requested, inclusion: { in: BOOLEAN_VALUES } + + validates :complete, inclusion: { in: BOOLEAN_VALUES } + + # ============= + # = Callbacks = + # ============= + before_validation :set_creation_defaults + + # ========== + # = Scopes = + # ========== # Scope queries # Note that in ActiveRecord::Enum the mappings are exposed through a class method with the pluralized attribute name (e.g visibilities rather than visibility) diff --git a/app/models/pref.rb b/app/models/pref.rb index ec0b9cb..e0e553e 100644 --- a/app/models/pref.rb +++ b/app/models/pref.rb @@ -8,15 +8,26 @@ # class Pref < ActiveRecord::Base + include ValidationMessages + ## # Serialize prefs to JSON # The settings object only stores deviations from the default serialize :settings, JSON - ## - # Associations + # ================ + # = Associations = + # ================ belongs_to :user + # =============== + # = Validations = + # =============== + + validates :user, presence: { message: PRESENCE_MESSAGE } + + validates :settings, presence: { message: PRESENCE_MESSAGE } + ## # Returns the hash generated from default preferences # diff --git a/app/models/question.rb b/app/models/question.rb index fd1b8b3..180b675 100644 --- a/app/models/question.rb +++ b/app/models/question.rb @@ -24,31 +24,68 @@ # class Question < ActiveRecord::Base + include ValidationMessages + include ActsAsSortable + # include ## # Sort order: Number ASC default_scope { order(number: :asc) } - ## - # Associations - has_many :answers, :dependent => :destroy - has_many :question_options, :dependent => :destroy, :inverse_of => :question # inverse_of needed for nester forms - has_many :annotations, :dependent => :destroy, :inverse_of => :question + # ================ + # = Associations = + # ================ + + has_many :answers, dependent: :destroy + + # inverse_of needed for nested forms + has_many :question_options, dependent: :destroy, inverse_of: :question + + has_many :annotations, dependent: :destroy, inverse_of: :question + has_and_belongs_to_many :themes, join_table: "questions_themes" + belongs_to :section + belongs_to :question_format + has_one :phase, through: :section + has_one :template, through: :section - ## - # Nested Attributes + + # =============== + # = Validations = + # =============== + + validates :text, presence: { message: PRESENCE_MESSAGE } + + validates :section, presence: { message: PRESENCE_MESSAGE } + + validates :question_format, presence: { message: PRESENCE_MESSAGE } + + validates :number, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { scope: :section_id, + message: UNIQUENESS_MESSAGE } + + + # ===================== + # = Nested Attributes = + # ===================== + # TODO: evaluate if we need this - accepts_nested_attributes_for :answers, :reject_if => lambda {|a| a[:text].blank? }, :allow_destroy => true - accepts_nested_attributes_for :question_options, :reject_if => lambda {|a| a[:text].blank? }, :allow_destroy => true - accepts_nested_attributes_for :annotations, :allow_destroy => true + accepts_nested_attributes_for :answers, reject_if: -> (a) { a[:text].blank? }, + allow_destroy: true + + accepts_nested_attributes_for :question_options, allow_destroy: true, + reject_if: -> (a) { a[:text].blank? } + + accepts_nested_attributes_for :annotations, allow_destroy: true - validates :text, :section, :number, presence: {message: _("can't be blank")} + # =========================== + # = Public instance methods = + # =========================== ## # returns the text from the question diff --git a/app/models/question_format.rb b/app/models/question_format.rb index d450406..a911ce0 100644 --- a/app/models/question_format.rb +++ b/app/models/question_format.rb @@ -12,15 +12,23 @@ # class QuestionFormat < ActiveRecord::Base + include ValidationMessages + include ValidationValues ## + # + FORMAT_TYPES = %i[textarea textfield radiobuttons checkbox dropdown + multiselectbox date rda_metadata] + ## # Associations has_many :questions - enum formattype: [ :textarea, :textfield, :radiobuttons, :checkbox, :dropdown, :multiselectbox, :date, :rda_metadata ] + enum formattype: FORMAT_TYPES - validates :title, presence: {message: _("can't be blank")}, uniqueness: {message: _("must be unique")} + validates :title, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE } + validates :option_based, inclusion: { in: BOOLEAN_VALUES } # Retrieves the id for a given formattype passed scope :id_for, -> (formattype) { where(formattype: formattype).pluck(:id).first } diff --git a/app/models/question_option.rb b/app/models/question_option.rb index cac8958..eb02e20 100644 --- a/app/models/question_option.rb +++ b/app/models/question_option.rb @@ -20,15 +20,41 @@ # class QuestionOption < ActiveRecord::Base - ## - # Associations + include ValidationMessages + include ValidationValues + + # ================ + # = Associations = + # ================ + belongs_to :question + has_and_belongs_to_many :answers, join_table: :answers_question_options - validates :text, :question, :number, presence: {message: _("can't be blank")} + # =============== + # = Validations = + # =============== + + validates :text, presence: { message: PRESENCE_MESSAGE } + + validates :question, presence: { message: PRESENCE_MESSAGE } + + validates :number, presence: { message: PRESENCE_MESSAGE } + + validates :is_default, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + # ========== + # = Scopes = + # ========== scope :by_number, -> { order(:number) } + + # ================= + # = Class methods = + # ================= + ## # deep copy the given question_option and all it's associations # @@ -40,6 +66,10 @@ return question_option_copy end + # =========================== + # = Public instance methods = + # =========================== + def deep_copy(**options) copy = self.dup copy.question_id = options.fetch(:question_id, nil) diff --git a/app/models/region.rb b/app/models/region.rb index d198285..6dd483f 100644 --- a/app/models/region.rb +++ b/app/models/region.rb @@ -9,11 +9,27 @@ # super_region_id :integer # -class Region < ActiveRecord::Base - has_many :sub_regions, class_name: 'Region', foreign_key: 'super_region_id' - - belongs_to :super_region, class_name: 'Region' - - validates :name, presence: {message: _("can't be blank")}, uniqueness: {message: _("must be unique")} - validates :abbreviation, uniqueness: {message: _("must be unique")}, allow_nil: true +class Region < ActiveRecord::Base + include ValidationMessages + + # ================ + # = Associations = + # ================ + + has_many :sub_regions, class_name: 'Region', foreign_key: 'super_region_id' + + belongs_to :super_region, class_name: 'Region' + + # =============== + # = Validations = + # =============== + + validates :name, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE } + + validates :description, presence: true + + validates :abbreviation, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE } + end diff --git a/app/models/role.rb b/app/models/role.rb index bbcfb53..8726a29 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -22,8 +22,10 @@ # class Role < ActiveRecord::Base - after_initialize :set_defaults + include FlagShihTzu + include ValidationMessages + include ValidationValues ## # Associationsrequire "role" @@ -41,8 +43,27 @@ 5 => :reviewer, # 16 column: 'access' - validates :user, :plan, :access, presence: {message: _("can't be blank")} - validates :access, numericality: {greater_than: 0, message: _("can't be less than zero")} + # =============== + # = Validations = + # =============== + + validates :user, presence: { message: PRESENCE_MESSAGE } + + validates :plan, presence: { message: PRESENCE_MESSAGE } + + validates :active, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + validates :access, presence: { message: PRESENCE_MESSAGE }, + numericality: { greater_than: 0, only_integer: true, + message: _("can't be less than zero") } + + # ============= + # = Callbacks = + # ============= + + # TODO: Push this down to the DB constraints + after_initialize :set_defaults ## # Roles with given FlagShihTzu access flags diff --git a/app/models/section.rb b/app/models/section.rb index 116e95f..fe812b6 100644 --- a/app/models/section.rb +++ b/app/models/section.rb @@ -22,28 +22,57 @@ # class Section < ActiveRecord::Base - ## - # Associations + include ValidationMessages + include ValidationValues + include ActsAsSortable + + # ================ + # = Associations = + # ================ + belongs_to :phase belongs_to :organisation has_many :questions, dependent: :destroy has_one :template, through: :phase - # Link the data - accepts_nested_attributes_for :questions, reject_if: lambda { |a| a[:text].blank? }, allow_destroy: true + # =============== + # = Validations = + # =============== + validates :phase, presence: { message: PRESENCE_MESSAGE } - ## - # Validations - validates :phase, :title, presence: { message: _("can't be blank") } + validates :title, presence: { message: PRESENCE_MESSAGE } - validates :number, presence: { message: _("can't be blank") }, - uniqueness: { scope: :phase_id } + # validates :description, presence: { message: PRESENCE_MESSAGE } + validates :number, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { scope: :phase_id, + message: UNIQUENESS_MESSAGE } + + validates :published, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + validates :modifiable, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + # ============= + # = Callbacks = + # ============= + + # TODO: Move this down to DB constraints before_validation :set_defaults - ## - # Scopes + # ===================== + # = Nested Attributes = + # ===================== + + accepts_nested_attributes_for :questions, + reject_if: -> (a) { a[:text].blank? }, + allow_destroy: true + + # ========== + # = Scopes = + # ========== # The sections for this Phase that have been added by the admin # @@ -57,22 +86,9 @@ # @return [ActiveRecord::Relation] Returns the sections that aren't modifiable scope :not_modifiable, -> { where(modifiable: false) } - # ================= - # = Class methods = - # ================= - - def self.update_numbers!(*ids, phase:) - # Ensure only section ids belonging to this Phase are included. - ids = ids.map(&:to_i) & phase.section_ids - return if ids.empty? - case connection.adapter_name - when "PostgreSQL" then update_numbers_postgresql!(ids) - when "Mysql2" then update_numbers_mysql2!(ids) - else - update_numbers_sequentially!(ids) - end - end - + # =========================== + # = Public instance methods = + # =========================== ## # return the title of the section @@ -106,29 +122,9 @@ private - def self.update_numbers_postgresql!(ids) - # Build an Array with each ID and its relative position in the Array - values = ids.each_with_index.map { |id, i| "(#{id}, #{i + 1})" }.join(", ") - # Run a single UPDATE query for all records. - query = <<~SQL - UPDATE #{table_name} \ - SET number = svals.number \ - FROM (VALUES #{sanitize_sql(values)}) AS svals(id, number) \ - WHERE svals.id = #{table_name}.id; - SQL - connection.execute(query) - end - - def self.update_numbers_mysql2!(ids) - ids_string = ids.map { |id| "'#{id}'" }.join(",") - update_all(%Q{ number = FIELD(id, #{sanitize_sql(ids_string)}) }) - end - - def self.update_numbers_sequentially!(ids) - ids.each_with_index.map do |id, number| - find(id).update_attribute(:number, number + 1) - end - end + # ============================ + # = Private instance methods = + # ============================ def set_defaults self.modifiable = true if modifiable.nil? diff --git a/app/models/template.rb b/app/models/template.rb index 50e2616..ef69476 100644 --- a/app/models/template.rb +++ b/app/models/template.rb @@ -33,26 +33,59 @@ class Template < ActiveRecord::Base include GlobalHelpers - include ActiveModel::Validations + include ValidationMessages + include ValidationValues validates_with TemplateLinksValidator - before_validation :set_defaults - after_update :reconcile_published, if: Proc.new { |template| template.published? } # Stores links as an JSON object: { funder: [{"link":"www.example.com","text":"foo"}, ...], sample_plan: [{"link":"www.example.com","text":"foo"}, ...]} # The links is validated against custom validator allocated at validators/template_links_validator.rb serialize :links, JSON - ## - # Associations + # ================ + # = Associations = + # ================ + belongs_to :org + has_many :plans + has_many :phases, dependent: :destroy + has_many :sections, through: :phases + has_many :questions, through: :sections + has_many :annotations, through: :questions + # =============== + # = Validations = + # =============== + + validates :title, presence: { message: PRESENCE_MESSAGE } + + validates :org, presence: { message: PRESENCE_MESSAGE } + + validates :locale, presence: { message: PRESENCE_MESSAGE } + + validates :version, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE, + scope: :family_id } + + validates :visibility, presence: { message: PRESENCE_MESSAGE } + + validates :family_id, presence: { message: PRESENCE_MESSAGE } + + + # ============= + # = Callbacks = + # ============= + before_validation :set_defaults + + after_update :reconcile_published, if: -> (template) { template.published? } + + # ========== # = Scopes = # ========== diff --git a/app/models/theme.rb b/app/models/theme.rb index 4a35f38..857ee30 100644 --- a/app/models/theme.rb +++ b/app/models/theme.rb @@ -11,19 +11,34 @@ # class Theme < ActiveRecord::Base + include ValidationMessages - ## - # Associations + # ================ + # = Associations = + # ================ + has_and_belongs_to_many :questions, join_table: "questions_themes" has_and_belongs_to_many :guidances, join_table: "themes_in_guidance" + # =============== + # = Validations = + # =============== - validates :title, presence: {message: _("can't be blank")} + validates :title, presence: { message: PRESENCE_MESSAGE } + + # ========== + # = Scopes = + # ========== scope :search, -> (term) { search_pattern = "%#{term}%" where("title LIKE ? OR description LIKE ?", search_pattern, search_pattern) } + + # =========================== + # = Public instance methods = + # =========================== + ## # returns the title of the theme # @@ -31,5 +46,4 @@ def to_s title end - end diff --git a/app/models/token_permission_type.rb b/app/models/token_permission_type.rb index 71370f6..8c75ee3 100644 --- a/app/models/token_permission_type.rb +++ b/app/models/token_permission_type.rb @@ -10,22 +10,42 @@ # class TokenPermissionType < ActiveRecord::Base + include ValidationMessages + + # ============= + # = Constants = + # ============= + ## - # Associations - #has_and_belongs_to_many :org_token_permissions, join_table: "org_token_permissions" -# has_and_belongs_to_many :organisations, join_table: 'org_token_permissions', unique: true + # + GUIDANCES = TokenPermissionType.where(token_type: 'guidances').first.freeze + + ## + # + PLANS = TokenPermissionType.where(token_type: 'plans').first.freeze + + ## + # + TEMPLATES = TokenPermissionType.where(token_type: 'templates').first.freeze + + ## + # + STATISTICS = TokenPermissionType.where(token_type: 'statistics').first.freeze + + + # ================ + # = Associations = + # ================ + has_and_belongs_to_many :orgs, join_table: 'org_token_permissions', unique: true - ## - # Validators - validates :token_type, presence: {message: _("can't be blank")}, uniqueness: {message: _("must be unique")} - ## - # Constant Token Permission Types - GUIDANCES = TokenPermissionType.where(token_type: 'guidances').first.freeze - PLANS = TokenPermissionType.where(token_type: 'plans').first.freeze - TEMPLATES = TokenPermissionType.where(token_type: 'templates').first.freeze - STATISTICS = TokenPermissionType.where(token_type: 'statistics').first.freeze + # ============== + # = Validators = + # ============== + + validates :token_type, presence: { message: PRESENCE_MESSAGE }, + uniqueness: { message: UNIQUENESS_MESSAGE } ## diff --git a/app/models/user.rb b/app/models/user.rb index 7e4157d..4c01e36 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -49,6 +49,9 @@ class User < ActiveRecord::Base include ConditionalUserMailer + include ValidationMessages + include ValidationValues + ## # Devise # Include default devise modules. Others available are: @@ -63,16 +66,27 @@ # User Notification Preferences serialize :prefs, Hash - ## - # Associations + # ================ + # = Associations = + # ================ + + has_and_belongs_to_many :perms, join_table: :users_perms + belongs_to :language + belongs_to :org + has_one :pref + has_many :answers + has_many :notes + has_many :exported_plans + has_many :roles, dependent: :destroy + has_many :plans, through: :roles do def filter(query) return self unless query.present? @@ -88,14 +102,26 @@ end end + has_many :user_identifiers + has_many :identifier_schemes, through: :user_identifiers + has_and_belongs_to_many :notifications, dependent: :destroy, join_table: 'notification_acknowledgements' - validates :email, email: true, allow_nil: true, uniqueness: {message: _("must be unique")} - ## - # Scopes + # =============== + # = Validations = + # =============== + + validates :active, inclusion: { in: BOOLEAN_VALUES, + message: INCLUSION_MESSAGE } + + + # ========== + # = Scopes = + # ========== + default_scope { includes(:org, :perms) } # Retrieves all of the org_admins for the specified org @@ -115,8 +141,16 @@ end } + # ============= + # = Callbacks = + # ============= + after_update :when_org_changes + # =========================== + # = Public instance methods = + # =========================== + ## # This method uses Devise's built-in handling for inactive users def active_for_authentication? @@ -139,7 +173,6 @@ end end - ## # gives either the name of the user, or the email if name unspecified # @@ -162,7 +195,6 @@ self.plans.includes(:template).where("roles.active": true).where(Role.not_reviewer_condition) end - ## # Returns the user's identifier for the specified scheme name # @@ -349,6 +381,11 @@ end private + + # ============================ + # = Private instance methods = + # ============================ + def when_org_changes if org_id != org_id_was unless can_change_org? diff --git a/app/models/user_identifier.rb b/app/models/user_identifier.rb index 60d83ee..5a1f5ad 100644 --- a/app/models/user_identifier.rb +++ b/app/models/user_identifier.rb @@ -20,11 +20,23 @@ # class UserIdentifier < ActiveRecord::Base + include ValidationMessages + + # ================ + # = Associations = + # ================ + belongs_to :user belongs_to :identifier_scheme - - # Should only be able to have one identifier per scheme! - validates_uniqueness_of :identifier_scheme, scope: :user - - validates :identifier, :user, :identifier_scheme, presence: {message: _("can't be blank")} + + # =============== + # = Validations = + # =============== + + validates :user, presence: true + + validates :identifier_scheme, presence: { message: PRESENCE_MESSAGE } + + validates :identifier, presence: { message: PRESENCE_MESSAGE } + end diff --git a/app/validators/after_validator.rb b/app/validators/after_validator.rb new file mode 100644 index 0000000..57fe579 --- /dev/null +++ b/app/validators/after_validator.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true +class AfterValidator < ActiveModel::EachValidator + + DEFAULT_MESSAGE = _("must be after %{date}") + + def validate_each(record, attribute, value) + return if value.nil? + return if record.persisted? && options[:on].to_s == 'create' + return if record.new_record? && options[:on].to_s == 'update' + date = options[:date] + msg = options.fetch(:message, DEFAULT_MESSAGE % { date: options[:date] }) + record.errors.add(attribute, msg) if value.to_date < options[:date] + end +end diff --git a/db/seeds.rb b/db/seeds.rb index ec54a12..2450b5e 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -2,29 +2,69 @@ # This file should contain all the record creation needed to seed the database with its default values. # The data can then be loaded with the rake db:seed (or created alongside the db with db:setup). +require 'factory_bot' +include FactoryBot::Syntax::Methods + # Identifier Schemes # ------------------------------------------------------- identifier_schemes = [ - {name: 'orcid', description: 'ORCID', active: true, - logo_url:'http://orcid.org/sites/default/files/images/orcid_16x16.png', - user_landing_url:'https://orcid.org' }, - {name: 'shibboleth', description: 'Your institutional credentials', active: true, + { + name: 'orcid', + description: 'ORCID', + active: true, + logo_url:'http://orcid.org/sites/default/files/images/orcid_16x16.png', + user_landing_url:'https://orcid.org' + }, + { + name: 'shibboleth', + description: 'Your institutional credentials', + active: true, + logo_url: 'http://newsite.shibboleth.net/wp-content/uploads/2017/01/Shibboleth-logo_2000x1200-1.png', + user_landing_url: "https://example.com" }, ] -identifier_schemes.map{ |is| IdentifierScheme.create!(is) if IdentifierScheme.find_by(name: is[:name]).nil? } +identifier_schemes.map { |is| create(:identifier_scheme, is) } # Question Formats # ------------------------------------------------------- question_formats = [ - {title: "Text area", option_based: false, formattype: 0}, - {title: "Text field", option_based: false, formattype: 1}, - {title: "Radio buttons", option_based: true, formattype: 2}, - {title: "Check box", option_based: true, formattype: 3}, - {title: "Dropdown", option_based: true, formattype: 4}, - {title: "Multi select box", option_based: true, formattype: 5}, - {title: "Date", option_based: true, formattype: 6} + { + title: "Text area", + option_based: false, + formattype: 0 + }, + { + title: "Text field", + option_based: false, + formattype: 1 + }, + { + title: "Radio buttons", + option_based: true, + formattype: 2 + }, + { + title: "Check box", + option_based: true, + formattype: 3 + }, + { + title: "Dropdown", + option_based: true, + formattype: 4 + }, + { + title: "Multi select box", + option_based: true, + formattype: 5 + }, + { + title: "Date", + option_based: true, + formattype: 6 + } ] -question_formats.map{ |qf| QuestionFormat.create!(qf) if QuestionFormat.find_by(title: qf[:title]).nil? } +question_formats.map{ |qf| create(:question_format, qf) } # Languages (check config/locales for any ones not defined here) # ------------------------------------------------------- @@ -50,7 +90,7 @@ name: 'Español', default_language: false} ] -languages.map{ |l| Language.create!(l) if Language.find_by(abbreviation: l[:abbreviation]).nil? } +languages.map { |l| create(:language, l) } # Scan through the locale files and add an entry if a file is present but # not defined in this seed file @@ -128,7 +168,7 @@ {name: 'grant_api_to_orgs'} ] -perms.map{ |p| Perm.create!(p) if Perm.find_by(name: p[:name]).nil? } +perms.map{ |p| create(:perm, p) } # Guidance Themes # ------------------------------------------------------- @@ -148,7 +188,7 @@ {title: 'Budget'}, {title: 'Related Policies'} ] -themes.map{ |t| Theme.create!(t) if Theme.find_by(title: t[:title]).nil? } +themes.map{ |t| create(:theme, t) } # Token Permission Types # ------------------------------------------------------- @@ -158,15 +198,15 @@ {token_type: 'templates', text_description: 'allows a user access to the templates api endpoint'}, {token_type: 'statistics', text_description: 'allows a user access to the statistics api endpoint'} ] -token_permission_types.map{ |tpt| TokenPermissionType.create!(tpt) if TokenPermissionType.find_by(token_type: tpt[:token_type]).nil? } +token_permission_types.map{ |tpt| create(:token_permission_type, tpt) } # Create our generic organisation, a funder and a University # ------------------------------------------------------- orgs = [ - {name: Rails.configuration.branding[:organisation][:name], - abbreviation: Rails.configuration.branding[:organisation][:abbreviation], + {name: Branding.fetch(:organisation, :name), + abbreviation: Branding.fetch(:organisation, :abbreviation), org_type: 4, links: {"org":[]}, - language_id: Language.find_by(abbreviation: 'en_GB'), + language: Language.find_by(abbreviation: 'en_GB'), token_permission_types: TokenPermissionType.all}, {name: 'Government Agency', abbreviation: 'GA', @@ -177,7 +217,7 @@ org_type: 1, links: {"org":[]}, language: Language.find_by(abbreviation: 'en_GB')} ] -orgs.map{ |o| Org.create!(o) if Org.find_by(abbreviation: o[:abbreviation]).nil? } +orgs.map{ |o| create(:org, o) } # Create a Super Admin associated with our generic organisation, # an Org Admin for our funder and an Org Admin and User for our University @@ -188,7 +228,7 @@ surname: "Admin", password: "password123", password_confirmation: "password123", - org: Org.find_by(abbreviation: Rails.configuration.branding[:organisation][:abbreviation]), + org: Org.find_by(abbreviation: Branding.fetch(:organisation, :abbreviation)), language: Language.find_by(abbreviation: FastGettext.locale), perms: Perm.all, accept_terms: true, @@ -226,7 +266,7 @@ accept_terms: true, confirmed_at: Time.zone.now} ] -users.map{ |u| User.create!(u) if User.find_by(email: u[:email]).nil? } +users.map{ |u| create(:user, u) } # Create a Guidance Group for our organisation and the funder # ------------------------------------------------------- @@ -240,7 +280,7 @@ optional_subset: false, published: true} ] -guidance_groups.map{ |gg| GuidanceGroup.create!(gg) if GuidanceGroup.find_by(name: gg[:name]).nil? } +guidance_groups.map{ |gg| create(:guidance_group, gg) } # Initialize with the generic Roadmap guidance and a sample funder guidance # ------------------------------------------------------- @@ -343,7 +383,7 @@ published: true, themes: [Theme.find_by(title: 'Data Description')]} ] -guidances.map{ |g| Guidance.create!(g) if Guidance.find_by(text: g[:text]).nil? } +guidances.map{ |g| create(:guidance, g) } # Create a default template for the curation centre and one for the example funder # ------------------------------------------------------- @@ -375,15 +415,7 @@ ] # Template creation calls defaults handler which sets is_default and # published to false automatically, so update them after creation -templates.map do |t| - if Template.find_by(title: t[:title]).nil? - tmplt = Template.create!(t) - tmplt.published = t[:published] - tmplt.is_default = t[:is_default] - tmplt.visibility = t[:visibility] - tmplt.save! - end -end +templates.each { |atts| create(:template, atts) } # Create 2 phases for the funder's template and one for our generic template # ------------------------------------------------------- @@ -407,11 +439,11 @@ modifiable: false, template: Template.find_by(title: "Department of Testing Award")} ] -phases.map{ |p| Phase.create!(p) if Phase.find_by(title: p[:title]).nil? } +phases.map{ |p| create(:phase, p) } generic_template_phase_1 = Phase.find_by(title: "Generic Data Management Planning Template") -funder_template_phase_1 = Phase.find_by(title: "Preliminary Statement of Work") -funder_template_phase_2 = Phase.find_by(title: "Detailed Overview") +funder_template_phase_1 = Phase.find_by(title: "Preliminary Statement of Work") +funder_template_phase_2 = Phase.find_by(title: "Detailed Overview") # Create sections for the 2 templates and their phases # ------------------------------------------------------- @@ -499,7 +531,7 @@ modifiable: false, phase: funder_template_phase_2} ] -sections.map{ |s| Section.create!(s) if Section.find_by(title: s[:title]).nil? } +sections.map{ |s| create(:section, s) } text_area = QuestionFormat.find_by(title: "Text area") @@ -680,9 +712,9 @@ modifiable: false, themes: [Theme.find_by(title: "Preservation"), Theme.find_by(title: "Data Sharing")]} ] -questions.map{ |q| Question.create!(q) if Question.find_by(text: q[:text]).nil? } +questions.map{ |q| create(:question, q) } -drop_down_question = Question.find_by(text: "Where will you store your data during the research period?") +drop_down_question = Question.find_by(text: "Where will you store your data during the research period?") multi_select_question = Question.find_by(text: "What type(s) of data will you collect?") radio_button_question = Question.find_by(text: "Please select the appropriate formats.") diff --git a/lib/data_cleanup.rb b/lib/data_cleanup.rb new file mode 100644 index 0000000..985768b --- /dev/null +++ b/lib/data_cleanup.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true +module DataCleanup + require_relative "data_cleanup/model_check" + require_relative "data_cleanup/instance_check" + require_relative "data_cleanup/reporting" + require_relative "data_cleanup/rules" + + module_function + + def logger + @logger ||= Logger.new(Rails.root.join("log", "validations.log")) + end + + COLOR_CODES = { red: 31, green: 32 } + + def logger.info(message, inline: false, color: nil) + message = message + "\n" unless inline + if color + message = "\e[#{COLOR_CODES[color]}m#{message}\e[0m" + end + super(message) unless inline + print message + end +end diff --git a/lib/data_cleanup/README.md b/lib/data_cleanup/README.md new file mode 100644 index 0000000..0574bf3 --- /dev/null +++ b/lib/data_cleanup/README.md @@ -0,0 +1,80 @@ +# DataCleanups + +This module is used to clean database records that have become invalid since data validation rules have changed within the app. + +## Rake tasks + +This module adds two rake tasks, one for finding invalid records, and one for fixing them. + +### Usage + +Find invalid records. + +#### Warning: + +This will iterate over every record on your database. It could easily take over an hour to run. + +``` bash +$ rake data_cleanup:find_invalid_records +``` + +Find invalid records for a given list of models. + +``` bash +$ rake data_cleanup:find_invalid_records INCLUDE=Note,User +``` + +...or... + +``` bash +$ rake data_cleanup:find_invalid_records EXCLUDE=Annotation +``` + +--- + +Fix invalid records based on the rules defined in `lib/data_cleanup/rules`. + +#### Warning: + +This will update the records on your database, often using `update_all`. Make sure you: + +- **have a backup of your database** +- **you are comfortable you understand what each of these rules are doing to your data** + +``` bash +$ rake data_cleanup:clean_invalid_records +``` + +Or to clean a given table... + +``` bash +$ rake data_cleanup:clean_invalid_records INCLUDE=Question +``` + +To avoid a given table... + +``` bash +$ rake data_cleanup:clean_invalid_records EXAMPLE=Plan +``` + +## Rules + +Each type of data error is fixed separately. + +These are defined in `lib/data_cleanup/rules`. + +### Creating a new rule + +You can create a new rule by running the following genrator: + +``` bash +$ rails g data_cleanup_rule user/fix_missing_emails +``` + +This will create a file `lib/data_cleanup/rules/user/fix_missing_emails.rb` which contains the rules for updating users with missing emails. + +Feel free to add your own rules where neccesary to fix your own data. + +## Logging output + +Output from the Rake tasks will be logged to `log/validations.log`. diff --git a/lib/data_cleanup/instance_check.rb b/lib/data_cleanup/instance_check.rb new file mode 100644 index 0000000..c7569b8 --- /dev/null +++ b/lib/data_cleanup/instance_check.rb @@ -0,0 +1,23 @@ +require_relative "reporting" + +module DataCleanup + # Check whether a given database record is valid or not + class InstanceCheck + # frozen_string_literal: true + + def call(instance) + Reporting.total_record_count += 1 + begin + if instance.invalid? + Reporting.invalid_record_count += 1 + Reporting.invalid_records << instance + DataCleanup.logger.info("F", inline: true) + else + DataCleanup.logger.info(".", inline: true) + end + rescue Dragonfly::Job::Fetch::NotFound + DataCleanup.logger.info(".", inline: true) + end + end + end +end \ No newline at end of file diff --git a/lib/data_cleanup/model_check.rb b/lib/data_cleanup/model_check.rb new file mode 100644 index 0000000..1b42446 --- /dev/null +++ b/lib/data_cleanup/model_check.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true +module DataCleanup + # Checks whether all records for a given model are valid or not + class ModelCheck + + require_relative "reporting" + + attr_reader :model + + def initialize(model) + @model = model + end + + def call + rule, models = ARGV[1].to_s.split("=") + case rule + when 'INCLUDE' + return unless model.model_name.in?(models.split(",")) + when 'EXCLUDE' + return if model.model_name.in?(models.split(",")) + end + DataCleanup.logger.info "Checking #{model.model_name.plural}:" + model.find_in_batches do |batch| + instance_check = InstanceCheck.new + batch.each { |instance| instance_check.(instance) } + end + DataCleanup.logger.info "" + end + end +end diff --git a/lib/data_cleanup/reporting.rb b/lib/data_cleanup/reporting.rb new file mode 100644 index 0000000..ebeba3b --- /dev/null +++ b/lib/data_cleanup/reporting.rb @@ -0,0 +1,34 @@ +module DataCleanup + # Report the status of the data validations after checks have been run. + module Reporting + mattr_accessor :total_record_count + mattr_accessor :invalid_record_count + mattr_accessor :invalid_records + mattr_accessor :issues_found + + self.total_record_count = 0 + self.invalid_record_count = 0 + self.invalid_records = [] + self.issues_found = [] + + module_function + + # Prepare the report for printing to log and STDOUT + def prepare! + invalid_records.each do |record| + record.errors.full_messages.each do |issue| + desc = "#{record.class.model_name} was invalid: #{issue}" + issues_found << desc unless issues_found.include?(desc) + end + end + end + + def report + issues_found.each { |issue| DataCleanup.logger.info issue } + color = invalid_record_count.zero? ? :green : :red + DataCleanup.logger.info(<<~TEXT, color: color) + Invalid records: #{invalid_record_count} / #{total_record_count} + TEXT + end + end +end diff --git a/lib/data_cleanup/rules.rb b/lib/data_cleanup/rules.rb new file mode 100644 index 0000000..155b86b --- /dev/null +++ b/lib/data_cleanup/rules.rb @@ -0,0 +1,7 @@ +require_relative "rules/base" + +module DataCleanup + module Rules + + end +end diff --git a/lib/data_cleanup/rules/answer/fix_blank_user.rb b/lib/data_cleanup/rules/answer/fix_blank_user.rb new file mode 100644 index 0000000..987834a --- /dev/null +++ b/lib/data_cleanup/rules/answer/fix_blank_user.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix blank user on Answer + module Answer + class FixBlankUser < Rules::Base + + def description + "Fix blank user on Answer" + end + + def call + ::Answer.where(user: nil).destroy_all + end + end + end + end +end diff --git a/lib/data_cleanup/rules/answer/fix_duplicate_question.rb b/lib/data_cleanup/rules/answer/fix_duplicate_question.rb new file mode 100644 index 0000000..300d4a5 --- /dev/null +++ b/lib/data_cleanup/rules/answer/fix_duplicate_question.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix duplicate question on Answer + module Answer + class FixDuplicateQuestion < Rules::Base + + def description + "Fix duplicate question on Answer" + end + + def call + # Take all answers that have duplicate question/plan combo... + dataset = ::Answer.group(:question_id, :plan_id) + .count.select { |k,v| v > 1 } + # Values looks like [{ [123, 199] => 2}, ...] + dataset.each do |values, count| + # ... and destroy all duplicates, keeping the latest record + ::Answer.where(question: values.first, plan_id: values.last) + .order("created_at DESC") + .offset(1) + .destroy_all + end + end + end + end + end +end diff --git a/lib/data_cleanup/rules/base.rb b/lib/data_cleanup/rules/base.rb new file mode 100644 index 0000000..cff2d1d --- /dev/null +++ b/lib/data_cleanup/rules/base.rb @@ -0,0 +1,17 @@ +module DataCleanup + module Rules + # Base class for rules to clean invalid database records + class Base + + # Description of the rule and how it's fixing the data + def description + self.class.name.humanize + end + + # Run this rule and fix data in the database. + def call + raise NotImplementedError, "Please define call() in #{self}" + end + end + end +end diff --git a/lib/data_cleanup/rules/exported_plan/fix_blank_plan.rb b/lib/data_cleanup/rules/exported_plan/fix_blank_plan.rb new file mode 100644 index 0000000..90d0363 --- /dev/null +++ b/lib/data_cleanup/rules/exported_plan/fix_blank_plan.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module DataCleanup + module Rules + # Fix blank plan on ExportedPlan + module ExportedPlan + class FixBlankPlan < Rules::Base + + def description + "Fix blank plan on ExportedPlan" + end + + def call + ::ExportedPlan.where(plan: nil).delete_all + end + end + end + end +end diff --git a/lib/data_cleanup/rules/guidance_group/fix_blank_optional_subset.rb b/lib/data_cleanup/rules/guidance_group/fix_blank_optional_subset.rb new file mode 100644 index 0000000..9cad432 --- /dev/null +++ b/lib/data_cleanup/rules/guidance_group/fix_blank_optional_subset.rb @@ -0,0 +1,17 @@ +module DataCleanup + module Rules + module GuidanceGroup + class FixBlankOptionalSubset < Rules::Base + + def description + "Fix blank optional subset on GuidanceGroup" + end + + def call + ::GuidanceGroup.where(optional_subset: nil) + .update_all(optional_subset: false) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/guidance_group/fix_blank_published.rb b/lib/data_cleanup/rules/guidance_group/fix_blank_published.rb new file mode 100644 index 0000000..f11b95b --- /dev/null +++ b/lib/data_cleanup/rules/guidance_group/fix_blank_published.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + module GuidanceGroup + # Fix blank published on GuidanceGroup + class FixBlankPublished < Rules::Base + + def description + "Fix blank published on GuidanceGroup" + end + + def call + ::GuidanceGroup.where(published: nil).update_all(published: false) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/note/fix_blank_archived.rb b/lib/data_cleanup/rules/note/fix_blank_archived.rb new file mode 100644 index 0000000..0f12339 --- /dev/null +++ b/lib/data_cleanup/rules/note/fix_blank_archived.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix blank archived on Note + module Note + class FixBlankArchived < Rules::Base + + def description + "Fix blank archived on Note" + end + + def call + ::Note.where(archived: nil).update_all(archived: false) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/org/fix_blank_abbreviation.example.yml b/lib/data_cleanup/rules/org/fix_blank_abbreviation.example.yml new file mode 100644 index 0000000..1133989 --- /dev/null +++ b/lib/data_cleanup/rules/org/fix_blank_abbreviation.example.yml @@ -0,0 +1,9 @@ +- + id: 1234 + name: University of Edinburgh + abbreviation: UoE +- + id: 1235 + name: De Montfort University + abbreviation: "DMU" +# ... \ No newline at end of file diff --git a/lib/data_cleanup/rules/org/fix_blank_abbreviation.rb b/lib/data_cleanup/rules/org/fix_blank_abbreviation.rb new file mode 100644 index 0000000..40354b6 --- /dev/null +++ b/lib/data_cleanup/rules/org/fix_blank_abbreviation.rb @@ -0,0 +1,27 @@ +module DataCleanup + module Rules + module Org + class FixBlankAbbreviation < Rules::Base + + YAML_FILE_PATH = Rails.root.join("lib", "data_cleanup", "rules", "org", + "fix_blank_abbreviation.yml") + + def description + "Fix blank abbreviation on Org" + end + + def call + if File.exists?(YAML_FILE_PATH) + YAML.load_file(YAML_FILE_PATH).each do |attributes| + attributes = attributes.with_indifferent_access + ::Org.where(name: attributes["name"], id: attributes['id']) + .update_all(abbreviation: attributes['abbreviation']) + end + else + raise "Please create a YAML file at #{YAML_FILE_PATH}" + end + end + end + end + end +end diff --git a/lib/data_cleanup/rules/org/fix_blank_feedback_email_msg.rb b/lib/data_cleanup/rules/org/fix_blank_feedback_email_msg.rb new file mode 100644 index 0000000..cefdc38 --- /dev/null +++ b/lib/data_cleanup/rules/org/fix_blank_feedback_email_msg.rb @@ -0,0 +1,26 @@ +module DataCleanup + module Rules + module Org + class FixBlankFeedbackEmailMsg < Rules::Base + + DEFAULT_MSG = <<~HTML +<p>Hello %{user_name}.</p> +<p> + Your plan "%{plan_name}" has been submitted for feedback from an administrator + at your organisation. If you have questions + pertaining to this action, please contact us at %{organisation_email}. +</p> + HTML + + def description + "Fix orgs where feedback_enabled is true" + end + + def call + ::Org.where(feedback_enabled: true, feedback_email_msg: "") + .update_all(feedback_email_msg: DEFAULT_MSG) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/org/fix_blank_feedback_email_subject.rb b/lib/data_cleanup/rules/org/fix_blank_feedback_email_subject.rb new file mode 100644 index 0000000..e190b22 --- /dev/null +++ b/lib/data_cleanup/rules/org/fix_blank_feedback_email_subject.rb @@ -0,0 +1,19 @@ +module DataCleanup + module Rules + module Org + class FixBlankFeedbackEmailSubject < Rules::Base + + DEFAULT_SUBJECT = "%{application_name}: Your plan has been submitted for feedback" + + def description + "Fix orgs where feedback_enabled is true" + end + + def call + ::Org.where(feedback_enabled: true, feedback_email_subject: "") + .update_all(feedback_email_subject: DEFAULT_SUBJECT) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/org/fix_blank_is_other.rb b/lib/data_cleanup/rules/org/fix_blank_is_other.rb new file mode 100644 index 0000000..b255720 --- /dev/null +++ b/lib/data_cleanup/rules/org/fix_blank_is_other.rb @@ -0,0 +1,16 @@ +module DataCleanup + module Rules + module Org + class FixBlankIsOther < Rules::Base + + def description + "Fix nil values for is_other on Org" + end + + def call + ::Org.where(is_other: nil).update_all(is_other: false) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/org/fix_blank_language.rb b/lib/data_cleanup/rules/org/fix_blank_language.rb new file mode 100644 index 0000000..14ba0ff --- /dev/null +++ b/lib/data_cleanup/rules/org/fix_blank_language.rb @@ -0,0 +1,18 @@ +module DataCleanup + module Rules + module Org + class FixBlankLanguage < Rules::Base + + DEFAULT_LANGUAGE = Language.find_by(abbreviation: FastGettext.default_locale) + + def description + "Fix blank language on Org" + end + + def call + ::Org.where(language: nil).update_all(language_id: DEFAULT_LANGUAGE.id) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/phase/fix_duplicate_number.rb b/lib/data_cleanup/rules/phase/fix_duplicate_number.rb new file mode 100644 index 0000000..e733dcd --- /dev/null +++ b/lib/data_cleanup/rules/phase/fix_duplicate_number.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix duplicate number on Phase + module Phase + class FixDuplicateNumber < Rules::Base + + def description + "Fix duplicate number on Phase" + end + + def call + data = ::Phase.group(:number, :template_id) + .count + .select { |k,v| v > 1 } + data.each do |values, count| + number, template_id = *values + ids = ::Phase.where(template_id: template_id) + .order("number ASC, created_at ASC") + .pluck(:id) + template = ::Template.find(template_id) + ::Phase.update_numbers!(*ids, parent: template) + end + end + end + end + end +end diff --git a/lib/data_cleanup/rules/plan/fix_blank_title.rb b/lib/data_cleanup/rules/plan/fix_blank_title.rb new file mode 100644 index 0000000..8dc87a9 --- /dev/null +++ b/lib/data_cleanup/rules/plan/fix_blank_title.rb @@ -0,0 +1,18 @@ +module DataCleanup + module Rules + module Plan + class FixBlankTitle < Rules::Base + + def description + "Fix blank title on Plan" + end + + def call + ::Plan.where(title: [nil, '']).each do |plan| + plan.update(title: "My plan (#{plan.template.title})") + end + end + end + end + end +end diff --git a/lib/data_cleanup/rules/question/fix_duplicate_number.rb b/lib/data_cleanup/rules/question/fix_duplicate_number.rb new file mode 100644 index 0000000..083b8fb --- /dev/null +++ b/lib/data_cleanup/rules/question/fix_duplicate_number.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix duplicate number on Question + module Question + class FixDuplicateNumber < Rules::Base + + def description + "Fix duplicate number on Question" + end + + def call + data = ::Question.group(:number, :section_id) + .count + .select { |k,v| v > 1 } + data.each do |values, count| + number, section_id = *values + ids = ::Question.where(section_id: section_id) + .order("number ASC, created_at ASC") + .pluck(:id) + section = ::Section.find(section_id) + ::Question.update_numbers!(*ids, parent: section) + end + end + end + end + end +end diff --git a/lib/data_cleanup/rules/question_format/fix_blank_description.rb b/lib/data_cleanup/rules/question_format/fix_blank_description.rb new file mode 100644 index 0000000..7c8f06c --- /dev/null +++ b/lib/data_cleanup/rules/question_format/fix_blank_description.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix blank description on QuestionFormat + module QuestionFormat + class FixBlankDescription < Rules::Base + + def description + "Fix blank description on QuestionFormat" + end + + def call + ::QuestionFormat.where(description: "").each do |qf| + qf.update!(description: "#{qf.title} format") + end + end + end + end + end +end diff --git a/lib/data_cleanup/rules/region/fix_blank_description.rb b/lib/data_cleanup/rules/region/fix_blank_description.rb new file mode 100644 index 0000000..1ddee40 --- /dev/null +++ b/lib/data_cleanup/rules/region/fix_blank_description.rb @@ -0,0 +1,18 @@ +module DataCleanup + module Rules + module Region + class FixBlankDescription < Rules::Base + + def description + "Fix blank description on region" + end + + def call + ::Region.where(description: [nil, '']).each do |region| + region.update!(description: "#{region.name} region") + end + end + end + end + end +end diff --git a/lib/data_cleanup/rules/role/fix_blank_plan.rb b/lib/data_cleanup/rules/role/fix_blank_plan.rb new file mode 100644 index 0000000..3e4219b --- /dev/null +++ b/lib/data_cleanup/rules/role/fix_blank_plan.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix blank plan on Role + module Role + class FixBlankPlan < Rules::Base + + def description + "Fix blank plan on Role" + end + + def call + ::Role.where(plan: nil).destroy_all + end + end + end + end +end diff --git a/lib/data_cleanup/rules/section/fix_duplicate_number.rb b/lib/data_cleanup/rules/section/fix_duplicate_number.rb new file mode 100644 index 0000000..c2c41c8 --- /dev/null +++ b/lib/data_cleanup/rules/section/fix_duplicate_number.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix duplicate number on Section + module Section + class FixDuplicateNumber < Rules::Base + + def description + "Fix duplicate number on Section" + end + + def call + data = ::Section.group(:number, :phase_id) + .count + .select { |k,v| v > 1 } + data.each do |values, count| + number, phase_id = *values + ids = ::Section.where(phase_id: phase_id) + .order("number ASC, created_at ASC") + .pluck(:id) + phase = ::Phase.find(phase_id) + ::Section.update_numbers!(*ids, parent: phase) + end + end + end + end + end +end diff --git a/lib/data_cleanup/rules/section/fix_nil_published.rb b/lib/data_cleanup/rules/section/fix_nil_published.rb new file mode 100644 index 0000000..cb75525 --- /dev/null +++ b/lib/data_cleanup/rules/section/fix_nil_published.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix nil published on Section + module Section + class FixNilPublished < Rules::Base + + def description + "Fix nil published on Section" + end + + def call + ::Section.where(published: nil).update_all(published: false) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/template/fix_blank_locale.rb b/lib/data_cleanup/rules/template/fix_blank_locale.rb new file mode 100644 index 0000000..4fdd5e7 --- /dev/null +++ b/lib/data_cleanup/rules/template/fix_blank_locale.rb @@ -0,0 +1,17 @@ +module DataCleanup + module Rules + module Template + class FixBlankLocale < Rules::Base + + def description + "Fix blank locale on template" + end + + def call + ::Template.where(locale: nil) + .update_all(locale: FastGettext.default_locale) + end + end + end + end +end diff --git a/lib/data_cleanup/rules/user_identifier/fix_blank_user.rb b/lib/data_cleanup/rules/user_identifier/fix_blank_user.rb new file mode 100644 index 0000000..9676652 --- /dev/null +++ b/lib/data_cleanup/rules/user_identifier/fix_blank_user.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # Fix blank user on UserIdentifier + module UserIdentifier + class FixBlankUser < Rules::Base + + def description + "Fix UserIdentifier records with no User" + end + + def call + ::UserIdentifier.where(user: nil).destroy_all + end + end + end + end +end diff --git a/lib/generators/data_cleanup_rule/USAGE b/lib/generators/data_cleanup_rule/USAGE new file mode 100644 index 0000000..7264472 --- /dev/null +++ b/lib/generators/data_cleanup_rule/USAGE @@ -0,0 +1,8 @@ +Description: + Add a new rule for cleaning invalid database records + +Example: + rails generate data_cleanup_rule user/fix_missing_emails + + This will create: + lib/tasks/data_cleanup/rules/user/fix_missing_emails.rb diff --git a/lib/generators/data_cleanup_rule/data_cleanup_rule_generator.rb b/lib/generators/data_cleanup_rule/data_cleanup_rule_generator.rb new file mode 100644 index 0000000..6e48031 --- /dev/null +++ b/lib/generators/data_cleanup_rule/data_cleanup_rule_generator.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +# Generator class for creating a new Rule to clean DB records. +class DataCleanupRuleGenerator < Rails::Generators::NamedBase + source_root File.expand_path('templates', __dir__) + + # Copy the Rule template and create a new Rule file + def add_rule_file + template "rule.rb.erb", rule_path.to_s + end + + private + + # The name of the model we're fixing (e.g. 'User') + # + # Returns String + def model_name + file_path.split("/").first.classify + end + + # The name of the Rule class we're creating (e.g. 'User::FixBlankEmail') + # + # Returns String + def rule_class_name + file_path.split("/").last.classify + end + + # The file path for the new Rule class + # + # Returns String + def rule_path + Rails.root.join("lib", "data_cleanup", "rules", "#{file_path}.rb") + end + + # A default description to populate the Rule#description method + # + # Returns String + def default_description + format("%<rule_name>s on %<model_name>s", + rule_name: rule_class_name.underscore + .split("_") + .join(" ") + .capitalize, + model_name: model_name.classify) + end +end diff --git a/lib/generators/data_cleanup_rule/templates/rule.rb.erb b/lib/generators/data_cleanup_rule/templates/rule.rb.erb new file mode 100644 index 0000000..bad37d1 --- /dev/null +++ b/lib/generators/data_cleanup_rule/templates/rule.rb.erb @@ -0,0 +1,18 @@ +# frozen_string_literal: true +module DataCleanup + module Rules + # <%= default_description %> + module <%= model_name %> + class <%= rule_class_name %> < Rules::Base + + def description + "<%= default_description %>" + end + + def call + ::<%= model_name %>.where(nil).update_all({}) + end + end + end + end +end diff --git a/lib/tasks/heal_invalid_records.rake b/lib/tasks/heal_invalid_records.rake new file mode 100644 index 0000000..18a017f --- /dev/null +++ b/lib/tasks/heal_invalid_records.rake @@ -0,0 +1,47 @@ +require "data_cleanup" + +namespace :data_cleanup do + + desc "Check each record on the DB is valid and report" + task :find_invalid_records => :environment do + models.each do |model| + DataCleanup::ModelCheck.new(model).call + end + DataCleanup::Reporting.prepare! + DataCleanup::Reporting.report + end + + desc "Clean invalid records on the database" + task :clean_invalid_records => :environment do + Dir[rule_paths].each do |rule_path| + load rule_path + klass_name = rule_path.split("rules/").last.gsub(".rb", '').classify + model_name = klass_name.split("::").first + opt, models = ARGV[1].to_s.split("=") + case opt + when 'INCLUDE' + next unless model_name.in?(models.split(",")) + when 'EXCLUDE' + next if model_name.in?(models.split(",")) + else + raise ArgumentError, "Unknown option: #{opt}" + end + rule_class = DataCleanup::Rules.const_get(klass_name) + rule = rule_class.new + puts rule.description + rule.call + end + end + + private + + def rule_paths + @rule_paths ||= Rails.root.join("lib", "data_cleanup", "rules", "*", "*.rb") + end + + def models + Dir[Rails.root.join("app", "models", "*.rb")].map do |model_path| + model_path.split("/").last.gsub(".rb", "").classify.constantize + end.sort_by(&:name) + end +end diff --git a/lib/tasks/upgrade.rake b/lib/tasks/upgrade.rake index e4e86fe..8a10e0c 100644 --- a/lib/tasks/upgrade.rake +++ b/lib/tasks/upgrade.rake @@ -5,7 +5,7 @@ task clean_section_numbers: :environment do Phase.all.each do |phase| ids_in_order = phase.sections.order("number, created_at").pluck(:id) - Section.update_numbers!(*ids_in_order, phase: phase) + Section.update_numbers!(*ids_in_order, parent: phase) end end diff --git a/spec/factories/annotations.rb b/spec/factories/annotations.rb index 04cef9d..fc8562c 100644 --- a/spec/factories/annotations.rb +++ b/spec/factories/annotations.rb @@ -16,5 +16,6 @@ question org text { Faker::Lorem.paragraph } + type { [0,1].sample } end end diff --git a/spec/factories/guidance_groups.rb b/spec/factories/guidance_groups.rb index 3e7ae69..f7e2b49 100644 --- a/spec/factories/guidance_groups.rb +++ b/spec/factories/guidance_groups.rb @@ -16,6 +16,7 @@ name { Faker::Lorem.unique.word } org published true + optional_subset false trait :unpublished do published false end diff --git a/spec/factories/guidances.rb b/spec/factories/guidances.rb index b4863f4..dbc6929 100644 --- a/spec/factories/guidances.rb +++ b/spec/factories/guidances.rb @@ -15,6 +15,6 @@ factory :guidance do text { Faker::Lorem.sentence } guidance_group - question_id { create(:question).id } + question end end diff --git a/spec/factories/identifier_schemes.rb b/spec/factories/identifier_schemes.rb index 11bacdf..34e2def 100644 --- a/spec/factories/identifier_schemes.rb +++ b/spec/factories/identifier_schemes.rb @@ -16,5 +16,8 @@ factory :identifier_scheme do name { Faker::Company.unique.name } description { Faker::StarWars.quote } + logo_url { Faker::Internet.url } + user_landing_url { Faker::Internet.url } + active true end end diff --git a/spec/factories/languages.rb b/spec/factories/languages.rb index 1eb5d11..3d227e5 100644 --- a/spec/factories/languages.rb +++ b/spec/factories/languages.rb @@ -13,7 +13,14 @@ factory :language do name "English" description "Test language English" - abbreviation { SecureRandom.hex(2) } + abbreviation { ("a".."z").to_a.shuffle.take(2).join } default_language true + trait :with_dialect do + abbreviation { + pre = ("a".."z").to_a.shuffle.take(2).join + suf = ("A".."Z").to_a.shuffle.take(2).join + [pre, suf].join("_") + } + end end end diff --git a/spec/factories/orgs.rb b/spec/factories/orgs.rb index 8c5eead..cfdcb3e 100644 --- a/spec/factories/orgs.rb +++ b/spec/factories/orgs.rb @@ -29,8 +29,12 @@ FactoryBot.define do factory :org do - sequence(:name) { |i| Faker::Company.unique.name + i.to_s } + name { Faker::Company.unique.name } links { { "org" => [] } } - abbreviation { SecureRandom.hex(2) } + abbreviation { self.name[0..10] } + feedback_enabled false + region { Region.first || create(:region) } + language { Language.first || create(:language) } + is_other false end end diff --git a/spec/factories/phases.rb b/spec/factories/phases.rb index 3adb7b4..8952830 100644 --- a/spec/factories/phases.rb +++ b/spec/factories/phases.rb @@ -19,5 +19,6 @@ description { Faker::Lorem.paragraph } sequence(:number) template + modifiable true end end diff --git a/spec/factories/plans.rb b/spec/factories/plans.rb index ee90ea9..ccfb538 100644 --- a/spec/factories/plans.rb +++ b/spec/factories/plans.rb @@ -37,5 +37,8 @@ principal_investigator_email { Faker::Internet.safe_email } feedback_requested false complete false + trait :creator do + after(:create) { |obj| obj.roles << create(:role, creator: true) } + end end end diff --git a/spec/factories/question_formats.rb b/spec/factories/question_formats.rb index 0336584..a79929c 100644 --- a/spec/factories/question_formats.rb +++ b/spec/factories/question_formats.rb @@ -15,5 +15,6 @@ factory :question_format do title { Faker::Lorem.words(3).join } description { Faker::Lorem.sentence } + formattype { QuestionFormat::FORMAT_TYPES.sample } end end diff --git a/spec/factories/templates.rb b/spec/factories/templates.rb index 539076e..9014287 100644 --- a/spec/factories/templates.rb +++ b/spec/factories/templates.rb @@ -24,5 +24,6 @@ org title { Faker::Lorem.sentence } description { Faker::Lorem.paragraph } + locale { "en_GB" } end end diff --git a/spec/factories/themes.rb b/spec/factories/themes.rb index a3c619d..9dc836d 100644 --- a/spec/factories/themes.rb +++ b/spec/factories/themes.rb @@ -14,5 +14,6 @@ factory :theme do title { Faker::Lorem.sentence } description { Faker::Lorem.paragraph } + locale "en_GB" end end diff --git a/spec/models/annotation_spec.rb b/spec/models/annotation_spec.rb new file mode 100644 index 0000000..cd62942 --- /dev/null +++ b/spec/models/annotation_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe Annotation, type: :model do + + context "validations" do + + subject { build(:annotation) } + + it { is_expected.to validate_presence_of(:text) } + + it { is_expected.to validate_presence_of(:org) } + + it { is_expected.to validate_presence_of(:question) } + + it { is_expected.to validate_presence_of(:type) } + + end + +end diff --git a/spec/models/answer_spec.rb b/spec/models/answer_spec.rb new file mode 100644 index 0000000..6761242 --- /dev/null +++ b/spec/models/answer_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe Answer, type: :model do + + context "validations" do + subject { build(:answer) } + + it { is_expected.to validate_presence_of(:plan) } + + it { is_expected.to validate_presence_of(:user) } + + it { is_expected.to validate_presence_of(:question) } + + it { is_expected.to validate_uniqueness_of(:question) + .scoped_to(:plan_id) + .with_message("must be unique") } + end + +end diff --git a/spec/models/guidance_group_spec.rb b/spec/models/guidance_group_spec.rb new file mode 100644 index 0000000..823a0c9 --- /dev/null +++ b/spec/models/guidance_group_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +RSpec.describe GuidanceGroup, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:name) } + + it { is_expected.to validate_uniqueness_of(:name) + .with_message('must be unique') + .scoped_to(:org_id) } + + it { is_expected.to validate_presence_of(:org) } + + it { is_expected.to allow_value(true).for(:optional_subset) } + + it { is_expected.to allow_value(true).for(:published) } + + it { is_expected.to allow_value(false).for(:optional_subset) } + + it { is_expected.to allow_value(false).for(:published) } + + end + +end diff --git a/spec/models/guidance_spec.rb b/spec/models/guidance_spec.rb new file mode 100644 index 0000000..20cc146 --- /dev/null +++ b/spec/models/guidance_spec.rb @@ -0,0 +1,17 @@ +require 'rails_helper' + +RSpec.describe Guidance, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:text) } + + it { is_expected.to validate_presence_of(:guidance_group) } + + it { is_expected.to allow_value(true).for(:published) } + + it { is_expected.to allow_value(false).for(:published) } + + end + +end diff --git a/spec/models/identifier_scheme_spec.rb b/spec/models/identifier_scheme_spec.rb new file mode 100644 index 0000000..2131076 --- /dev/null +++ b/spec/models/identifier_scheme_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe IdentifierScheme, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:name) } + + it { is_expected.to validate_length_of(:name).is_at_most(30) } + + it { is_expected.to allow_value(true).for(:name) } + + it { is_expected.to allow_value(false).for(:name) } + + it { is_expected.to_not allow_value(nil).for(:name) } + + end + +end diff --git a/spec/models/language_spec.rb b/spec/models/language_spec.rb new file mode 100644 index 0000000..225b3fb --- /dev/null +++ b/spec/models/language_spec.rb @@ -0,0 +1,28 @@ +require 'rails_helper' + +RSpec.describe Language, type: :model do + + context "validations" do + + subject { build(:language) } + + it { is_expected.to validate_presence_of(:name) } + + it { is_expected.to validate_length_of(:name).is_at_most(20) } + + it { is_expected.to validate_presence_of(:abbreviation) } + + it { is_expected.to validate_uniqueness_of(:abbreviation) + .with_message("must be unique") } + + it { is_expected.to allow_values('en', 'en_GB').for(:abbreviation) } + + it { is_expected.not_to allow_value('NOOP', 'en_', 'EN') + .for(:abbreviation) } + + it { is_expected.to validate_length_of(:abbreviation).is_at_most(5) } + + + end + +end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb new file mode 100644 index 0000000..03f7627 --- /dev/null +++ b/spec/models/note_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe Note, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:text) } + + it { is_expected.to validate_presence_of(:answer) } + + it { is_expected.to validate_presence_of(:user) } + + it { is_expected.to allow_values(true, false).for(:archived) } + + it { is_expected.not_to allow_value(nil).for(:archived) } + + end + +end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb new file mode 100644 index 0000000..be310a3 --- /dev/null +++ b/spec/models/notification_spec.rb @@ -0,0 +1,33 @@ +require 'rails_helper' + +RSpec.describe Notification, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:notification_type) } + + it { is_expected.to validate_presence_of(:title) } + + it { is_expected.to validate_presence_of(:level) } + + it { is_expected.to validate_presence_of(:body) } + + it { is_expected.to allow_values(true, false).for(:dismissable) } + + it { is_expected.not_to allow_value(nil).for(:dismissable) } + + it { is_expected.to validate_presence_of(:starts_at) } + + it { is_expected.to validate_presence_of(:expires_at) } + + it { is_expected.to allow_value(Date.today).for(:starts_at) } + + it { is_expected.not_to allow_value(1.day.ago).for(:starts_at) } + + it { is_expected.to allow_value(2.days.from_now).for(:expires_at) } + + it { is_expected.not_to allow_value(Date.today).for(:expires_at) } + + end + +end diff --git a/spec/models/org_identifier_spec.rb b/spec/models/org_identifier_spec.rb new file mode 100644 index 0000000..98d58d1 --- /dev/null +++ b/spec/models/org_identifier_spec.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +RSpec.describe OrgIdentifier, type: :model do + + context "validations" do + + it do + # https://github.com/thoughtbot/shoulda-matchers/issues/682 + subject.identifier_scheme = create(:identifier_scheme) + is_expected.to validate_uniqueness_of(:identifier_scheme_id) + .scoped_to(:org_id) + .with_message("must be unique") + end + + end + + it { is_expected.to validate_presence_of(:identifier) } + + it { is_expected.to validate_presence_of(:org) } + + it { is_expected.to validate_presence_of(:identifier_scheme) } + +end diff --git a/spec/models/org_spec.rb b/spec/models/org_spec.rb new file mode 100644 index 0000000..3209373 --- /dev/null +++ b/spec/models/org_spec.rb @@ -0,0 +1,49 @@ +require 'rails_helper' + +RSpec.describe Org, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:name) } + + it { + subject.name = "DMP Company" + is_expected.to validate_uniqueness_of(:name) + .with_message("must be unique") + } + + it { is_expected.to validate_presence_of(:abbreviation) } + + it { is_expected.to allow_values(true, false).for(:is_other) } + + it { is_expected.not_to allow_value(nil).for(:is_other) } + + it { is_expected.to validate_presence_of(:language) } + + it "validates presence of contact_email if feedback_enabled" do + subject.feedback_enabled = true + is_expected.to validate_presence_of(:contact_email) + end + + it "doesn't validate presence of contact_email if feedback_enabled nil" do + subject.feedback_enabled = false + is_expected.not_to validate_presence_of(:contact_email) + end + + # validates :contact_email, presence: { message: PRESENCE_MESSAGE, + # if: :feedback_enabled } + # + # validates :org_type, presence: { message: PRESENCE_MESSAGE } + # + # validates :feedback_enabled, inclusion: { in: BOOLEAN_VALUES, + # message: INCLUSION_MESSAGE } + # + # validates :feedback_email_subject, presence: { message: PRESENCE_MESSAGE, + # if: :feedback_enabled } + # + # validates :feedback_email_msg, presence: { message: PRESENCE_MESSAGE, + # if: :feedback_enabled } + # + end + +end diff --git a/spec/models/perm_spec.rb b/spec/models/perm_spec.rb new file mode 100644 index 0000000..5a5e780 --- /dev/null +++ b/spec/models/perm_spec.rb @@ -0,0 +1,11 @@ +require 'rails_helper' + +RSpec.describe Perm, type: :model do + context "validations" do + + it { is_expected.to validate_presence_of(:name) } + + it { is_expected.to validate_uniqueness_of(:name) + .with_message("must be unique") } + end +end diff --git a/spec/models/phase_spec.rb b/spec/models/phase_spec.rb new file mode 100644 index 0000000..fbcaeaa --- /dev/null +++ b/spec/models/phase_spec.rb @@ -0,0 +1,20 @@ +require 'rails_helper' + +RSpec.describe Phase, type: :model do + context "validations" do + it { is_expected.to validate_presence_of(:title) } + + it { is_expected.to validate_presence_of(:number) } + + it { is_expected.to validate_presence_of(:template) } + + it { is_expected.to validate_uniqueness_of(:number) + .scoped_to(:template_id) + .with_message("must be unique") } + + it { is_expected.to allow_values(true, false).for(:modifiable) } + + it { is_expected.not_to allow_value(nil).for(:modifiable) } + + end +end diff --git a/spec/models/plan_spec.rb b/spec/models/plan_spec.rb index 9b4e919..3c03568 100644 --- a/spec/models/plan_spec.rb +++ b/spec/models/plan_spec.rb @@ -2,9 +2,18 @@ describe Plan do - describe "test" do - subject { true } - it { is_expected.to be_truthy } + context "validations" do + it { is_expected.to validate_presence_of(:title) } + + it { is_expected.to validate_presence_of(:template) } + + it { is_expected.to allow_values(true, false).for(:feedback_requested) } + + it { is_expected.not_to allow_value(nil).for(:feedback_requested) } + + it { is_expected.to allow_values(true, false).for(:complete) } + + it { is_expected.not_to allow_value(nil).for(:complete) } end end diff --git a/spec/models/pref_spec.rb b/spec/models/pref_spec.rb new file mode 100644 index 0000000..1cd289a --- /dev/null +++ b/spec/models/pref_spec.rb @@ -0,0 +1,8 @@ +require 'rails_helper' + +RSpec.describe Pref, type: :model do + context "validations" do + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_presence_of(:settings) } + end +end diff --git a/spec/models/question_format_spec.rb b/spec/models/question_format_spec.rb new file mode 100644 index 0000000..ebc66f3 --- /dev/null +++ b/spec/models/question_format_spec.rb @@ -0,0 +1,31 @@ +require 'rails_helper' + +RSpec.describe QuestionFormat, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:title) } + + it { is_expected.to validate_uniqueness_of(:title) + .with_message("must be unique") } + + it { is_expected.to allow_values(true, false).for(:option_based) } + + it { is_expected.not_to allow_value(nil).for(:option_based) } + + it { is_expected.to allow_values(:textarea, :textfield, :radiobuttons, + :checkbox, :dropdown, :multiselectbox, + :date, :rda_metadata) + .for(:formattype) } + + end + + describe "#formattype" do + + it "raises an exception when value not recognised" do + expect { subject.formattype = :foo }.to raise_error(ArgumentError) + end + + end + +end diff --git a/spec/models/question_option_spec.rb b/spec/models/question_option_spec.rb new file mode 100644 index 0000000..90f2635 --- /dev/null +++ b/spec/models/question_option_spec.rb @@ -0,0 +1,18 @@ +require 'rails_helper' + +RSpec.describe QuestionOption, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:question) } + + it { is_expected.to validate_presence_of(:text) } + + it { is_expected.to validate_presence_of(:number) } + + it { is_expected.to allow_values(true, false).for(:is_default) } + + it { is_expected.not_to allow_value(nil).for(:is_default) } + + end +end diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb new file mode 100644 index 0000000..77f4560 --- /dev/null +++ b/spec/models/question_spec.rb @@ -0,0 +1,30 @@ +require 'rails_helper' + +RSpec.describe Question, type: :model do + + context "validations" do + + it { is_expected.to validate_presence_of(:text) } + + it { is_expected.to validate_presence_of(:number) } + + it { is_expected.to validate_uniqueness_of(:number) + .scoped_to(:section_id) + .with_message("must be unique") } + + it { is_expected.to validate_presence_of(:section) } + + + it { is_expected.to validate_presence_of(:question_format) } + + it { is_expected.to allow_values(true, false).for(:option_comment_display) } + + it { is_expected.to allow_value(nil).for(:option_comment_display) } + + it { is_expected.to allow_values(true, false).for(:modifiable) } + + it { is_expected.to allow_value(nil).for(:modifiable) } + + end + +end diff --git a/spec/models/region_spec.rb b/spec/models/region_spec.rb new file mode 100644 index 0000000..b9b6b84 --- /dev/null +++ b/spec/models/region_spec.rb @@ -0,0 +1,14 @@ +require 'rails_helper' + +RSpec.describe Region, type: :model do + context "validations" do + it { is_expected.to validate_presence_of(:abbreviation) } + + it { is_expected.to validate_uniqueness_of(:abbreviation) + .with_message("must be unique") } + + it { is_expected.to validate_presence_of(:description) } + + it { is_expected.to validate_presence_of(:name) } + end +end diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb new file mode 100644 index 0000000..7bd2f42 --- /dev/null +++ b/spec/models/role_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec.describe Role, type: :model do + + context "validations" do + it { is_expected.to validate_presence_of(:user) } + + it { is_expected.to validate_presence_of(:plan) } + + it { is_expected.to allow_values(true, false).for(:active) } + + it { is_expected.not_to allow_value(nil).for(:active) } + + it { is_expected.to validate_numericality_of(:access) + .only_integer + .is_greater_than(0) + .with_message("can't be less than zero") } + + end + +end diff --git a/spec/models/section_spec.rb b/spec/models/section_spec.rb new file mode 100644 index 0000000..9be4d0f --- /dev/null +++ b/spec/models/section_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' + +RSpec.describe Section, type: :model do + context "validations" do + it { is_expected.to validate_presence_of(:title) } + + it { is_expected.to validate_presence_of(:number) } + + it { is_expected.to validate_presence_of(:phase) } + + it { is_expected.to validate_uniqueness_of(:number) + .scoped_to(:phase_id) + .with_message("must be unique") } + + it { is_expected.to allow_values(true, false).for(:published) } + + it { is_expected.not_to allow_value(nil).for(:published) } + + it { is_expected.to allow_values(true, false).for(:modifiable) } + end +end diff --git a/spec/models/template_spec.rb b/spec/models/template_spec.rb new file mode 100644 index 0000000..434b166 --- /dev/null +++ b/spec/models/template_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +RSpec.describe Template, type: :model do + context "validations" do + it { is_expected.to validate_presence_of(:title) } + + it { is_expected.to allow_values(true, false).for(:published) } + + it "validates uniqueness of version by family_id" do + template = create(:template, version: 1, family_id: 1234) + new_template = build(:template, version: 1, family_id: 1234) + expect(new_template).not_to be_valid + expect(new_template).to have(1).error_on(:version) + end + + # This is currently being set in the defaults before validation + # it { is_expected.not_to allow_value(nil).for(:published) } + + it { is_expected.to validate_presence_of(:org) } + + it { is_expected.to validate_presence_of(:locale) } + + it { is_expected.to allow_values(true, false).for(:is_default) } + + # This is currently being set in the defaults before validation + # it { is_expected.not_to allow_value(nil).for(:is_default) } + + # This is currently being set in the defaults before validation + # it { is_expected.to validate_presence_of(:version) } + + # This is currently being set in the defaults before validation + # it { is_expected.to validate_presence_of(:visibility) } + + # This is currently being set in the defaults before validation + # it { is_expected.to validate_presence_of(:family_id) } + + it { is_expected.to allow_values(true, false).for(:archived) } + + # This is currently being set in the defaults before validation + # it { is_expected.not_to allow_value(nil).for(:archived) } + end +end diff --git a/spec/models/theme_spec.rb b/spec/models/theme_spec.rb new file mode 100644 index 0000000..2d1f31f --- /dev/null +++ b/spec/models/theme_spec.rb @@ -0,0 +1,9 @@ +require 'rails_helper' + +RSpec.describe Theme, type: :model do + context "validations" do + + it { is_expected.to validate_presence_of(:title) } + + end +end diff --git a/spec/models/token_permission_type_spec.rb b/spec/models/token_permission_type_spec.rb new file mode 100644 index 0000000..17956bd --- /dev/null +++ b/spec/models/token_permission_type_spec.rb @@ -0,0 +1,7 @@ +require 'rails_helper' + +RSpec.describe TokenPermissionType, type: :model do + context "validations" do + it { is_expected.to validate_presence_of(:token_type) } + end +end diff --git a/spec/models/user_identifier_spec.rb b/spec/models/user_identifier_spec.rb new file mode 100644 index 0000000..f49f212 --- /dev/null +++ b/spec/models/user_identifier_spec.rb @@ -0,0 +1,11 @@ +require 'rails_helper' + +RSpec.describe UserIdentifier, type: :model do + context "validations" do + it { is_expected.to validate_presence_of(:identifier) } + + it { is_expected.to validate_presence_of(:user) } + + it { is_expected.to validate_presence_of(:identifier_scheme) } + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb new file mode 100644 index 0000000..0a4cb80 --- /dev/null +++ b/spec/models/user_spec.rb @@ -0,0 +1,24 @@ +require 'rails_helper' + +RSpec.describe User, type: :model do + context "validations" do + it { is_expected.to validate_presence_of(:email) } + + it "should validate that email addres is unqique" do + subject.email = "text-email@example.com" + is_expected.to validate_uniqueness_of(:email) + .case_insensitive + .with_message("has already been taken") + end + + it { is_expected.to allow_values("one@example.com", "foo-bar@ed.ac.uk") + .for(:email) } + + it { is_expected.not_to allow_values("example.com", "foo bar@ed.ac.uk") + .for(:email) } + + it { is_expected.to allow_values(true, false).for(:active) } + + it { is_expected.not_to allow_value(nil).for(:active) } + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 5ed51e8..09013da 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -6,7 +6,6 @@ abort("The Rails environment is running in production mode!") if Rails.env.production? require 'rspec/rails' # Add additional requires below this line. Rails is not loaded until this point! - # Requires supporting ruby files with custom matchers and macros, etc, in # spec/support/ and its subdirectories. Files matching `spec/**/*_spec.rb` are # run as spec files by default. This means that files in spec/support that end diff --git a/spec/support/factory_bot.rb b/spec/support/factory_bot.rb index 1ee3e3f..ce5b54a 100644 --- a/spec/support/factory_bot.rb +++ b/spec/support/factory_bot.rb @@ -1,5 +1,8 @@ RSpec.configure do |config| + + config.include(FactoryBot::Syntax::Methods) + config.before(:suite) do - FactoryBot.lint + # FactoryBot.lint end end diff --git a/spec/support/shoulda.rb b/spec/support/shoulda.rb new file mode 100644 index 0000000..afa6498 --- /dev/null +++ b/spec/support/shoulda.rb @@ -0,0 +1,6 @@ +require 'shoulda/matchers' + +RSpec.configure do |config| + config.include(Shoulda::Matchers::ActiveModel, type: :model) + config.include(Shoulda::Matchers::ActiveRecord, type: :model) +end