Skip to content

Commit

Permalink
Update the configuration of model label as per discussion here: #319
Browse files Browse the repository at this point in the history
The model label is now configurable at the model level only, and is no longer configurable
at the section level (list, navigate, update,...), as this was considered overkill.
This refactoring was in part motivated by issue #319 which reported that display of labels was very inconsistent across
various screens, and the label configuration, if given, was not consistently effective.
The Labelable module was removed, and the methods model into config/model.rb
All references to label across the code have been updated to use the model configuration.
Specs updated and passing. Readme also updated accordingly.
  • Loading branch information
Wolfram Arnold committed Mar 16, 2011
1 parent f07bcbe commit c67eccc
Show file tree
Hide file tree
Showing 24 changed files with 117 additions and 142 deletions.
83 changes: 42 additions & 41 deletions README.mkd
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ Help
If you have a question, you can ask the [official RailsAdmin mailing list](http://groups.google.com/group/rails_admin)
or ping sferik on IRC in [#railsadmin on irc.freenode.net](http://webchat.freenode.net/?channels=railsadmin).

API Update Note
---------------
The ability to set model labels for each section (list, navigation, update, ...) has been removed,
as it was deemed unnecessarily granular and was not fully honored all displays.
That also means that the methods `label_for_navigation`, etc. are no longer functional. They print a warning at the moment.
See details in the examples below for the currently supported way to label models.
This change was motivated by the conversation following a [bug report](https://github.com/sferik/rails_admin/issues/319/#issue/319/comment/875868)
about label display errors.

Screenshots
-----------
![List view](https://github.com/sferik/rails_admin/raw/master/screenshots/list.png "List view")
Expand Down Expand Up @@ -163,27 +172,51 @@ Both also accept a block:

**Setting the model's label**

If you need to customize the label of the model within the navigation tab, use:
If you need to customize the label of the model, use:

RailsAdmin.config do |config|
config.model Team do
navigation do
label "List of teams"
end
label "List of teams"
end
end

Remember, you can pass the value as an argument or as a block as with the
before mentioned visibility options. Besides that, the label also has a
shorthand syntax:
This label will be used anywhere the model name is shown, e.g. on the navigation tabs,
Dashboard page, list pages, etc.

**The object_label method**

The model configuration has another option `object_label` which configures
the title display of a single database record.

By default it queries if the record in question has columns named "name" or
"title". If neither is found it returns the model's classname appended with its
database identifier. You can add label methods (or replace the default [:name, :title]) with:

RailsAdmin.config {|c| c.label_methods << :description}

This value is used in a number of places in RailsAdmin - for instance as the
output of belongs to associations in the listing views of related models, as
the option labels of the relational fields' input widgets in the edit views of
related models and as part of the audit information stored in the history
records - so keep in mind that this configuration option has widespread
effects.

RailsAdmin.config do |config|
config.model Team do
label_for_navigation "List of teams"
object_label do
"#{bindings[:object].name} - #{bindings[:object].league.name}"
end
end
end

which allows both forms of configuration value passing as well.
This would output "Team's name - Team's league's name" in all the places
mentioned in paragraph above example.

*Difference between `label` and `object_label`*

`label` and `object_label` are both model configuration options. `label` is used
whenever Rails Admin refers to a model class, while `object_label` is used whenever
Rails Admin refers to an instance of a model class (representing a single database record).

**To enable CKEditor for a field**

Expand Down Expand Up @@ -230,7 +263,6 @@ evaluated at runtime.
* Sortability
* Column CSS class
* Column width
* The object_label method

**Number of items per page**

Expand Down Expand Up @@ -456,37 +488,6 @@ column, you can:
end
end

**Fields - The object_label method**

List section has a configuration option `object_label` which configures
the title of a single database record.

By default it queries if the record in question has columns named "name" or
"title". If neither is found it returns the model's classname appended with its
database identifier. You can add label methods (or replace the default [:name, :title]) with:

RailsAdmin.config {|c| c.label_methods << :description}

This value is used in a number of places in RailsAdmin - for instance as the
output of belongs to associations in the listing views of related models, as
the option labels of the relational fields' input widgets in the edit views of
related models and as part of the audit information stored in the history
records - so keep in mind that this configuration option has widespread
effects.

RailsAdmin.config do |config|
config.model Team do
list do
object_label do
"#{bindings[:object].name} - #{bindings[:object].league.name}"
end
end
end
end

This would output "Team's name - Team's league's name" in all the places
mentioned in paragraph above example.

### Create and update views

* Field groupings
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/rails_admin/history_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def slider

def for_model
@page_type = @abstract_model.pretty_name.downcase
@page_name = t("admin.history.page_name", :name => @model_config.list.label)
@page_name = t("admin.history.page_name", :name => @model_config.label)
@general = true

@page_count, @history = AbstractHistory.history_for_model @abstract_model, params[:query], params[:sort], params[:sort_reverse], params[:all], params[:page]
Expand All @@ -34,7 +34,7 @@ def for_model

def for_object
@page_type = @abstract_model.pretty_name.downcase
@page_name = t("admin.history.page_name", :name => @model_config.list.with(:object => @object).object_label)
@page_name = t("admin.history.page_name", :name => @model_config.with(:object => @object).object_label)
@general = false

@history = AbstractHistory.history_for_object @abstract_model, @object, params[:query], params[:sort], params[:sort_reverse]
Expand Down
26 changes: 13 additions & 13 deletions app/controllers/rails_admin/main_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def new
end
@authorization_adapter.authorize(:new, @abstract_model, @object)
end
@page_name = t("admin.actions.create").capitalize + " " + @model_config.create.label.downcase
@page_name = t("admin.actions.create").capitalize + " " + @model_config.label.downcase
@page_type = @abstract_model.pretty_name.downcase
render :layout => 'rails_admin/form'
end
Expand All @@ -66,11 +66,11 @@ def create
end
@object.attributes = @attributes
@object.associations = params[:associations]
@page_name = t("admin.actions.create").capitalize + " " + @model_config.create.label.downcase
@page_name = t("admin.actions.create").capitalize + " " + @model_config.label.downcase
@page_type = @abstract_model.pretty_name.downcase

if @object.save
AbstractHistory.create_history_item("Created #{@model_config.list.with(:object => @object).object_label}", @object, @abstract_model, _current_user)
AbstractHistory.create_history_item("Created #{@model_config.with(:object => @object).object_label}", @object, @abstract_model, _current_user)
redirect_to_on_success
else
render_error
Expand All @@ -80,7 +80,7 @@ def create
def edit
@authorization_adapter.authorize(:edit, @abstract_model, @object) if @authorization_adapter

@page_name = t("admin.actions.update").capitalize + " " + @model_config.update.label.downcase
@page_name = t("admin.actions.update").capitalize + " " + @model_config.label.downcase
@page_type = @abstract_model.pretty_name.downcase

render :layout => 'rails_admin/form'
Expand All @@ -92,7 +92,7 @@ def update
@cached_assocations_hash = associations_hash
@modified_assoc = []

@page_name = t("admin.actions.update").capitalize + " " + @model_config.update.label.downcase
@page_name = t("admin.actions.update").capitalize + " " + @model_config.label.downcase
@page_type = @abstract_model.pretty_name.downcase

@old_object = @object.clone
Expand All @@ -111,7 +111,7 @@ def update
def delete
@authorization_adapter.authorize(:delete, @abstract_model, @object) if @authorization_adapter

@page_name = t("admin.actions.delete").capitalize + " " + @model_config.list.label.downcase
@page_name = t("admin.actions.delete").capitalize + " " + @model_config.label.downcase
@page_type = @abstract_model.pretty_name.downcase

render :layout => 'rails_admin/delete'
Expand All @@ -121,17 +121,17 @@ def destroy
@authorization_adapter.authorize(:destroy, @abstract_model, @object) if @authorization_adapter

@object = @object.destroy
flash[:notice] = t("admin.delete.flash_confirmation", :name => @model_config.list.label)
flash[:notice] = t("admin.delete.flash_confirmation", :name => @model_config.label)

AbstractHistory.create_history_item("Destroyed #{@model_config.list.with(:object => @object).object_label}", @object, @abstract_model, _current_user)
AbstractHistory.create_history_item("Destroyed #{@model_config.with(:object => @object).object_label}", @object, @abstract_model, _current_user)

redirect_to rails_admin_list_path(:model_name => @abstract_model.to_param)
end

def bulk_delete
@authorization_adapter.authorize(:bulk_delete, @abstract_model) if @authorization_adapter

@page_name = t("admin.actions.delete").capitalize + " " + @model_config.list.label.downcase
@page_name = t("admin.actions.delete").capitalize + " " + @model_config.label.downcase
@page_type = @abstract_model.pretty_name.downcase

render :layout => 'rails_admin/delete'
Expand All @@ -144,7 +144,7 @@ def bulk_destroy
@destroyed_objects = @abstract_model.destroy(params[:bulk_ids], scope)

@destroyed_objects.each do |object|
message = "Destroyed #{@model_config.list.with(:object => object).object_label}"
message = "Destroyed #{@model_config.with(:object => object).object_label}"
AbstractHistory.create_history_item(message, object, @abstract_model, _current_user)
end

Expand Down Expand Up @@ -241,7 +241,7 @@ def get_attributes

def redirect_to_on_success
param = @abstract_model.to_param
pretty_name = @model_config.update.label
pretty_name = @model_config.label
action = params[:action]

if params[:_add_another]
Expand All @@ -258,7 +258,7 @@ def redirect_to_on_success

def render_error whereto = :new
action = params[:action]
flash.now[:error] = t("admin.flash.error", :name => @model_config.update.label, :action => t("admin.actions.#{action}d"))
flash.now[:error] = t("admin.flash.error", :name => @model_config.label, :action => t("admin.actions.#{action}d"))
render whereto, :layout => 'rails_admin/form'
end

Expand Down Expand Up @@ -301,7 +301,7 @@ def list_entries(other = {})
@record_count = @abstract_model.count(options, scope)

@page_type = @abstract_model.pretty_name.downcase
@page_name = t("admin.list.select", :name => @model_config.list.label.downcase)
@page_name = t("admin.list.select", :name => @model_config.label.downcase)
end

def associations_hash
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/rails_admin/delete.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
</li>
<li>
&rsaquo;
<%= link_to(@model_config.list.label, rails_admin_list_path(:model_name => @abstract_model.to_param)) %>
<%= link_to(@model_config.label, rails_admin_list_path(:model_name => @abstract_model.to_param)) %>
</li>
<li>
&rsaquo;
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/rails_admin/form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
</li>
<li>
&rsaquo;
<%= link_to(@model_config.update.label, rails_admin_list_path(:model_name => @abstract_model.to_param)) %>
<%= link_to(@model_config.label, rails_admin_list_path(:model_name => @abstract_model.to_param)) %>
</li>
<li>
&rsaquo;
Expand Down
4 changes: 2 additions & 2 deletions app/views/layouts/rails_admin/list.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
<% if @history %>
<li>
&rsaquo;
<%= link_to(@model_config.list.label, rails_admin_list_path(:model_name => @abstract_model.to_param)) %>
<%= link_to(@model_config.label, rails_admin_list_path(:model_name => @abstract_model.to_param)) %>
</li>
<li>
&rsaquo;
Expand All @@ -52,7 +52,7 @@
<% else %>
<li>
&rsaquo;
<span><%= @model_config.list.label %></span>
<span><%= @model_config.label %></span>
</li>
<% end%>
</ul>
Expand Down
6 changes: 3 additions & 3 deletions app/views/rails_admin/main/_delete_notice.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@
%>
<ul>
<li>
<%= @abstract_model.pretty_name %>: <%= link_to(model_config.list.with(:object => object).object_label, rails_admin_edit_path(:model_name => @abstract_model.to_param, :id => object.id)) %>
<%= @abstract_model.pretty_name %>: <%= link_to(model_config.with(:object => object).object_label, rails_admin_edit_path(:model_name => @abstract_model.to_param, :id => object.id)) %>
<ul>
<% @abstract_model.has_many_associations.each do |association| %>
<% object.send(association[:name]).reject{|child| child.nil?}.each do |child| %>
<li>
One or more <%= @abstract_model.pretty_name.pluralize.downcase %> in <%= association[:pretty_name].downcase %>: <%= link_to(RailsAdmin.config(child).list.with(:object => child).object_label, rails_admin_edit_path(:model_name => association[:name].to_s.singularize.to_sym, :id => child.id)) %>
One or more <%= @abstract_model.pretty_name.pluralize.downcase %> in <%= association[:pretty_name].downcase %>: <%= link_to(RailsAdmin.config(child).with(:object => child).object_label, rails_admin_edit_path(:model_name => association[:name].to_s.singularize.to_sym, :id => child.id)) %>
</li>
<% end %>
<% end %>
<% @abstract_model.has_one_associations.each do |association| %>
<% child = object.send(association[:name]) %>
<% next if child.nil? %>
<li>
A <%= @abstract_model.pretty_name.downcase %> in <%= association[:pretty_name].downcase %>: <%= link_to(RailsAdmin.config(child).list.with(:object => child).object_label, rails_admin_edit_path(:model_name => association[:name], :id => child.id)) %>
A <%= @abstract_model.pretty_name.downcase %> in <%= association[:pretty_name].downcase %>: <%= link_to(RailsAdmin.config(child).with(:object => child).object_label, rails_admin_edit_path(:model_name => association[:name], :id => child.id)) %>
</li>
<% end %>
</ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

# for edit action
selected = field.bindings[:object].send(association_name).map{|object|
[field.associated_model_config.list.with(:object => object).object_label, object.id]
[field.associated_model_config.with(:object => object).object_label, object.id]
}.sort_by{|object| object.first}

# if error accurs - show the selected associations
Expand Down
2 changes: 1 addition & 1 deletion app/views/rails_admin/main/_has_many_association.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

# for edit action
selected = field.bindings[:object].send(association_name).map{|object|
[field.associated_model_config.list.with(:object => object).object_label, object.id]
[field.associated_model_config.with(:object => object).object_label, object.id]
}.sort_by{|object| object.first}

# if error accurs - show the selected associations
Expand Down
4 changes: 2 additions & 2 deletions app/views/rails_admin/main/_navigation.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
<ul id="nav">
<li <%if @page_type == "dashboard" %>class="active"<% end %>><%= link_to(t("admin.dashboard.name"), rails_admin_dashboard_path) %></li>
<% models[0..max_visible_tabs-1].each do |model| %>
<li <%if @page_type == model.abstract_model.pretty_name.downcase %>class="active"<% end %>><%= link_to(model.navigation.label, rails_admin_list_path(:model_name => model.abstract_model.to_param)) %></li>
<li <%if @page_type == model.abstract_model.pretty_name.downcase %>class="active"<% end %>><%= link_to(model.label, rails_admin_list_path(:model_name => model.abstract_model.to_param)) %></li>
<% end %>
<% if models.size > max_visible_tabs %>
<li class="more">
<a href="#">&raquo;</a>
<ul>
<% models[max_visible_tabs..models.size].each do |model| %>
<li <%if @page_type == model.abstract_model.pretty_name.downcase %>class="active"<% end %>><%= link_to(model.navigation.label, rails_admin_list_path(:model_name => model.abstract_model.to_param)) %></li>
<li <%if @page_type == model.abstract_model.pretty_name.downcase %>class="active"<% end %>><%= link_to(model.label, rails_admin_list_path(:model_name => model.abstract_model.to_param)) %></li>
<% end %>
</ul>
</li>
Expand Down
2 changes: 1 addition & 1 deletion app/views/rails_admin/main/delete.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
</div>
<%= render(:partial => 'layouts/rails_admin/flash', :locals => {:flash => flash}) -%>
<div id="contentMainDelete">
<p>Are you sure you want to delete the <%= @abstract_model.pretty_name.downcase %> &ldquo;<strong><%= @model_config.list.with(:object => @object).object_label %></strong>&rdquo;? All of the following related items will be deleted:</p>
<p>Are you sure you want to delete the <%= @abstract_model.pretty_name.downcase %> &ldquo;<strong><%= @model_config.with(:object => @object).object_label %></strong>&rdquo;? All of the following related items will be deleted:</p>
<%= render :partial => "delete_notice", :object => @object %>
<%= form_for(@object, :url => rails_admin_destroy_path(:model_name => @abstract_model.to_param, :id => @object.id), :html => {:method => "delete"}) do %>
<div id="deleteConfirmation">
Expand Down
2 changes: 1 addition & 1 deletion app/views/rails_admin/main/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<% if authorized? :list, abstract_model %>
<tr class="<%= cycle 'odd', 'even' %>">
<td class="modelNameRow">
<%= link_to(RailsAdmin.config(abstract_model).list.label, rails_admin_list_path(:model_name => abstract_model.to_param), :class => "show") %>
<%= link_to(RailsAdmin.config(abstract_model).label, rails_admin_list_path(:model_name => abstract_model.to_param), :class => "show") %>
</td>
<td>
<% if (last_used = @most_recent_changes[abstract_model.pretty_name]) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/rails_admin/main/list.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
<%= paginate(@current_page, @page_count, :url => params).html_safe %>
<% end %>
<%= @record_count %>
<%= @model_config.abstract_model.model.model_name.human(:count => @record_count, :default => @record_count != 1 ? @model_config.list.label.downcase.pluralize : @model_config.list.label.downcase) %>
<%= @model_config.abstract_model.model.model_name.human(:count => @record_count, :default => @record_count != 1 ? @model_config.label.downcase.pluralize : @model_config.label.downcase) %>
<% if @page_count.to_i == 2 %>
<%= link_to(t("admin.list.show_all"), params.merge(:all => true), :class => "showall") %>
<% end %>
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_admin/config/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def with(bindings = {})
end

# Register an instance option. Instance option is a configuration
# option that stores it's value within an instance variable and is
# option that stores its value within an instance variable and is
# accessed by an instance method. Both go by the name of the option.
def self.register_instance_option(option_name, scope = self, &default)
unless options = scope.instance_variable_get("@config_options")
Expand Down
2 changes: 1 addition & 1 deletion lib/rails_admin/config/fields/association.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def association
# [label, id] arrays.
def associated_collection
associated_model_config.abstract_model.all.map do |object|
[associated_model_config.list.with(:object => object).object_label, object.id]
[associated_model_config.with(:object => object).object_label, object.id]
end
end

Expand Down
Loading

6 comments on commit c67eccc

@jdmorani
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this commit is that the sections can't be localized anymore because we can't apply a translatable label to them

@sferik
Copy link
Collaborator

@sferik sferik commented on c67eccc Mar 17, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted in a8f20fa.

There was a conflict in the README. Wolfram, can you please check to make sure I resolved that conflict correctly?

I reopened #322 so we can try to find a solution to this issue that respects localization.

@kaapa
Copy link
Collaborator

@kaapa kaapa commented on c67eccc Mar 17, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guys, I'm sorry, but I'm bit confused what the actual problem is?

As each view (navigation, create, list, update) in this commit just uses the model's label configuration option which still is overrideable and has ActiveModel::Base.model_name.human as the default every model's label should be as translatable as it used to be (via standard Rails I18n). It just isn't possible to use section specific translations anymore. With section specific I mean that one would set one translation for the list view and another for the navigation etc.

So could someone please elaborate so I could try to tackle the problem?

@sferik
Copy link
Collaborator

@sferik sferik commented on c67eccc Mar 17, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JD: Can you confirm/deny that reverting this commit fixed the problem for you. If not, I will revert the revert.

Someone else traced the problem back to this commit d036623, but that seemed too old to be relevant.

@jdmorani
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Kaapa's workaround is ok. I did not think of localizing the activerecord model names. We should probably update the readme accordingly.

Thanks!
JD

@sferik
Copy link
Collaborator

@sferik sferik commented on c67eccc Mar 17, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll revert the revert.

Please sign in to comment.