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

Saving to db when it shouldn't #3

Closed
Crystark opened this issue Feb 21, 2013 · 10 comments
Closed

Saving to db when it shouldn't #3

Crystark opened this issue Feb 21, 2013 · 10 comments

Comments

@Crystark
Copy link

Hi,

I'm working on me seed.db file. The goal is to insert the base roles and create an admin user. Here's what it looks like right now:

    SimpleRoles::Configuration.valid_roles.each do |role|
      r = Role.new
      r.name = role.to_s
      r.save
    end

    user = User.new
    user.skip_confirmation!
    user.assign_attributes(
      username: 'admin',
      email: 'mymail@domain.com',
      password: 'mysecretpassword',
      password_confirmation: 'mysecretpassword',
      active: true,
      roles: :admin
    )

As you can see, this code should create the roles but as i haven't called user.save yet it should not be saving the user. Well as you probably guessed, it does save the user.

I tried removing roles: :admin and then it worked as intended : no user saved.

I actually need the user to be created so it's doesn't really bother me here but i think it shouldn't be acting like and it could be a problem in the future with the confirmation mail beeing sent and all... Or maybe am i doing something wrong ? (i'm quite new to rails)

Thanks for your help.

@stanislaw
Copy link
Owner

As you can see, I didn't touch my Simple Roles for a long time. Many thing could have been improved since then.

I'll take a look. Also, I wouldn't mind if you will suggest a pull request to handle this.

@stanislaw
Copy link
Owner

Ah, I see: roles= method works in a persistent manner. This is the design decision I always wasn't sure about.

I need to think, how to change it, so it would not affect current simple roles users.

@Crystark
Copy link
Author

Yep, i kinda noticed it hadn't been updated for a while. But it's still the simplest way to implement roles :)

I'm afraid i'm too much of a rails newbie to be suggesting any pull request.
I've managed to find the method you're talking about though. If i may ask, what was the reason that made you save roles there ? I'd like to understand how removing it can affect current users (beside not saving the object).

Thanks for your help.

@stanislaw
Copy link
Owner

I've managed to find the method you're talking about though.

Could you please post a couple of strings here so we could discuss your method?

If i may ask, what was the reason that made you save roles there?

It was about how associations work - there was a problem with the fact that without calling save, the roles set non-persistently didn't preserve themselves in memory so they always were lost when save actually was called later. You could experiment with how roles= setter works in https://github.com/stanislaw/simple_roles/blob/master/lib/simple_roles/many/persistence.rb. It looks dead simple, but it is really a tricky one.

I'd like to understand how removing it can affect current users (beside not saving the object).

beside not saving the object - this is exactly this ;) Imagine some production code having just roles=. Some users could have been accustomed to current persistent manner, though I really don't how many folks does really use simple roles.

See http://stackoverflow.com/questions/15020016/does-has-many-association-attribute-setter-always-persist-its-value-to-a-databas I've just opened. The current roles= works in a persistent manner by itself (it is when we call super in roles= method in quoted persistence.rb file. I've tried to make a quick hack on it but didn't succeed to make it behave like any other regular attributes like fx User's name.

@Crystark
Copy link
Author

Could you please post a couple of strings here so we could discuss your method?

I was talking about the roles= method

I see you got an answer on stackoverflow. The virtual attribute seems a good idea. It seems to me that this could absolve you from making breaking changes though that virtual attribute would need to have an other name than just "roles" making the getting roles part a bit trickier.

Anyway, if you want to keep things clean so that everyone can still use roles= to affect roles, i don't see how you cannot make a breaking change and go for a new major version to avoid unwanted updates in production.

@stanislaw
Copy link
Owner

Anyway, if you want to keep things clean so that everyone can still use roles= to affect roles, i don't see how you cannot make a breaking change and go for a new major version to avoid unwanted updates in production.

I don't really like the current way it works and I am ready to make breaking changes. One way to achieve this is to place new Many roles classes just by the current ones and provide a configuration option to use new classes or old. Also it is possible to add deprecation message to current Many- files and hold it for a month or a couple of months or so.

I would begin doing all this immediately after I've got a whole vision of new design. Let me know, if you have any ideas about how it could be done.

Cluster444 added a commit to Cluster444/simple_roles that referenced this issue Feb 22, 2013
stanislaw#3

Signed-off-by: Chris Boertien <chris@aktionlab.com>
@stanislaw
Copy link
Owner

@Crystark, please confirm, that it works for you now.

@Crystark
Copy link
Author

Awesome, just changed me gemfile to:

    gem 'simple_roles', git: 'git://github.com/stanislaw/simple_roles.git' 

And it works like a charm =)

Are you planning to tag it 0.0.9 ?

@stanislaw and @Cluster444 Great work to both of you ! And many thanks !

@stanislaw
Copy link
Owner

I am glad it works for you now. I will release 0.0.9 right now.

@stanislaw
Copy link
Owner

0.0.9 is released.

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

No branches or pull requests

2 participants