Global config for slug builder

[johnnyshields]

I'd like to raise a PR for a global config for alternate slug generation (currently it must be configured in each model.) Something like the following in an initializer:

Mongoid::Slug.config do |c|
  c.slug_builder = ->{ my_proc }
end

In most cases, I would think users (like myself) would want the slug logic to be consistent across their app. In my case, I need to convert Japanese to alphabet and current stringex logic doesn't do a good job (it assumes the characters as Chinese, not Japanese)

Please confirm you'd accept this PR if raised?

[astjohn]

An alternative solution might be to include a module in your models to extend them.

module Extensions
  module MySluggable

    extend ActiveSupport::Concern

    included do
      include Mongoid::Slug

      slug :slugged_attribute
    end

    # == CLASS METHODS == #
    module ClassMethods
    end

    # == INSTANCE METHODS == #
    def slug_builder
      # do something here
    end

  end
end

class MyClass
  include Mongoid::Document
  include Extensions::MySluggable
end

[johnnyshields]

That would work as well, but I see the config approach in many other gems, think it's cleaner overall.

[digitalplaywright]

@johnnyshields I agree that a global configuration for this is better and I am very interested in such a pull request.

A few thoughts on the API:

• current behavior should be default
• no configuration should be required
• custom slug generation specified on a per-model basis should still be supported

That said I believe stringex should be an optional dependency since it is rather heavy and not applicable in all settings. Can you think of a way to make stringex an optional dependency that is pulled in by default?

[digitalplaywright]

@astjohn I agree that this solution would work, but if a user forget to include the module then it will silently "fail". That is why I think a global configuration is better.

[johnnyshields]

@digitalplaywright agreed on all your points above.

For making Stringex optional, what I would propose is that if Stringex is not defined, we simply truncate out all non-alphanumeric chars, and any punctuation/whitespace is converted to '-' char. If Stringex is defined, its functionality is used automatically. (I've done a similar thing in many other gems--see for example http://github.com/radar/by_star using Chronic as dependency).

[johnnyshields]

By the way, I'm not sure when I'll get around to this PR, but it definitely is on my to-do list.

[johnnyshields]

One more note--are there any other settings which should be made globally configurable (+ locally overrideable)?

[digitalplaywright]

@johnnyshields Your solution for making stringex optional seems reasonable. It breaks the API though and would require a major version change. However, instead of truncating all non-alpha-numberic-chars we could use URI::Escape (see http://apidock.com/ruby/URI/Escape/escape) together with converting any punctuation/whitespace to '-' char.

@astjohn What do you think?

The find strategy should also be globally configurable (see https://github.com/digitalplaywright/mongoid-slug#custom-find-strategies). Otherwise I can't think of any obvious candidates.

[astjohn]

@digitalplaywright, I think if this is something that should have exposure in the API, then it's a great approach. You're correct on the major version change as well if you'd like to drop stringex completely.

[johnnyshields]

Agreed on URI::Escape.

[digitalplaywright]

@johnnyshields I noticed that URI::encode is deprecated in favor of ERB::Util.url_encode (See http://ruby-doc.org/stdlib-1.9.3/libdoc/erb/rdoc/ERB/Util.html#method-c-url_encode).

@astjohn I think the URL encoding mechanism should be exposed in the API, which is why we already have custom slugging, but I am on the fence about dropping stringex as a dependency.

Why drop stringex:

• stringex is not applicable in all settings, like eg for Japanese
• stringex is a rather heavy dependency
• stringex implements a subset of the Mongoid Slug functionality so users may be confused why they would need it

Why not:

• 80/20 stringex works well in most contexts as a mechanism to encode strings as urls
• if most users want stringex the extra configuration step makes Mongoid Slug harder to use

@astjohn @johnnyshields Any thoughts on pro/cons of dropping the stringex dependency?

[johnnyshields]

I agree to drop it with graceful degradation. The only con I can think of is that if users don't pay attention when upgrading it may bite them, but a major version bump is fair enough to cover this.

[digitalplaywright]

@johnnyshields We should implement this change. I think it will be a great addition.

[johnnyshields]

Yep, I think so too. I have a reference implementation of this sort of config in the Mongoid Userstamp gem. Unfortunately I'm a bit swamped at my startup at the moment, but may be able to find some time mid-next month. It's on my todo-list.

[anujaware]

@johnnyshields: I added above feature. Thanks for this discussion. It helped me to implement the solution. I hope this will help.

[anujaware]

@dblock I think we can close this issue.

[dblock]

Closed via #196.

[dblock]

FYI was released in 5.3.0