Hm, nevermind - I was pretty sure that I had run both sets of tests, and after re-running the cucumber tests - they all pass for me. I guess we'll see what's up tomorrow.
Mainn ----- Original Message ----- > From: "Tzu-Mainn Chen" <[email protected]> > To: "Matt Wagner" <[email protected]> > Cc: [email protected] > Sent: Monday, October 1, 2012 7:01:34 PM > Subject: Re: [PATCH] BZ859998 disable certain fields from editing if in ldap > mode > > Ah, no, it's my fault - I forgot to run the cucumber tests, only the > spec tests. I'll take a look a re-send tomorrow. > > Mainn > > ----- Original Message ----- > > From: "Matt Wagner" <[email protected]> > > To: [email protected] > > Cc: [email protected] > > Sent: Monday, October 1, 2012 6:12:18 PM > > Subject: Re: [PATCH] BZ859998 disable certain fields from editing > > if in ldap mode > > > > On Mon, Oct 01, 2012 at 04:47:38PM -0400, [email protected] > > wrote: > > > From: Tzu-Mainn Chen <[email protected]> > > > > > > --- > > > src/app/controllers/users_controller.rb | 1 + > > > src/app/models/user.rb | 9 +++++++++ > > > src/app/views/users/_form.html.haml | 12 ++++++------ > > > src/config/locales/en.yml | 1 + > > > 4 files changed, 17 insertions(+), 6 deletions(-) > > > > > > diff --git a/src/app/controllers/users_controller.rb > > > b/src/app/controllers/users_controller.rb > > > index d26028d..4a2868f 100644 > > > --- a/src/app/controllers/users_controller.rb > > > +++ b/src/app/controllers/users_controller.rb > > > @@ -97,6 +97,7 @@ class UsersController < ApplicationController > > > @user = params[:id] ? User.find(params[:id]) : current_user > > > require_privilege(Privilege::MODIFY, User) unless @user == > > > current_user > > > @title = t'users.edit.edit_user' > > > + @ldap_user = (SETTINGS_CONFIG[:auth][:strategy] == "ldap") > > > end > > > > > > def update > > > diff --git a/src/app/models/user.rb b/src/app/models/user.rb > > > index e8bee26..ee7ba1f 100644 > > > --- a/src/app/models/user.rb > > > +++ b/src/app/models/user.rb > > > @@ -77,6 +77,8 @@ class User < ActiveRecord::Base > > > before_validation :strip_whitespace > > > after_save :update_entity > > > > > > + validate :validate_ldap_changes, :if => Proc.new {|user| > > > + !user.new_record? && SETTINGS_CONFIG[:auth][:strate gy] == > > > "ldap"} > > > validates_presence_of :quota > > > validates_length_of :first_name, :maximum => 255, :allow_blank > > > => true > > > validates_length_of :last_name, :maximum => 255, :allow_blank > > > => true > > > @@ -185,6 +187,13 @@ class User < ActiveRecord::Base > > > end > > > end > > > > > > + def validate_ldap_changes > > > + if self.first_name_changed? || self.last_name_changed? || > > > self.email_changed? || > > > + self.username_changed? || self.crypted_password_changed? > > > then > > > + errors.add(:base, > > > I18n.t("users.errors.cannot_edit_ldap_user")) > > > + end > > > + end > > > + > > > def encrypt_password > > > self.crypted_password = Password::update(password) unless > > > password.blank? > > > end > > > diff --git a/src/app/views/users/_form.html.haml > > > b/src/app/views/users/_form.html.haml > > > index 859d920..abbb89a 100644 > > > --- a/src/app/views/users/_form.html.haml > > > +++ b/src/app/views/users/_form.html.haml > > > @@ -5,28 +5,28 @@ > > > .field > > > = form.label :first_name > > > .input > > > - = form.text_field :first_name, :class =>"check_change" > > > + = form.text_field :first_name, :class =>"check_change", > > > :disabled => @ldap_user > > > .field > > > = form.label :last_name > > > .input > > > - = form.text_field :last_name, :class =>"check_change" > > > + = form.text_field :last_name, :class =>"check_change", > > > :disabled => @ldap_user > > > .field > > > = form.label :email, t(:email), :required => true > > > .input > > > - = form.text_field :email, :class =>"check_change" > > > + = form.text_field :email, :class =>"check_change", > > > :disabled > > > => @ldap_user > > > %fieldset > > > .field > > > = form.label :username, t(:choose_name), :required => true > > > .input > > > - = form.text_field :username, :class => "check_change" > > > + = form.text_field :username, :class => "check_change", > > > :disabled => @ldap_user > > > .field > > > = form.label :password, form.object.new_record? ? > > > t(:choose_password) : t(:change_password), :required => > > > form.object.new_record? > > > .input > > > - = form.password_field :password > > > + = form.password_field :password, :disabled => @ldap_user > > > .field > > > = form.label :password_confirmation, t(:confirm_password), > > > :required => form.object.new_record? > > > .input > > > - = form.password_field :password_confirmation > > > + = form.password_field :password_confirmation, :disabled => > > > @ldap_user > > > > > > - if check_privilege(Privilege::MODIFY, User) > > > %fieldset > > > diff --git a/src/config/locales/en.yml > > > b/src/config/locales/en.yml > > > index cd43c3b..df75c38 100644 > > > --- a/src/config/locales/en.yml > > > +++ b/src/config/locales/en.yml > > > @@ -153,6 +153,7 @@ en: > > > name_starts_with_B: "Name starts with B" > > > errors: > > > has_running_instances: '%{username} has running instances' > > > + cannot_edit_ldap_user: Cannot edit LDAP user > > > user_groups: > > > groups: "User Groups" > > > local: Local > > > -- > > > 1.7.6.5 > > > > I'm afraid I have to head out for the night without being able to > > dig > > into this fully, but I'm seeing some test failures when I have LDAP > > enabled: > > > > Failing Scenarios: > > cucumber features/authentication.feature:37 # Scenario: Change user > > login to one with invalid length > > cucumber features/user.feature:11 # Scenario: Change the password > > cucumber features/user.feature:89 # Scenario: Edit existing user > > cucumber features/user_group.feature:25 # Scenario: Delete user > > groups > > > > The last one looks to fail on master in that case as well. > > > > I would dive a bit deeper and try to see what's going on, but > > unfortunately I've got head out for the night. > > > > -- Matt > > >
