samvera / hyrax

Hyrax is a Ruby on Rails Engine built by the Samvera community. Hyrax provides a foundation for creating many different digital repository applications.
http://hyrax.samvera.org/
Apache License 2.0
183 stars 124 forks source link

Need consistency in terminology for product_name/application_name #179

Open mjgiarlo opened 7 years ago

mjgiarlo commented 7 years ago

Issue by vantuyls Friday Sep 30, 2016 at 22:46 GMT Originally opened as https://github.com/projecthydra/sufia/issues/2762


in: sufia/app/helpers/sufia/sufia_helper_behavior.rb

lines 7-9 there is some confusing inconsistency in the terms used to describe the application/product (see below):

def application_name t('sufia.product_name', default: super) end

It would be less confusing if the terminology was consistent.

mjgiarlo commented 7 years ago

Comment by mjgiarlo Saturday Oct 01, 2016 at 22:54 GMT


@vantuyls I suspect Sufia should just use Blacklight's application_name helper, which looks in i18n though in a different location. In retrospect, we probably should never have duplicated this Blacklight functionality (using a different name) in Sufia. Here's what Blacklight does already:

https://github.com/projectblacklight/blacklight/blob/9c5d7ea48e9794960a578fe1512ed7609b634068/app/helpers/blacklight/blacklight_helper_behavior.rb#L14-L18

Basically: look in your app config for a different value at runtime, or fallback on i18n. If you find sufia.product_name confusing, Sufia's current impl does fallback to Blacklight's, so you could theoretically replace sufia.product_name with blacklight.application_name. We could end support for sufia.product_name, but that might trip people up so we could bundle that change into Sufia 8.0.0.

Thoughts, @jcoyne @jeremyf @cbeer?

no-reply commented 6 years ago

I'm promoting this to 2.x, since it impacts starting a new project named anything but "Hyrax".

See, e.g.: https://github.com/curationexperts/tenejo/commit/649ee9a1b4512d960623ca1c0049e5c0ff95540b

cudevmaxwell commented 3 years ago

Other than the locales files and helpers, product_name is only used in:

./app/models/concerns/hyrax/solr_document/export.rb:        END_NOTE_MAPPINGS.merge({ '%~' => I18n.t('hyrax.product_name'),
./app/presenters/hyrax/file_set_presenter.rb:      "#{human_readable_type} | #{title.first} | ID: #{id} | #{I18n.t('hyrax.product_name')}"
./app/presenters/hyrax/work_show_presenter.rb:      "#{human_readable_type} | #{title.first} | ID: #{id} | #{I18n.t('hyrax.product_name')}"
./app/views/shared/_footer.html.erb:        <p><%= t('hyrax.product_name') %> v<%= Hyrax::VERSION %></p>

application_name is not a internationalized field, but in practice I'm not sure product_name is either, and it's only used in a handful of places. Is it reasonable to just replace product_name with application_name in these 4 files and call this issue closed?

(Then I need to figure out when to use ApplicationController.helpers.application_name and when to use application_name 😄)

no-reply commented 3 years ago

amendment to the above: application_name is i18nized as blacklight.application_name.

the trouble with this is that i don't see a non-breaking way out of it. anything we do to change this is going to pull the rug out from under some applications that are using one of the several reasonable strategies for dealing with this variation. i'm moving this to a new milestone since i can't dream up a way for us to resolve it in 2.x or 3.x.

(Then I need to figure out when to use ApplicationController.helpers.application_name and when to use application_name)

it looks to me like the ApplicationController.helpers context is only provided in content block templates (perhaps they don't have access to the helpers otherwise? the code invoked should be the same, so if just application_name doesn't raise NameError, that's what you should use.