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

Development for v3 #101

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Development for v3 #101

wants to merge 6 commits into from

Conversation

lwe
Copy link
Owner

@lwe lwe commented Apr 29, 2015

Any other wishes? Also feel free to try this branch and see if it fixes #100 as expected.

This is a new major version, because the change to return symbols is a breaking change.

/cc @lucke84, @matthewrudy

@lucke84
Copy link

lucke84 commented Apr 29, 2015

Awesome! Can we put #95 on the table too?

@matthewrudy
Copy link

I think that including :scope as a default :with is something I'd remove

My project has

  • 42 as_enum declarations
  • a maximum of 50 values from one of them

It means we create a whole mess of scopes.

I created my own alternative

SimpleEnum.configure do |e|
  e.with = [:attribute, :dirty, :single_scope]
end

module SimpleEnumCustomisations

  # I want to say
  #
  #   Coupon.status_scope(:active)
  #
  def generate_enum_single_scope_methods_for(enum, accessor)
    return unless respond_to?(:where)

    name = accessor.name

    singleton_class.send(:define_method, "#{name}_scope") do |key|
      where(accessor.source => enum[key])
    end
  end
end

ActiveRecord::Base.send :extend, SimpleEnumCustomisations

if we had enum.fetch(key) I'd use that,
and raise an exception trying to scope by a non-existing enum key

@lwe
Copy link
Owner Author

lwe commented Apr 29, 2015

@lucke84 - added 👍
@matthewrudy mhh, yes probably makes sense to remove that by default, maybe we can merge that single scope idea with what is proposed in #95 somehow?

@hgani
Copy link

hgani commented Jun 16, 2015

Speaking about wishes :), I am not sure if this has been discussed/decided before, but is it possible to support integration with DB-backed enums?

So instead of mapping to integers, we can map to DB's (e.g postgres) native enums.

Thanks

@lwe
Copy link
Owner Author

lwe commented Jun 16, 2015

@hgani no, but this would of course be awesome - any idea how to support this?

@hgani
Copy link

hgani commented Jun 18, 2015

@lwe I did a bit of research on this and the conclusion is that we can't use db-backed enums easily in Rails yet because they break schema.rb (http://stackoverflow.com/questions/7337411/how-to-use-enum-in-postgres-database-in-rails-3/16033077#16033077)

If one day Rails migration supports db-backed enums, then we can start thinking about how to make simple_enum compatible. I think it should be easy actually, probably just use strings instead of integers (which should already work with the current simple_enum)

@lwe
Copy link
Owner Author

lwe commented Jun 18, 2015

Hey @hgani thanks a lot for the effort to check this out and to bad Rails does not support this out of the box yet!

@matthewrudy
Copy link

@hgani you should try schema plus

It does most of this stuff.

@hgani
Copy link

hgani commented Jun 18, 2015

@lwe no worries, happy to help

@matthewrudy thanks, the gem looks promising. I'll keep an eye on it.

@scarroll32
Copy link

Great gem. Multiple select would be awesome, and especially if the selected values could be stored in a Postgres array column, rather than a join table (if strings).

See also:
https://github.com/bbtfr/simple_enum-multiple
https://github.com/bbtfr/simple_enum-persistence

@drewish
Copy link

drewish commented May 23, 2016

As far as feature requests I'd love an accessor that's more compatible with Rails validations.

Specifically, it doesn't seem possible to have an optional enum that is validated. On assignment unknown values are either discarded or trigger argument errors (depending on the accessor) meaning that when the validator runs it can only check for a nil value. It would be nice if there was an option to choose an accessor that would at least hold a copy of the invalid value so the validator could differentiate between missing and invalid.

I'll mention that I'm still on 1.6 so this is just based on a review of the 2.0 code. It looks like it would be easier to plug this into the new code that 1.6, but I'll have to wait until we get through our rails upgrade.

@lwe
Copy link
Owner Author

lwe commented May 25, 2016

Thanks and a good point, maybe rather tricky TBH, as it's hard to distinguish between: okay, I want the value and okay, I want to the original value...

@drewish
Copy link

drewish commented May 25, 2016

Yeah definitely tricky. The two paths I can see are:

  • keep a copy of the raw value that the validator could go off of
  • allow the assignment of an invalid value and just assume that they won't bypass validations

But I'm not sure what issues that would cause elsewhere. For my project rather than getting hacky for one optional field I'm just going to create a new model to reference. Short term it's over kill but longer term it'll set me up for customizable options.

@aledalgrande
Copy link
Contributor

Is this still a valid plan? If yes, can we split it into different PRs? Maybe the planning can be done with the Github Projects feature now that it is available.

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

Successfully merging this pull request may close these issues.

Enum.keys returns strings
7 participants