diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 391120b..eead0ca 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -8,10 +8,12 @@ def edit @plan = Plan.eager_load2(params[:plan_id]) + # authorization done on plan so found in plan_policy authorize @plan phase_id = params[:id].to_i @phase = @plan.template.phases.select {|p| p.id == phase_id}.first + @readonly = !@plan.editable_by?(current_user.id) # the eager_load pulls in ALL answers # need to restrict to just ones for this plan diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 526f76a..e554b93 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -140,7 +140,7 @@ authorize @plan # If there was no phase specified use the template's 1st phase @phase = (params[:phase].nil? ? @plan.template.phases.first : Phase.find(params[:phase])) - @readonly = @plan.editable_by?(current_user.id) + @readonly = !@plan.editable_by?(current_user.id) respond_to :html end diff --git a/app/models/user.rb b/app/models/user.rb index 352b564..14f1af2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -107,14 +107,13 @@ # # @param new_organisation_id [Integer] the id for an organisation # @return [String] the empty string as a causality of setting api_token -=begin - def organisation_id=(new_organisation_id) - unless self.can_change_org? || new_organisation_id.nil? || self.organisation.nil? + def org_id=(new_org_id) + unless self.can_change_org? || new_org_id.nil? || self.org.nil? # rip all permissions from the user self.perms.delete_all end # set the user's new organisation - super(new_organisation_id) + super(new_org_id) self.save! # rip api permissions from the user self.remove_token! @@ -124,10 +123,9 @@ # sets a new organisation for the user # # @param new_organisation [Organisation] the new organisation for the user - def organisation=(new_organisation) - organisation_id = new_organisation.id unless new_organisation.nil? + def organisation=(new_org) + org_id = new_org.id unless new_org.nil? end -=end ## # checks if the user is a super admin diff --git a/app/policies/phase_policy.rb b/app/policies/phase_policy.rb index 5d43a7e..caa0e3c 100644 --- a/app/policies/phase_policy.rb +++ b/app/policies/phase_policy.rb @@ -8,10 +8,10 @@ end ## + # Org-admin side # Users can modify phases if: # - They can modify templates # - The template which they are modifying belongs to their org - ## def admin_show? user.can_modify_templates? && (phase.template.org_id == user.org_id) diff --git a/app/policies/plan_policy.rb b/app/policies/plan_policy.rb index d6c8703..aa99a2b 100644 --- a/app/policies/plan_policy.rb +++ b/app/policies/plan_policy.rb @@ -7,13 +7,13 @@ @user = user @plan = plan end - + def show? @plan.readable_by?(@user.id) end def edit? - @plan.editable_by?(@user.id) + @plan.readable_by?(@user.id) end def update_guidance_choices? diff --git a/app/views/phases/_answer_form_ro.html.erb b/app/views/phases/_answer_form_ro.html.erb index fcf175d..915db2a 100644 --- a/app/views/phases/_answer_form_ro.html.erb +++ b/app/views/phases/_answer_form_ro.html.erb @@ -1,4 +1,4 @@ - <% q_format = question.question_format%> + <% if readonly != "always" then %>
> - <%= semantic_form_for answer, :url => {:controller => :answers, :action => :create }, :html=>{:method=>:post}, :remote => true do |f| %> + <%= semantic_form_for answer, :url => {:controller => :answers, :action => :update }, :html=>{:method=>:post}, :remote => true do |f| %> <%= f.inputs do %> <%= f.input :plan_id, :as => :hidden %> <%= f.input :user_id, :as => :hidden, :input_html => { :value => current_user.id } %> @@ -20,10 +21,10 @@ - <%= raw question.text %> +

<%= raw question.text %>

- <<% if question.annotations.where('type <> ?',Annotation.types[:example_answer]).any? %> + <% if question.annotations.where('type <> ?',Annotation.types[:example_answer]).any? %> <% annotation = question.annotations.where('type <> ?',Annotation.types[:example_answer]).order(:created_at).first %>
@@ -41,21 +42,21 @@ <% if q_format.title == _('Check box') || q_format.title == _('Multi select box') || q_format.title == _('Radio buttons') || q_format.title == _('helpers.dropdown') then%> <% options = question.options.order("number") %> - <% if q_format.title == _('Check box') then %> - <% if readonly then %> + <% if q_format.title == _('Check box') %> + <% if readonly %> <%= f.input :options, :as => :check_boxes, :collection => options, :label => false, input_html => { :disabled => true, :id => "options-#{question.id}" } %> <% else %> <%= f.input :options, :as => :check_boxes, :collection => options, :label => false, :input_html => { :id => "options-#{question.id}" } %> <% end %> - <% elsif q_format.title == _('Multi select box') then %> + <% elsif q_format.title == _('Multi select box') %> <% if readonly then %> <%= f.input :options, :as => :select, :collection => options, :label => false, :input_html => { :multiple => true, :disabled => true , :id => "options-#{question.id}" } %> <% else %> <%= f.input :options, :as => :select, :collection => options, :label => false, :input_html => { :multiple => true , :id => "options-#{question.id}" } %> <% end %> - <% elsif q_format.title == _('Radio buttons') then%> + <% elsif q_format.title == _('Radio buttons') %>
    <% options.each do |op| %>
  1. @@ -63,31 +64,25 @@ <% if readonly then %> <%= f.radio_button :option_ids, op.id, :checked => true, disabled: true, id: "answer_option_ids_#{op.id}"%> <% else %> - <%= f.radio_button :option_ids, op.id, :checked => true, id: "answer_option_ids_#{op.id}"%> + <%= f.radio_button :option_ids, op.id, :checked => true, id: "answer_option_ids_#{op.id}"%> <% end %> <%else%> <% if readonly then %> - <%= f.radio_button :option_ids, op.id, :checked => false, disabled: true, id: "answer_option_ids_#{op.id}"%> + <%= f.radio_button :option_ids, op.id, :checked => false, disabled: true, id: "answer_option_ids_#{op.id}"%> <% else %> - <%= f.radio_button :option_ids, op.id, :checked => false, id: "answer_option_ids_#{op.id}"%> + <%= f.radio_button :option_ids, op.id, :checked => false, id: "answer_option_ids_#{op.id}"%> <% end %> <% end %> <%= op.text %>
  2. <% end %>
- <% elsif q_format.title == _('Dropdown') then%> - <% if readonly then %> + <% elsif q_format.title == _('Dropdown') %> + <% if readonly %> <%= f.input :options, :as => :select, :collection => options, :label => false, :input_html => { :multiple => false, :disabled => true, :id => "options-#{question.id}" } %> <% else %> <%= f.input :options, :as => :select, :collection => options, :label => false, :input_html => { :multiple => false, :id => "options-#{question.id}" } %> <% end %> - <% end %> - - <% if question.option_comment_display == true then%> - <%= label_tag("answer-text-#{question.id}".to_sym, _('Comment')) %> - <%= text_area_tag("answer-text-#{question.id}".to_sym, answer.text, class: "tinymce") %> - <%end%> <% elsif q_format.title == _('Text field') then %> @@ -96,16 +91,17 @@ <% elsif q_format.title == _('Text area') then%> <%= text_area_tag("answer-text-#{question.id}".to_sym, answer.text, class: "tinymce") %> <% end %> + + <% if question.option_comment_display == true then%> + <%= label_tag("answer-text-#{question.id}".to_sym, _('Comment')) %> + <%= text_area_tag("answer-text-#{question.id}".to_sym, answer.text, class: "tinymce") %> + <%end%> <% end %> <%= f.actions do %> - <% if readonly then %> - <%= f.action :submit, :label => _('Save'), :button_html => { :class => "btn btn-primary"}, :input_html => { :disabled => true } %> - <% else %> - <%= f.action :submit, :label => _('Save'), :button_html => { :class => "btn btn-primary"} %> - <% end %> + <% end %> <% end %> @@ -113,7 +109,6 @@ <% end %>
> -

<%= question.text %>

<% if q_format.title == _('Check box') || q_format.title == _('Multi select box') || q_format.title == _('Radio buttons') || q_format.title == _('Dropdown') %>
    @@ -134,7 +129,7 @@ <% if answer.created_at.nil? then %> <%= _('Not answered yet') %> <% else %> - <%= _('Answered')%><%= answer.created_at %><%= _(' by')%><%= answer.user.name %> + <%= _('Answered ')%><%= answer.created_at %><%= _(' by ')%><%= answer.user.name %> <% end %>
