5
\$\begingroup\$

In order to only show relevant information to my users I want to disable the whole panel when all fields inside that panel are empty.

Currently I do this hacky method to get to this result:

if resource.phone.present? or
    resource.fax.present? or 
    resource.email.present? or 
    resource.website.present?
  panel I18n.t('contact_details') do
    attributes_table_for resource do
      row :phone if resource.phone.present?
      row :fax if resource.fax.present?
      row :email if resource.email.present?
      row :website if resource.website.present?
    end
  end
end

So I need to check all fields in order to know if I display the panel. Then once I display the panel I need to check all the fields again in order to display the given row or not.

Now the user sees only what is relevant but my admin code is cluttered and no longer as maintainable.

\$\endgroup\$
  • 3
    \$\begingroup\$ I don't get it, the first if checks if you have all the elements present, why do you test again in the attributes_table_for block? \$\endgroup\$ – tokland Sep 8 '16 at 11:34
  • 3
    \$\begingroup\$ Welcome to Code Review! Good job on your first post, though some more context would be nice IMO, it might be worth checking if changes to other parts of the code / data model would make this part easier (apart from what @tokland already said). \$\endgroup\$ – ferada Sep 8 '16 at 12:15
  • \$\begingroup\$ @tokland: you're right, these have to be ors rather than ands. I changed that \$\endgroup\$ – Besi Sep 9 '16 at 12:26
3
\$\begingroup\$

I don't know ActiveAdmin, so I can only offer a local improvement. That said, a little metaprogramming could work here. The idea is to have the list of resource items in one place:

RESOURCES_ITEMS = [
  :phone,
  :fax,  
  :email,   
  :resource,
]

and then use that list with Object#public_send and Enumerable#any?:

if RESOURCE_ITEMS.any? { |item| resource.public_send(item).present? }
  panel I18n.t('contact_details') do
    attributes_table_for resource do
      RESOURCE_ITEMS.each do |item|
        if resource.public_send(item).present?
          row.public_send(item)
        end
      end
    end
  end
end

While not a huge improvement, this does DRY the list of resource items.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.