diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..6accf4a --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,46 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +In the interest of fostering an open and welcoming environment, we as contributors and maintainers pledge to making participation in our project and our community a harassment-free experience for everyone, regardless of age, body size, disability, ethnicity, gender identity and expression, level of experience, nationality, personal appearance, race, religion, or sexual identity and orientation. + +## Our Standards + +Examples of behavior that contributes to creating a positive environment include: + +* Using welcoming and inclusive language +* Being respectful of differing viewpoints and experiences +* Gracefully accepting constructive criticism +* Focusing on what is best for the community +* Showing empathy towards other community members + +Examples of unacceptable behavior by participants include: + +* The use of sexualized language or imagery and unwelcome sexual attention or advances +* Trolling, insulting/derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or electronic address, without explicit permission +* Other conduct which could reasonably be considered inappropriate in a professional setting + +## Our Responsibilities + +Project maintainers are responsible for clarifying the standards of acceptable behavior and are expected to take appropriate and fair corrective action in response to any instances of unacceptable behavior. + +Project maintainers have the right and responsibility to remove, edit, or reject comments, commits, code, wiki edits, issues, and other contributions that are not aligned to this Code of Conduct, or to ban temporarily or permanently any contributor for other behaviors that they deem inappropriate, threatening, offensive, or harmful. + +## Scope + +This Code of Conduct applies both within project spaces and in public spaces when an individual is representing the project or its community. Examples of representing a project or community include using an official project e-mail address, posting via an official social media account, or acting as an appointed representative at an online or offline event. Representation of a project may be further defined and clarified by project maintainers. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be reported by contacting the project team at dmponline@dcc.ac.uk. The project team will review and investigate all complaints, and will respond in a way that it deems appropriate to the circumstances. The project team is obligated to maintain confidentiality with regard to the reporter of an incident. Further details of specific enforcement policies may be posted separately. + +Project maintainers who do not follow or enforce the Code of Conduct in good faith may face temporary or permanent repercussions as determined by other members of the project's leadership. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.4, available at [http://contributor-covenant.org/version/1/4][version] + +[homepage]: http://contributor-covenant.org +[version]: http://contributor-covenant.org/version/1/4/ diff --git a/Gemfile b/Gemfile index 2d9b5b9..7ed9ca7 100644 --- a/Gemfile +++ b/Gemfile @@ -72,7 +72,7 @@ gem 'wkhtmltopdf-binary' gem 'thin' gem 'wicked_pdf' -gem 'htmltoword' +gem 'htmltoword', '>= 0.7' gem 'feedjira' gem 'yaml_db', :git => 'https://github.com/vyruss/yaml_db.git' diff --git a/Gemfile.lock b/Gemfile.lock index 53bf3cf..bde99fd 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -142,7 +142,7 @@ activesupport (>= 4.1.0) hashdiff (0.3.0) hashie (3.4.6) - htmltoword (0.5.1) + htmltoword (0.7.0) actionpack nokogiri rubyzip (>= 1.0) @@ -282,7 +282,7 @@ railties (>= 4.2.0, < 5.1) rolify (5.1.0) ruby-progressbar (1.8.1) - rubyzip (1.2.0) + rubyzip (1.2.1) safe_yaml (1.0.4) sass (3.4.22) sass-rails (5.0.6) @@ -368,7 +368,7 @@ gettext (>= 3.0.2) gettext_i18n_rails (~> 1.8) gettext_i18n_rails_js (~> 1.2.0) - htmltoword + htmltoword (>= 0.7) i18n-js (>= 3.0.0.rc11) jbuilder jquery-rails diff --git a/ISSUE_TEMPLATE.md b/ISSUE_TEMPLATE.md new file mode 100644 index 0000000..ed97d04 --- /dev/null +++ b/ISSUE_TEMPLATE.md @@ -0,0 +1,7 @@ +Please complete the following fields as applicable: + +**Expected behaviour:** + +**Actual behaviour:** + +**Steps to reproduce:** diff --git a/PULL_REQUEST_TEMPLATE.md b/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..460ec83 --- /dev/null +++ b/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,4 @@ +Fixes # . + +Changes proposed in this PR: +- diff --git a/app/controllers/answers_controller.rb b/app/controllers/answers_controller.rb index 7d48ddf..406d75f 100644 --- a/app/controllers/answers_controller.rb +++ b/app/controllers/answers_controller.rb @@ -3,31 +3,33 @@ respond_to :html # PUT/PATCH /answers/[:id] - def update + def update p_params = permitted_params() - @answer = Answer.find_by({plan_id: p_params[:plan_id], question_id: p_params[:question_id], }) - begin - if @answer - authorize @answer - @answer.update(p_params) - if p_params[:question_option_ids].present? - @answer.touch() # Saves the record with the updated_at set to the current time. Needed if only answer.question_options is updated - end - else - @answer = Answer.new(p_params) - @answer.lock_version = 1 - authorize @answer - @answer.save() # NOTE, there is a chance to create multiple answer associated for a plan/question (IF any concurrent thread) INSERTS an answer after checking the existence of an answer (Line 8) - # In order to avoid that edge-case, it is recommended to create answers whenever a new plan is created (e.g. after_create callback) - end - rescue ActiveRecord::StaleObjectError - @stale_answer = @answer + Answer.transaction do @answer = Answer.find_by({plan_id: p_params[:plan_id], question_id: p_params[:question_id]}) + begin + if @answer.present? + authorize @answer + @answer.update(p_params) + if p_params[:question_option_ids].present? + @answer.touch() # Saves the record with the updated_at set to the current time. Needed if only answer.question_options is updated + end + else + @answer = Answer.new(p_params) + @answer.lock_version = 1 + authorize @answer + # NOTE: save! and destroy! must be used for transactions as they raise errors instead of returning false + @answer.save! + end + rescue ActiveRecord::StaleObjectError + @stale_answer = @answer + @answer = Answer.find_by({plan_id: p_params[:plan_id], question_id: p_params[:question_id]}) + end end - + @plan = Plan.includes({ - sections: { - questions: [ + sections: { + questions: [ :answers, :question_format ] @@ -37,7 +39,7 @@ @section = @plan.get_section(@question.section_id) respond_to do |format| - format.js {} + format.js {} end end # End update diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 474281a..42b2c6d 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -8,22 +8,25 @@ @note = Note.new user_id = params[:new_note][:user_id] @note.user_id = user_id - answer_id = params[:new_note][:answer_id] question_id = params[:new_note][:question_id] plan_id = params[:new_note][:plan_id] # create answer if we dont already have one - if answer_id.present? - answer = Answer.find(answer_id) - else - answer = Answer.new - answer.plan_id = plan_id - answer.question_id = question_id - answer.user_id = user_id - answer.save! + answer=nil # if defined within transaction block, was not accessable after + # ensure user has access to plan BEFORE creating/finding an answer + raise Pundit::NotAuthorizedError unless Plan.find(plan_id).readable_by?(user_id) + Answer.transaction do + answer = Answer.find_by(question_id: question_id, plan_id: plan_id) + if answer.blank? + answer = Answer.new + answer.plan_id = plan_id + answer.question_id = question_id + answer.user_id = user_id + answer.save! + end end - @note.answer= answer + @note.answer = answer @note.text = params["#{question_id}new_note_text"] authorize @note diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 21084a9..82633c7 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -7,12 +7,12 @@ # GET /plans/:plan_id/phases/:id/edit def edit - @plan = Plan.eager_load2(params[:plan_id]) + @plan = Plan.load_for_phase(params[:plan_id], params[: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 + @phase = @plan.template.phases.first @readonly = !@plan.editable_by?(current_user.id) # Now we need to get all the themed guidance for the plan. @@ -46,35 +46,35 @@ end end - # create hash from question id to theme to guidance array - # so when we arerendering a question we can grab the guidance out of this - # - # question_guidance = { - # question.id => { - # theme => [ {text: "......", org: "....."} ] - # } - # } + questions = [] + # Appends all the questions for a given phase into questions Array. + @phase.sections.each do |section| + section.questions.each do |question| + questions.push(question) + end + end @question_guidance = {} - @plan.questions.each do |question| + # Puts in question_guidance (key/value) entries where key is the question id and value is a hash. + # Each question id hash has (key/value) entries where key is a theme and value is an Array of {text, org} objects + # Example hash + # question_guidance = { question.id => + # { theme => [ {text: "......", org: "....."} ] } + # } + questions.each do |question| qg = {} question.themes.each do |t| title = t.title qg[title] = theme_guidance[title] if theme_guidance.has_key?(title) end - if !@question_guidance.has_key?(question.id) - @question_guidance[question.id] = Array.new - end @question_guidance[question.id] = qg end if !user_signed_in? then respond_to do |format| format.html { redirect_to edit_user_registration_path } - end - end - + end end - + end # GET /plans/PLANID/phases/PHASEID/status.json def status diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 2c4d8c6..3c713bb 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -66,7 +66,7 @@ published: true) if !ggs.blank? then @plan.guidance_groups << ggs end - default = Template.find_by(is_default: true) + default = Template.default msg = "#{_('Plan was successfully created.')} " @@ -404,7 +404,7 @@ # If no templates were available use the generic templates if @templates.empty? @msg = _("Using the generic Data Management Plan") - @templates << Template.where(is_default: true, published: true).first + @templates << Template.default end @templates = @templates.sort{|x,y| x.title <=> y.title } if @templates.count > 1 diff --git a/app/models/plan.rb b/app/models/plan.rb index 8688f34..b32f3f6 100644 --- a/app/models/plan.rb +++ b/app/models/plan.rb @@ -990,16 +990,15 @@ ]).find(id) end - def self.eager_load2(id) + def self.load_for_phase(id, phase_id) Plan.includes( - [{template: [ + [template: [ {phases: {sections: {questions: [{answers: :notes}, :annotations, :question_format, :themes]}}}, {customizations: :org}, :org - ]}, - {plans_guidance_groups: {guidance_group: {guidances: :themes}}}, - {questions: :themes} - ]).find(id) + ], + plans_guidance_groups: {guidance_group: {guidances: :themes}} + ]).where(id: id, phases: { id: phase_id }).first end diff --git a/app/models/template.rb b/app/models/template.rb index b72d216..f8a7e6a 100644 --- a/app/models/template.rb +++ b/app/models/template.rb @@ -17,7 +17,7 @@ ## # Possibly needed for active_admin # -relies on protected_attributes gem as syntax depricated in rails 4.2 - attr_accessible :id, :org_id, :description, :published, :title, :locale, :customization_of, + attr_accessible :id, :org_id, :description, :published, :title, :locale, :customization_of, :is_default, :guidance_group_ids, :org, :plans, :phases, :dmptemplate_id, :migrated, :version, :visibility, :published, :as => [:default, :admin] @@ -33,16 +33,20 @@ Template.all.valid.distinct.pluck(:dmptemplate_id) end - # Retrieves the most recent version of the template for the specified Org and dmptemplate_id + # Retrieves the most recent version of the template for the specified Org and dmptemplate_id def self.current(dmptemplate_id) Template.where(dmptemplate_id: dmptemplate_id).order(version: :desc).valid.first end - - # Retrieves the current published version of the template for the specified Org and dmptemplate_id + + # Retrieves the current published version of the template for the specified Org and dmptemplate_id def self.live(dmptemplate_id) Template.where(dmptemplate_id: dmptemplate_id, published: true).valid.first end + def self.default + Template.valid.where(is_default: true, published: true).order(:version).last + end + ## # Retrieves the most current customization of the template for the # specified org and dmptemplate_id @@ -79,8 +83,8 @@ ## # convert the given template to a hash and return with all it's associations - # to use, please pre-fetch org, phases, section, questions, annotations, - # question_options, question_formats, + # to use, please pre-fetch org, phases, section, questions, annotations, + # question_options, question_formats, # TODO: Themes & guidance? # # @return [hash] hash of template, phases, sections, questions, question_options, annotations @@ -145,7 +149,7 @@ self.visibility = 1 self.is_default = false self.version = 0 if self.version.nil? - + # Generate a unique identifier for the dmptemplate_id if necessary if self.dmptemplate_id.nil? self.dmptemplate_id = loop do diff --git a/app/views/notes/_add.html.erb b/app/views/notes/_add.html.erb index 2817b90..e335daa 100644 --- a/app/views/notes/_add.html.erb +++ b/app/views/notes/_add.html.erb @@ -12,7 +12,6 @@ id: "new_note_form_#{questionid}") do |f| %> <%= f.hidden_field :user_id, value: current_user.id %> <%= f.hidden_field :question_id, value: questionid %> - <%= f.hidden_field :answer_id, value: answer.id %> <%= f.hidden_field :plan_id, value: plan_id %>
diff --git a/db/seeds.rb b/db/seeds.rb index fabb9a8..1c6d553 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -368,10 +368,19 @@ org: Org.find_by(abbreviation: 'GA'), is_default: false, version: 0, - migrated:false, + migrated: false, dmptemplate_id: 3} ] -templates.map{ |t| Template.create!(t) if Template.find_by(title: t[:title]).nil? } +# Template creation calls defaults handler which sets is_default and +# published to false automatically, so update them after creation +templates.map do |t| + if Template.find_by(title: t[:title]).nil? + tmplt = Template.create!(t) + tmplt.published = t[:published] + tmplt.is_default = t[:is_default] + tmplt.save! + end +end # Create 2 phases for the funder's template and one for our generic template # ------------------------------------------------------- diff --git a/test/integration/template_selection_test.rb b/test/integration/template_selection_test.rb index 2567954..eb95dc7 100644 --- a/test/integration/template_selection_test.rb +++ b/test/integration/template_selection_test.rb @@ -5,100 +5,102 @@ setup do scaffold_template - @template.is_default = true - @template.published = true - @template.save! - + @template = Template.default + @researcher = User.last - + scaffold_org_admin(@template.org) - + @funder = Org.find_by(org_type: 2) - @funder_template = Template.create(title: 'Funder template', org: @funder, migrated: false) + @funder_template = @funder.templates.where(published: true).first #Template.create(title: 'Funder template', org: @funder, migrated: false) # Template can't be published on creation so do it afterward @funder_template.published = true @funder_template.save - + @org = @researcher.org @org_template = Template.create(title: 'Org template', org: @org, migrated: false) # Template can't be published on creation so do it afterward @org_template.published = true @org_template.save end - + # ---------------------------------------------------------- test 'plan gets publish versions of templates' do original_id = @template.id template = version_template(@template) - + sign_in @researcher - + post plans_path(format: :js), {plan: {org_id: @template.org.id}} assert_response :success assert @response.body.include?("$(\"#plan_template_id\").val(\"#{original_id}\");") assert_equal original_id, Template.live(@template.dmptemplate_id).id - + # Version the template again original_id = template.id template = version_template(template) - + # Make sure the published version is used post plans_path(format: :js), {plan: {org_id: @template.org.id}} assert_response :success assert @response.body.include?("$(\"#plan_template_id\").val(\"#{original_id}\");") assert_equal original_id, Template.live(@template.dmptemplate_id).id - + # Update the template and make sure the published version stayed the same sign_in @user put admin_update_template_path(template), {template: {title: "Blah blah blah"}} - + sign_in @researcher - + post plans_path(format: :js), {plan: {org_id: @template.org.id}} assert_response :success assert @response.body.include?("$(\"#plan_template_id\").val(\"#{original_id}\");") assert_equal original_id, Template.live(@template.dmptemplate_id).id end - + # ---------------------------------------------------------- test 'plan gets generic template when no funder or org' do - @template.is_default = true - @template.save! - + temp = Template.find_by(published: true, is_default: true) + if temp.blank? + @template.is_default = true + @template.save! + temp = @template + end + sign_in @researcher - + post plans_path(format: :js), {plan: {org_id: nil}} assert_response :success - assert @response.body.include?("$(\"#plan_template_id\").val(\"#{@template.id}\");"), @response.body + assert @response.body.include?("$(\"#plan_template_id\").val(\"#{temp.id}\");"), @response.body end - + # ---------------------------------------------------------- test 'plan gets org template when no funder' do sign_in @researcher - + post plans_path(format: :js), {plan: {org_id: @org.id, funder_id: nil}} assert_response :success assert @response.body.include?("$(\"#plan_template_id\").val(\"#{@org_template.id}\");"), @response.body end - + # ---------------------------------------------------------- test 'plan gets funder template when no org' do sign_in @researcher - + post plans_path(format: :js), {plan: {org_id: nil, funder_id: @funder.id}} assert_response :success assert @response.body.include?("$(\"#plan_template_id\").val(\"#{@funder_template.id}\");"), @response.body end - + # ---------------------------------------------------------- test 'plan gets funder template when org has no customization' do sign_in @researcher - + post plans_path(format: :js), {plan: {org_id: @org.id, funder_id: @funder.id}} assert_response :success assert @response.body.include?("$(\"#plan_template_id\").val(\"#{@funder_template.id}\");"), @response.body end - + # ---------------------------------------------------------- test 'plan gets customized version of funder template' do customization = Template.create(title: 'Customization', org: @org) @@ -106,9 +108,9 @@ customization.published = true customization.customization_of = @funder_template.dmptemplate_id customization.save - + sign_in @researcher - + post plans_path(format: :js), {plan: {org_id: @org.id, funder_id: @funder.id}} assert_response :success assert @response.body.include?("$(\"#plan_template_id\").val(\"#{customization.id}\");"), @response.body @@ -120,17 +122,17 @@ # Template can't be published on creation so do it afterward funder_template2.published = true funder_template2.save - + sign_in @researcher - + post plans_path(format: :js), {plan: {org_id: @org.id, funder_id: @funder.id}} assert_response :success assert_select "option", 3, "expected a dropdown with 2 templates and a 'please select' option" assert @response.body.include?("