Mohammed Morsi wrote:
>   Looks good, and works save one nit that needs to be changed and a 
> couple of comments below.
>   
I'm in the process of reworking this patch (with input from one of 
mtaylor's as well)
> On 09/24/2010 10:35 AM, [email protected] wrote:
>   
>> From: Jozef Zigmund<[email protected]>
>>
>> ---
>>   src/app/controllers/users_controller.rb |   22 ++++++++++++------
>>   src/app/views/users/_form.haml          |    7 ++++++
>>   src/app/views/users/_treatment.haml     |   37 
>> +++++++++++++++++++++++++++++++
>>   src/app/views/users/edit.haml           |   27 ++++++++++++++++------
>>   4 files changed, 79 insertions(+), 14 deletions(-)
>>   create mode 100644 src/app/views/users/_treatment.haml
>>
>> diff --git a/src/app/controllers/users_controller.rb 
>> b/src/app/controllers/users_controller.rb
>> index f61c6d2..20cbfdf 100644
>> --- a/src/app/controllers/users_controller.rb
>> +++ b/src/app/controllers/users_controller.rb
>> @@ -51,19 +51,27 @@ class UsersController<  ApplicationController
>>
>>     def edit
>>       @user = @current_user
>> +    @is_admin = @current_user.permissions.collect { |p| p.role }.
>> +                              find { |r| r.name == "Administrator" }
>>     end
>>
>>     
> This is fine for now, but I'd image at some point we'd want to set this 
> based on whether the user has the 'user_modify' privilege.
>
> Checking for admin has the same effect though since that is the only 
> role that does currently.
>
>   
This change will be in my version.
>
>> +      = form.label :user_status
>> +      = radio_button_tag "user_status","Active",true
>> +      = label_tag "user_status", "Active"
>> +      = radio_button_tag "user_status","Inactive"
>> +      = label_tag "user_status", "Inactive"
>>     
> This is indented too far here. Need to remove a couple spaces be pushing.
>
> Also changing this status doesn't have any permanent effect, will this 
> be added at some point?
>
>   
Once we get the fully styling this will be grayed out/un-editable
>>   %fieldset
>>     = form.label :first_name
>>     = form.text_field :first_name
>> diff --git a/src/app/views/users/_treatment.haml 
>> b/src/app/views/users/_treatment.haml
>> new file mode 100644
>> index 0000000..f6ab887
>> --- /dev/null
>> +++ b/src/app/views/users/_treatment.haml
>> @@ -0,0 +1,37 @@
>> +%h3 USER TREATMENT
>> +- form_tag '/treatment' do
>> +  = label_tag 'apply_treatment', "Apply User Treatment:"
>> +  = select_tag 'user_treatment', options_for_select(["Choose Treatment"])
>> +  = submit_tag 'Apply', :disabled =>  true
>> +%h3 TEMPLATES
>> +- form_tag '/template' do
>> +  = label_tag 'template_group', "Template Group:"
>> +  %br/
>> +  = label_tag 'permissions', "Permissions:"
>> +  = text_field_tag 'permissions_text', "General Usage"
>> +  = select_tag 'template_group', options_for_select(["Choose Template 
>> Group"])
>>     
> These select boxes are supposed to be empty save the "choose" item correct?
>
>   
These are being removed -- 'templates' stuff doesn't belong here like this.
>> +  %hr
>> +  = label_tag 'quota', "Quota:"
>> +  = text_field_tag 'quota'
>> +  (instances)
>>     
> The fields on this form, don't seem to have an impact on the user update 
> operation. Are these a placeholder as well for something else?
>
>   
Fixing quota is part of my update to the patch too.
>> diff --git a/src/app/views/users/edit.haml b/src/app/views/users/edit.haml
>> index 7654866..f580482 100644
>> --- a/src/app/views/users/edit.haml
>> +++ b/src/app/views/users/edit.haml
>> @@ -1,8 +1,21 @@
>> -.dcloud_form
>> -  %h2 Edit my profile.
>> -  - form_for @user, :url =>  account_path do |f|
>> -    = f.error_messages
>> -    = render :partial =>  "form", :object =>  f
>> -    = f.submit "Update", :class =>  "submit"
>> +.formwindow
>> +- if @is_admin
>> +  %h2
>> +    EDITING USER:
>> +    = @user.first_name + " " + @user.last_name
>> +  %h3 BASIC USER INFORMATION
>> +- else
>> +  %h2 Edit an Account
>> +- form_for @user, :url =>  account_path, :html =>  {:id =>  "login"} do |f|
>> +  = f.error_messages
>> +  = render :partial =>  "form", :object =>  f
>> +  - if @is_admin
>> +    = f.submit "Back", :class =>  "formbutton", :name =>  "back"
>> +    = f.submit "Reset", :class =>  "formbutton", :name =>  "reset"
>> +    = f.submit "Save", :class =>  "formbutton", :name =>  "save"
>> +  - else
>> +    = f.submit "Make Changes", :class =>  "formbutton", :name =>  
>> "make_changes"
>>       %br/
>> -    = link_to "Cancel", account_path, :class =>  'actionlink'
>> +    = link_to "Cancel", account_path, :class =>  "formbuton"
>> +- if @is_admin
>> +  =render :partial =>'treatment'
>>     
> ACK after the spacing in the haml is fixed and provided the placeholder 
> stuff should be there.
>
>    -Mo
>   
Actually, NACK to this particular patch, but I'm submitting an updated 
patch that incorporates this with some fixes.

Scott
_______________________________________________
deltacloud-devel mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/deltacloud-devel

Reply via email to