@@ -143,12 +138,13 @@
- <% @comments = Notes.where("question_id = ? AND plan_id = ?", question.id, answer.plan_id ) %> + <% @comments = answer.notes.all %> <%= hidden_field_tag :question_id, question.id, :class => "question_id" %>
    - <% if (!question.guidance.nil? && question.guidance != "") || !question_guidances.empty? then %> + <% q_guidance = question.get_guidance_annotation(current_user.org) %> + <% if (q_guidance.present? && q_guidance.text.present?) || !question_guidances.empty? %> <% css_style_comment_div = "display: none;"%> <% css_style_guidance_div = ""%> @@ -156,8 +152,8 @@ <%= link_to _('Guidance'), "#", :class => "guidance_accordion_button" %>
  • - <% if @comments.count > 0 then%> - <% comments_label_with_count = "#{_('Notes')} (#{@comments.count})"%> + <% if @comments.length > 0 %> + <% comments_label_with_count = "#{_('Notes')} (#{@comments.length})"%> <%= link_to comments_label_with_count , "#", :class => "comments_accordion_button" %> <%else%> <%= link_to _('Share note'), "#", :class => "comments_accordion_button" %> @@ -184,59 +180,56 @@
    - <% if !question.guidance.nil? && question.guidance != "" then %> + <% q_guidance = question.get_guidance_annotation(current_user.org) %> + <% if q_guidance.present? && q_guidance.text.present %> <% end %> - <% guidance_accordion_id = 1 %> - <% question_guidances.each_pair do |theme, group| %> - <% group.each do |gobj| %> -
    - -
    -
    <%= raw gobj[:text] %>
    -
    - <% guidance_accordion_id += 1 %> -
    - <% end %> + <% guidance_accordion_id = 0 %> + <% question_guidances.each_pair do |theme, group| %> + <% group.each do |gobj| %> +
    + +
    +
    <%= raw gobj[:text] %>
    +
    + <% guidance_accordion_id += 1 %> +
    + <% end %> + <% end %> <% end %>
    - <%= render :partial => "comments", locals: {questionId: question.id, plan_id: answer.plan_id }%> + <%= render :partial => "note", locals: {question: question, plan: @plan, answer: answer }%>
-<% if last_question_id == question.id then %> +<% if last_question_id == question.id %>
<% else %>
-<% end %> +<% end %> \ No newline at end of file diff --git a/app/views/phases/edit.html.erb b/app/views/phases/edit.html.erb index f91d5f7..3a29a21 100644 --- a/app/views/phases/edit.html.erb +++ b/app/views/phases/edit.html.erb @@ -28,10 +28,10 @@ <% if session[:question_id_comments].to_i != 0 then %> <% question_from_comment = Question.find(session[:question_id_comments])%> - <% if sectionid == question_from_comment.section_id then %> + <% if sectionid == question_from_comment.section_id then %> <%= hidden_field_tag :comment_section_id, question_from_comment.section_id, :class => "comment_section_id" %> - <%end%> - <% end%> + <%end%> + <% end%>
@@ -91,9 +91,9 @@ partialname += "_ro" end %> - + <% guidances = @question_guidance[question.id] %> - + <%= render partial: partialname, locals: { plan: @plan, @@ -101,9 +101,10 @@ question: question, question_guidances: guidances, last_question_id: section.questions.last.id, + readonly: @readonly } %> -
+
<% end %>
diff --git a/app/views/plans/edit.html.erb b/app/views/plans/edit.html.erb index d44caea..57a7d38 100644 --- a/app/views/plans/edit.html.erb +++ b/app/views/plans/edit.html.erb @@ -8,6 +8,7 @@ <%= render :partial => "plan_title", locals: {plan: @plan} %> +

<%=@readonly%>

<% status = @plan.status %> diff --git a/test/functional/plans_controller_test.rb b/test/functional/plans_controller_test.rb index 259bfbf..3811003 100644 --- a/test/functional/plans_controller_test.rb +++ b/test/functional/plans_controller_test.rb @@ -107,20 +107,6 @@ assert assigns(:selected_guidance_groups) end - # GET /plan/:id/edit (edit_plan_path) - # ---------------------------------------------------------- - test 'show the edit plan page' do - # Should redirect user to the root path if they are not logged in! - try_no_user_and_unauthorized(edit_plan_path(@plan)) - - sign_in @user - get edit_plan_path(@plan) - assert_response :success - assert assigns(:plan) - assert assigns(:phase) - assert assigns(:readonly) - end - # PUT /plan/:id (plan_path) # ---------------------------------------------------------- test 'update the plan' do