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

Establish how Admin Model Customizations and Upmin Admin Configs will be set. #46

Open
joncalhoun opened this issue Sep 21, 2014 · 4 comments

Comments

@joncalhoun
Copy link
Member

@reneklacan @jdurand @jasoncox @scalhoun - I am tagging you all because I would like your feedback on this since you are all involved in other PRs and issues that I think are relevant.

TL;DR; - I think we should use an approach like Draper for admin specific model logic, including the color of the model, the display name, etc, and we should use Upmin.configure do |config| ... end to set things that are more global - eg which models are actually displayed in upmin-admin.

Long Version
There are basically 2 issues to address here. First, we need a good way to separate admin specific logic from models. This has been requested several times, and it makes the code much easier to maintain. I think that draper is a great example of how this can be done. For example:

# This resides in app/upmin/admin_user_persistence.rb

class AdminUserPersistance < Upmin::AdminModel
  display_name "User"
  menu_group "Some Dropdown Group Name" # This would be optional.
  # color :red # or see below
  color custom: "#001100"

  attributes :id, :name, :email, :admin_only_attr
  searchable :id, :name, :email
  actions :reset_password, :admin_only_method

  def admin_only_attr
    # Do some work
  end

  def admin_only_method
    # Do some work
  end

end

If the admin logic is in it's own file, it makes sense to me to start putting configs specific to that model inside of that file. Eg - the display_name can for UserPersistence can be set there and it wouldn't clock up the actual user_persistence.rb file with admin specifics.

Going beyond this, I think that we should veer away from the direction we were initially headed in #19 of putting configs in the model file. Things like the model's group could be set in the individual model file, and which models are displayed could be customized on a more global config like the one proposed in #36.

I think that the draper-like solution needs to get kicked off sooner rather than later, so I am going to start working on that this week and try to push a few other features off until a later release to make time for it, but I would love to hear your feedback on all of this before getting too involved.

@jdurand
Copy link
Contributor

jdurand commented Sep 22, 2014

@joncalhoun yes, the decorator approach is much cleaner. I don't like throwing stuff that has nothing to do with data persistence in my models.

@reneklacan
Copy link

@joncalhoun @jdurand I totally agree. This approach is much cleaner. Putting this into the seperate layer is the way because of better maintainability and testability.

@joncalhoun
Copy link
Member Author

Awesome. I'll start working on a first version and hopefully one of you can look it over to check my sanity 😄

@joncalhoun
Copy link
Member Author

Most of the dev work for isolating the code is done. I still need to go add in configs and update docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants