Issue#dcc2983 - Fix for preventing adding a Contributor via Contributor

[johnpinto1]

form for Plan based on whether email and/or name are required.

Fix for DCC bug #2983

Changes:

• Added two new properties to config/initializers/_dmproadmap.rb
  (both devault to false):
  config.x.application.require_contributor_name = false
  config.x.application.require_contributor_email = false
  -In Contributor model the private method name_or_email_presence()
  has been updated to take account of the above mentioned property
  settings.

Fixes # .

Changes proposed in this PR:

[johnpinto1]

@briri & @raycarrick-ed This is attempt to address @briri comment #3062 (comment)
The default leaves the current contributor form validation unchanged.
@raycarrick-ed DCC will need to set properties below to address dcc bug #2983

# Setting require contributor requirement of contributor name and email
      config.x.application.require_contributor_name = true
      config.x.application.require_contributor_email = true

[briri]

thanks @johnpinto1!

[johnpinto1]

@briri I like you to double-check the logic as it took me trial and error to get it to what seemed like "work". Thanks.

[briri]

Hey @johnpinto1 upon further review and testing of different options I would suggest that we do the following:

• Add the config options (just as you have them) ... but make the default require_contributor_name = true
• Remove the current validation method and replace them with the ones below.
• Update the email 'uniqueness' validator to allow nulls. See https://guides.rubyonrails.org/active_record_validations.html#allow-nil
• Update the view to check the config setting to determine if the red asterisk should be displayed

  validates :name, presence: { message: PRESENCE_MESSAGE },
                   if: -> { Rails.configuration.x.application.require_contributor_name }

  validates :email, presence: { message: PRESENCE_MESSAGE },
                    if: -> { Rails.configuration.x.application.require_contributor_email }

[raycarrick-ed]

Looks good overall.
Agree with @briri 's comments above.
I think the code can be simplified where you have:

Rails.configuration.x.application.foobar.present? && Rails.configuration.x.application.foobar

I think that is exactly the same as just

Rails.configuration.x.application.foobar

if it's not present it will return nil which is equivalent to false. That would cut out quite a few lines of code.

[raycarrick-ed]

And another thing .....

when this change comes through we will need to add in the 2 new config setting. Is there an easy way to track this? I could just add them in now on the assumption that they won't have any effect until this comes through or I could leave a note for myself to watch out for it but those both seem quite clumsy ways to do it. Either of you know a better way to handle something like this?

[johnpinto1]

@raycarrick-ed Adding the properties now seems a good idea. I have done something similar in another project to prevent forgetting about it in future.

[johnpinto1]

@raycarrick-ed & @briri I will look at it again. I had issues with Rails.configuration.x.application.foobar
when property not in config. I play around to establish if I was seeing another issue.

[johnpinto1]

@briri & @raycarrick-ed I have updated code based on your comments and my experimenting with nil (when a property is missing by usiing ! or !! (if I want current value or nil -> false)
irb(main):001:0> nil
=> nil
irb(main):002:0> !nil
=> true
irb(main):003:0> nil == true
=> false
Method now

def name_or_email_presence
  return true unless (name.blank? && email.blank?) ||
                      (!!Rails.configuration.x.application.require_contributor_name &&
                      name.blank?) ||
                      (!!Rails.configuration.x.application.require_contributor_email &&
                      email.blank?)

  # Error messages for case where name.blank? and email.blank? and
  # properties Rails.configuration.x.application.require_contributor_name and
  # Rails.configuration.x.application.require_contributor_email both not set (i.e, nil) or false.
  if (name.blank? && email.blank?) &&
     !Rails.configuration.x.application.require_contributor_name &&
     !Rails.configuration.x.application.require_contributor_email
    errors.add(:name, _("can't be blank if no email is provided."))
    errors.add(:email, _("can't be blank if no name is provided."))
  else

    # Error messages cases where enither ame.blank? or email.blank?
    if name.blank? && !!Rails.configuration.x.application.require_contributor_name
      errors.add(:name, _("can't be blank."))
    end

    if email.blank? && !!Rails.configuration.x.application.require_contributor_email
      errors.add(:email, _("can't be blank."))
    end
  end
end