Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Default settings #2699

Open
adrianthedev opened this issue Apr 17, 2024 · 2 comments
Open

Default settings #2699

adrianthedev opened this issue Apr 17, 2024 · 2 comments
Labels
Configuration Enhancement New feature or request Task Something to get done

Comments

@adrianthedev
Copy link
Collaborator

adrianthedev commented Apr 17, 2024

Context

We want to give the ability to developers to set certain settings to a certain value globally.

For example, set all the belongs_to fields to have the searchable value to true.

Explorations

  config.default_settings = {
    fields: {
      belongs_to: {
        searchable: false,
        polymorphic_as: false,
        relation_method: false,
      }
    },
    "fields.belongs_to.searchable": false,
    "fields.belongs_to.polymorphic_as": false,
    "fields.belongs_to.relation_method": false,
  }


@searchable = default_attribute("fields.belongs_to.searchable", args[:searchable] == true)
@polymorphic_as = default_attribute("fields.belongs_to.polymorphic_as", args[:can_create].nil? ? true : args[:can_create])


      def default_attribute(key, default)
        if value_exists?(key)
          value(key)
        else
          default
        end
      end
@Paul-Bob
Copy link
Contributor

Possible approaches (none of the code was executed/tested):

1. Merge defaults on boot

  1. Create a hash for each field defaults, let's take an example on Avo::Fields::BelongsTo
BELONGS_TO_DEFAULTS = {
  searchable: false,
  can_create: true
}
  1. Collect user defaults for that field (to be decided where and how)
USER_BELONGS_TO_DEFAULTS = {
  searchable: true,
  can_create: true
}
  1. On boot merge options and override avo defaults with user defaults:
    BELONGS_TO_DEFAULTS.merge!(USER_BELONGS_TO_DEFAULTS)

  2. Use the merged defaults hash to access default values:

def initialize(id, **args, &block)
  # ...
  @searchable = args[:searchable].nil? ? BELONGS_TO_DEFAULTS[:searchable] : args[:searchable]
  @can_create = args[:can_create].nil? ? BELONGS_TO_DEFAULTS[:can_create] : args[:can_create]
  # ...
end
  1. Optional: DRY refactor
def initialize(id, **args, &block)
  # ...
  initialize_option :searchable, args
  initialize_option :can_create, args
  # ...
end

def initialize_option(option, args)
  user_option = args.dig(option)

  instance_variable_set(:"@#{option}", user_option.nil? ? BELONGS_TO_DEFAULTS.dig(option) : user_option)
end

2. Check for user default on each initialize

  1. Collect user defaults for that field (to be decided where and how)
USER_BELONGS_TO_DEFAULTS = {
  searchable: true,
  can_create: true
}
  1. When user option is not provided check for user defaults first then apply avo defaults if not present
def initialize(id, **args, &block)
  # ...
  @searchable = if args[:searchable].nil?
    user_default = USER_BELONGS_TO_DEFAULTS[:searchable]
    user_default.nil? ? false : user_default
  else
    args[:searchable]
  end

  @can_create = if args[:can_create].nil?
    user_default = USER_BELONGS_TO_DEFAULTS[:can_create]
    user_default.nil? ? true : user_default
  else
    args[:can_create]
  end
  # ...
end
  1. Optional: DRY refactor
def initialize(id, **args, &block)
  # ...
  initialize_option :searchable, args
  initialize_option :can_create, args
  # ...
end

def initialize_option(option, args)
  user_option = args.dig(option)

  value = if user_option.nil?
    user_default = USER_BELONGS_TO_DEFAULTS.dig(option)
    user_default.nil? ? false : user_default
  else
    user_option
  end

  instance_variable_set(:"@#{option}", value)
end

Things to have in consideration

  1. Performance. We should identify the approach with less performance impact
  2. Presence checks. We should validate present by using nil? method instead present? because the presence method can return false if the value is intentionally setted as false or empty.
  3. Complexity. Lets ensure that the complexity of this mechanism is maintained as low as possible to preserve easy readability and maintainability

Copy link
Contributor

github-actions bot commented May 4, 2024

This issue has been marked as stale because there was no activity for the past 15 days.

@github-actions github-actions bot added the Stale label May 4, 2024
@adrianthedev adrianthedev added Enhancement New feature or request Configuration Task Something to get done and removed Stale labels May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Configuration Enhancement New feature or request Task Something to get done
Projects
Status: No status
Development

No branches or pull requests

2 participants