ANUSF / ADAUsers

The ADA user management server.
MIT License
4 stars 0 forks source link

Refactor user saving #13

Closed RohanM closed 13 years ago

RohanM commented 13 years ago

At the moment, the saving logic for user management (not user registration) is split between the controller and the model. It was set up this way because the user.save() method won't work directly - many users in the database have missing fields, so the validations fail.

There's a Better Way. We could create a base class for User, called UserWithoutValidations, and then have User inherit from it. We could then use UserWithoutValidations for the admin interface, and the User class for registration.

Once we had this in place, we could perform all saving for the admin interface through the UserWithoutValidations model. Instead of having a number of setter methods (ie. update_role!, add_datasets! etc.), we could perform these actions in a before_update hook. This hook would need to handle partial data, such as having only the datasets_cat_a_to_add attr set. Essentially, we'd be moving the controller and model setter code into this update hook.

The controller could then simply call update_attributes(), and all the business of saving would be handled by the model, where it belongs.

odf commented 13 years ago

Another possibility would be to set a (non-persistent) flag in the user instance when saving via the admin interface and then use the :unless option on validations that should be skipped in that case. Your idea sounds cleaner, though.

RohanM commented 13 years ago

I've implemented it as the sub/super class arrangement for now. If the split between the two files becomes confusing, I'll consider changing to the validation :unless method.

odf commented 13 years ago

I've had another idea earlier today: couldn't we just split the user model into one that contains the absolutely essentials that have to be there, such as username, password and email, and one that contains all the personal details? The latter would then be fully validated upon registration, but the admin interface could call safe_without_validation (or whatever the method is called) for any records that didn't come in valid.

RohanM commented 13 years ago

I'm using update_attributes in this case, and it looks like there's no easy way to circumvent validations with that call (unlike with save). The User / UserWithoutValidations model split seems like the best solution at the moment.