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..bb1c5b9 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' @@ -89,7 +89,7 @@ # ------------------------------------------------ # CODE DOCUMENTATION -gem 'yard' +gem 'yard', '>= 0.9.11' gem 'redcarpet' diff --git a/Gemfile.lock b/Gemfile.lock index 53bf3cf..6e6fceb 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) @@ -188,7 +188,7 @@ mime-types (3.1) mime-types-data (~> 3.2015) mime-types-data (3.2016.0521) - mini_portile2 (2.1.0) + mini_portile2 (2.3.0) minitest (5.9.1) minitest-capybara (0.8.2) capybara (~> 2.2) @@ -215,9 +215,8 @@ multi_xml (0.5.5) multipart-post (2.0.0) mysql2 (0.3.21) - nokogiri (1.6.8) - mini_portile2 (~> 2.1.0) - pkg-config (~> 1.1.7) + nokogiri (1.8.1) + mini_portile2 (~> 2.3.0) normalize-rails (4.1.1) oauth2 (1.2.0) faraday (>= 0.8, < 0.10) @@ -237,7 +236,6 @@ omniauth (>= 1.0.0) orm_adapter (0.5.0) pg (0.19.0) - pkg-config (1.1.7) po_to_json (1.0.1) json (>= 1.6.0) protected_attributes (1.1.3) @@ -282,7 +280,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) @@ -346,7 +344,7 @@ wkhtmltopdf-binary (0.12.3) xpath (2.0.0) nokogiri (~> 1.3) - yard (0.9.5) + yard (0.9.12) PLATFORMS ruby @@ -368,7 +366,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 @@ -406,7 +404,7 @@ wicked_pdf wkhtmltopdf-binary yaml_db! - yard + yard (>= 0.9.11) RUBY VERSION ruby 2.2.2p95 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/annotations_controller.rb b/app/controllers/annotations_controller.rb index e83366b..73a34fb 100644 --- a/app/controllers/annotations_controller.rb +++ b/app/controllers/annotations_controller.rb @@ -16,6 +16,7 @@ # if they dont exist, no requirement for them to be saved ex_save = example_answer.present? ? example_answer.save : true guid_save = guidance.present? ? guidance.save : true + @question.section.phase.template.dirty = true if ex_save && guid_save redirect_to admin_show_phase_path(id: @question.section.phase_id, section_id: @question.section_id, question_id: @question.id, edit: 'true'), notice: _('Information was successfully created.') @@ -73,6 +74,8 @@ @section = @question.section @phase = @section.phase + @phase.template.dirty = true + if ex_save && guid_save redirect_to admin_show_phase_path(id: @phase.id, section_id: @section.id, question_id: @question.id, edit: 'true'), notice: _('Information was successfully updated.') else @@ -95,6 +98,7 @@ @question = @example_answer.question @section = @question.section @phase = @section.phase + @phase.template.dirty = true if @example_answer.destroy redirect_to admin_show_phase_path(id: @phase.id, section_id: @section.id, edit: 'true'), notice: _('Information was successfully deleted.') else @@ -113,4 +117,4 @@ return annotation end -end \ No newline at end of file +end 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 f7edc1f..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.')} " @@ -370,10 +370,10 @@ def template_options(org_id, funder_id) @templates = [] - if !org_id.blank? || !funder_id.blank? + if org_id.present? || funder_id.present? if funder_id.blank? # Load the org's template(s) - unless org_id.nil? + if org_id.present? org = Org.find(org_id) @templates = Template.valid.where(published: true, org: org, customization_of: nil).to_a @msg = _("We found multiple DMP templates corresponding to the research organisation.") if @templates.count > 1 @@ -384,27 +384,27 @@ # Load the funder's template(s) @templates = Template.valid.where(published: true, org: funder).to_a - unless org_id.blank? + if org_id.present? org = Org.find(org_id) # Swap out any organisational cusotmizations of a funder template @templates.each do |tmplt| customization = Template.valid.find_by(published: true, org: org, customization_of: tmplt.dmptemplate_id) - unless customization.nil? + if customization.present? && tmplt.updated_at < customization.created_at @templates.delete(tmplt) @templates << customization end end end - msg = _("We found multiple DMP templates corresponding to the funder.") if @templates.count > 1 + @msg = _("We found multiple DMP templates corresponding to the funder.") if @templates.count > 1 end end # 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/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 3a50d54..ad2b69b 100644 --- a/app/controllers/roles_controller.rb +++ b/app/controllers/roles_controller.rb @@ -8,21 +8,22 @@ authorize @role access_level = params[:role][:access_level].to_i set_access_level(access_level) + message = '' if params[:user].present? if @role.plan.owner.present? && @role.plan.owner.email == params[:user] flash[:notice] = _('Cannot share plan with %{email} since that email matches with the owner of the plan.') % {email: params[:user]} else - if Role.find_by(plan: @role.plan, user: User.find_by(email: params[:user])) # role already exists + user = User.where_case_insensitive('email',params[:user]).first + if Role.find_by(plan: @role.plan, user: user) # role already exists flash[:notice] = _('Plan is already shared with %{email}.') % {email: params[:user]} - else - message = _('Plan shared with %{email}.') % {email: params[:user]} - user = User.find_by(email: params[:user]) + else if user.nil? registered = false User.invite!(email: params[:user]) - message = _('Invitation to %{email} issued successfully.') % {email: params[:user]} + message = _('Invitation to %{email} issued successfully. \n') % {email: params[:user]} user = User.find_by(email: params[:user]) end + message += _('Plan shared with %{email}.') % {email: user.email} @role.user = user if @role.save if registered then UserMailer.sharing_notification(@role, current_user).deliver_now end diff --git a/app/controllers/templates_controller.rb b/app/controllers/templates_controller.rb index 42eb4ee..2717fef 100644 --- a/app/controllers/templates_controller.rb +++ b/app/controllers/templates_controller.rb @@ -55,6 +55,7 @@ random = rand 2147483647 break random unless Template.exists?(dmptemplate_id: random) end + customisation.dirty = true customisation.save customisation.phases.includes(:sections, :questions).each do |phase| 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/settings/template.rb b/app/models/settings/template.rb index 59ac3de..a9fd6f8 100644 --- a/app/models/settings/template.rb +++ b/app/models/settings/template.rb @@ -1,11 +1,11 @@ module Settings class Template < RailsSettings::SettingObject - + #attr_accessible :var, :target, :target_id, :target_type VALID_FONT_FACES = [ - 'Arial, Helvetica, Sans-Serif', - '"Times New Roman", Times, Serif' + '"Times New Roman", Times, Serif', + 'Arial, Helvetica, Sans-Serif' ] VALID_FONT_SIZE_RANGE = (8..14) @@ -17,13 +17,13 @@ DEFAULT_SETTINGS = { formatting: { margin: { # in millimeters - top: 20, - bottom: 20, - left: 20, - right: 20 + top: 10, + bottom: 10, + left: 10, + right: 10 }, font_face: VALID_FONT_FACES.first, - font_size: 12 # pt + font_size: 10 # pt }, max_pages: 3, fields: { 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/models/user.rb b/app/models/user.rb index f900030..f35abe3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -6,8 +6,8 @@ # Include default devise modules. Others available are: # :token_authenticatable, :confirmable, # :lockable, :timeoutable and :omniauthable - devise :invitable, :database_authenticatable, :registerable, :recoverable, - :rememberable, :trackable, :validatable, :omniauthable, + devise :invitable, :database_authenticatable, :registerable, :recoverable, + :rememberable, :trackable, :validatable, :omniauthable, :omniauth_providers => [:shibboleth, :orcid] ## @@ -26,14 +26,14 @@ q = "%#{query}%" conditions = t[:title].matches(q) columns = %i( - grant_number identifier description principal_investigator data_contact + grant_number identifier description principal_investigator data_contact ) columns = ['grant_number', 'identifier', 'description', 'principal_investigator', 'data_contact'] columns.each {|col| conditions = conditions.or(t[col].matches(q)) } self.where(conditions) end end - + has_many :user_identifiers has_many :identifier_schemes, through: :user_identifiers @@ -41,12 +41,12 @@ # Possibly needed for active_admin # -relies on protected_attributes gem as syntax depricated in rails 4.2 #accepts_nested_attributes_for :roles - #attr_accessible :password_confirmation, :encrypted_password, :remember_me, - # :id, :email, :firstname, :last_login,:login_count, :orcid_id, - # :password, :shibboleth_id, :user_status_id, :surname, - # :user_type_id, :org_id, :skip_invitation, :other_organisation, + #attr_accessible :password_confirmation, :encrypted_password, :remember_me, + # :id, :email, :firstname, :last_login,:login_count, :orcid_id, + # :password, :shibboleth_id, :user_status_id, :surname, + # :user_type_id, :org_id, :skip_invitation, :other_organisation, # :accept_terms, :role_ids, :dmponline3, :api_token, - # :organisation, :language, :language_id, :org, :perms, + # :organisation, :language, :language_id, :org, :perms, # :confirmed_at, :org_id validates :email, email: true, allow_nil: true, uniqueness: {message: _("must be unique")} @@ -62,13 +62,13 @@ # What do they do? do they do it efficiently, and do we need them? # Determines the locale set for the user or the organisation he/she belongs - # @return String or nil + # @return String or nil def get_locale if !self.language.nil? return self.language.abbreviation elsif !self.org.nil? return self.org.get_locale - else + else return nil end end @@ -126,7 +126,7 @@ def organisation=(new_org) org_id = new_org.id unless new_org.nil? end - + ## # checks if the user is a super admin # if the user has any privelege which requires them to see the super admin page @@ -144,7 +144,7 @@ # # @return [Boolean] true if the user is an organisation admin def can_org_admin? - return self.can_grant_permissions? || self.can_modify_guidance? || + return self.can_grant_permissions? || self.can_modify_guidance? || self.can_modify_templates? || self.can_modify_org_details? end @@ -223,7 +223,7 @@ return org_type end =end - + ## # removes the api_token from the user # modifies the user model @@ -254,11 +254,11 @@ # -------------------------------------------------------------- def self.from_omniauth(auth) scheme = IdentifierScheme.find_by(name: auth.provider.downcase) - + if scheme.nil? throw Exception.new('Unknown OAuth provider: ' + auth.provider) else - joins(:user_identifiers).where('user_identifiers.identifier': auth.uid, + joins(:user_identifiers).where('user_identifiers.identifier': auth.uid, 'user_identifiers.identifier_scheme_id': scheme.id).first end end @@ -269,7 +269,14 @@ def deliver_invitation(options = {}) super(options.merge(subject: _('A Data Management Plan in %{application_name} has been shared with you') % {application_name: Rails.configuration.branding[:application][:name]})) end - + ## + # Case insensitive search over User model + # @param field [string] The name of the field being queried + # @param val [string] The string to search for, case insensitive + # @return [ActiveRecord::Relation] The result of the search + def self.where_case_insensitive(field, val) + User.where("lower(#{field}) = ?", val.downcase) + end # TODO: Remove this, its never called. # this generates a reset password link for a given user @@ -278,12 +285,12 @@ =begin def reset_password_link raw, enc = Devise.token_generator.generate(self.class, :reset_password_token) - self.reset_password_token = enc + self.reset_password_token = enc self.reset_password_sent_at = Time.now.utc save(validate: false) edit_user_password_path + '?reset_password_token=' + raw end =end - + end 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/app/views/templates/admin_index.html.erb b/app/views/templates/admin_index.html.erb index 2acb25a..7532bec 100644 --- a/app/views/templates/admin_index.html.erb +++ b/app/views/templates/admin_index.html.erb @@ -46,10 +46,10 @@ <% if hash[:live].nil? %> <%= _('Unpublished') %> - + <% elsif hash[:current].dirty? %> <%= _('Unpublished changes') %> - + <% else %> <%= _('Published') %> <% end %> @@ -58,7 +58,7 @@ <% last_temp_updated = hash[:current].updated_at %> <%= l last_temp_updated.to_date, formats: :short %> - + <%= link_to _('Edit'), admin_template_template_path(id: hash[:current].id, edit: "true"), class: "dmp_table_link" %> <%= link_to _('History'), admin_template_history_template_path(id: hash[:current].id), class: "dmp_table_link" %> @@ -119,7 +119,7 @@ <% elsif hash[:live].nil? %> <%= b_label = _('Un-published') %> - <% elsif !hash[:current].published? %> + <% elsif hash[:current].dirty? %> <%= _('You have un-published changes') %> <% else %> <%= _('Published') %> @@ -142,14 +142,14 @@ <% b_label = _('Edit customisation') %> <%= link_to b_label, admin_template_template_path(hash[:current]), class: "dmp_table_link" %> <% end %> - + <% end %> - <% if !hash[:current].customization_of.nil? %> + <% if !hash[:current].customization_of.nil? && !hash[:stale] %> <% if hash[:live].nil? || hash[:current].dirty? %> <%= link_to _('Publish'), admin_publish_template_path(hash[:current]), method: :put, class: "dmp_table_link" %> <% end %> - <% if !hash[:live].nil? %> + <% if hash[:live].present? %> <%= link_to _('Unpublish'), admin_unpublish_template_path(hash[:current]), method: :put, class: "dmp_table_link" %> <% end %> <% end %> diff --git a/app/views/templates/admin_template.html.erb b/app/views/templates/admin_template.html.erb index 75564de..fc7cbfe 100644 --- a/app/views/templates/admin_template.html.erb +++ b/app/views/templates/admin_template.html.erb @@ -36,7 +36,7 @@ <%= render partial: 'templates/show_phases_sections', locals: {phase: phase[:data], phase_hash: phase, template: @template, current: @current} %> <% end %> <% else %> - <% (1..@hash[:template][:phases].length).each do |phase_no| %> + <% @hash[:template][:phases].keys.sort.each do |phase_no| %> <% phase = @hash[:template][:phases][phase_no] %>
diff --git a/db/migrate/20170702012742_ensure_indexes_in_place.rb b/db/migrate/20170702012742_ensure_indexes_in_place.rb index 6bd9410..367ce5f 100644 --- a/db/migrate/20170702012742_ensure_indexes_in_place.rb +++ b/db/migrate/20170702012742_ensure_indexes_in_place.rb @@ -1,8 +1,12 @@ class EnsureIndexesInPlace < ActiveRecord::Migration def change #users_perms + remove_foreign_key :users_perms, :perms + remove_foreign_key :users_perms, :users remove_index :users_perms, name: 'index_users_perms_on_user_id_and_perm_id' add_index :users_perms, :user_id + add_foreign_key :users_perms, :perms + add_foreign_key :users_perms, :users #user_identifiers add_index :user_identifiers, :user_id #roles @@ -27,14 +31,22 @@ #annotations add_index :annotations, :question_id #question_themes + remove_foreign_key :questions_themes, :questions + remove_foreign_key :questions_themes, :themes remove_index :questions_themes, name: 'question_theme_index' remove_index :questions_themes, name: 'theme_question_index' add_index :questions_themes, :question_id + add_foreign_key :questions_themes, :questions + add_foreign_key :questions_themes, :themes #question_options add_index :question_options, :question_id #answers_question_options + remove_foreign_key :answers_question_options, :answers + remove_foreign_key :answers_question_options, :question_options remove_index :answers_question_options, name: 'answer_question_option_index' remove_index :answers_question_options, name: 'question_option_answer_index' add_index :answers_question_options, :answer_id + add_foreign_key :answers_question_options, :answers + add_foreign_key :answers_question_options, :question_options end end diff --git a/db/seeds.rb b/db/seeds.rb index 6c4aded..1c6d553 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -8,7 +8,7 @@ {name: 'orcid', description: 'ORCID', active: true, logo_url:'http://orcid.org/sites/default/files/images/orcid_16x16.png', user_landing_url:'https://orcid.org' }, - {name: 'shibboleth', description: 'your institutional credentials', active: true, + {name: 'shibboleth', description: 'Your institutional credentials', active: true, }, ] identifier_schemes.map{ |is| IdentifierScheme.create!(is) if IdentifierScheme.find_by(name: is[:name]).nil? } @@ -16,13 +16,13 @@ # Question Formats # ------------------------------------------------------- question_formats = [ - {title: "Text area", option_based: false}, - {title: "Text field", option_based: false}, - {title: "Radio buttons", option_based: true}, - {title: "Check box", option_based: true}, - {title: "Dropdown", option_based: true}, - {title: "Multi select box", option_based: true}, - {title: "Date", option_based: false} + {title: "Text area", option_based: false, formattype: 0}, + {title: "Text field", option_based: false, formattype: 1}, + {title: "Radio buttons", option_based: true, formattype: 2}, + {title: "Check box", option_based: true, formattype: 3}, + {title: "Dropdown", option_based: true, formattype: 4}, + {title: "Multi select box", option_based: true, formattype: 5}, + {title: "Date", option_based: true, formattype: 6} ] question_formats.map{ |qf| QuestionFormat.create!(qf) if QuestionFormat.find_by(title: qf[:title]).nil? } @@ -154,7 +154,9 @@ # ------------------------------------------------------- token_permission_types = [ {token_type: 'guidances', text_description: 'allows a user access to the guidances api endpoint'}, - {token_type: 'plans', text_description: 'allows a user access to the plans api endpoint'} + {token_type: 'plans', text_description: 'allows a user access to the plans api endpoint'}, + {token_type: 'templates', text_description: 'allows a user access to the templates api endpoint'}, + {token_type: 'statistics', text_description: 'allows a user access to the statistics api endpoint'} ] token_permission_types.map{ |tpt| TokenPermissionType.create!(tpt) if TokenPermissionType.find_by(token_type: tpt[:token_type]).nil? } @@ -366,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/lib/assets/stylesheets/bootstrap_and_overrides.css.less b/lib/assets/stylesheets/bootstrap_and_overrides.css.less index 11fcd58..12f5905 100644 --- a/lib/assets/stylesheets/bootstrap_and_overrides.css.less +++ b/lib/assets/stylesheets/bootstrap_and_overrides.css.less @@ -175,13 +175,14 @@ margin-right: 0px; } -#main-nav-tabs li.active a, #main-nav-tabs li.active a:active, #main-nav-tabs li a:hover, lang-dropdown-link:hover{ +#main-nav-tabs li.active a, #main-nav-tabs li.active a:active, #main-nav-tabs li a:hover, #main-nav-tabs .main_nav li .lang-dropdown-link:hover{ background-color: @orange_colour; color: @white_colour !important; font-weight: bold; border: 1px solid @orange_colour; padding: 4px 15px 4px 15px; text-decoration: none !important; + background-image: none; } .caret-orange { @@ -197,7 +198,7 @@ border-bottom-color: white!important; } -.lang-dropdown-link { +#main-nav-tabs .main_nav li .lang-dropdown-link { margin-bottom: 0!important; } diff --git a/lib/tasks/bugfix.rake b/lib/tasks/bugfix.rake new file mode 100644 index 0000000..5fc6bf1 --- /dev/null +++ b/lib/tasks/bugfix.rake @@ -0,0 +1,49 @@ +namespace :bugfix do + + desc "Bug fixes for version v0.3.3" + task v0_3_3: :environment do + Rake::Task['bugfix:fix_question_formats'].execute + Rake::Task['bugfix:add_missing_token_permission_types'].execute + end + + desc "Add the missing formattype to the question_formats table" + task fix_question_formats: :environment do + QuestionFormat.all.each do |qf| + case qf.title.downcase + when 'text area' + qf.formattype = :textarea + when 'text field' + qf.formattype = :textfield + when 'radio buttons' + qf.formattype = :radiobuttons + when 'check box' + qf.formattype = :checkbox + when 'dropdown' + qf.formattype = :dropdown + when 'multi select box' + qf.formattype = :multiselectbox + when 'date' + qf.formattype = :date + end + + qf.save! + end + + if QuestionFormat.find_by(formattype: :date).nil? + QuestionFormat.create!({title: "Date", option_based: true, formattype: 6}) + end + end + + desc "Add the missing token_permission_types" + task add_missing_token_permission_types: :environment do + if TokenPermissionType.find_by(token_type: 'templates').nil? + TokenPermissionType.create!({token_type: 'templates', + text_description: 'allows a user access to the templates api endpoint'}) + end + if TokenPermissionType.find_by(token_type: 'statistics').nil? + TokenPermissionType.create!({token_type: 'statistics', + text_description: 'allows a user access to the statistics api endpoint'}) + end + end + +end \ No newline at end of file diff --git a/lib/tasks/migrate.rake b/lib/tasks/migrate.rake index 7bfa27e..f070809 100644 --- a/lib/tasks/migrate.rake +++ b/lib/tasks/migrate.rake @@ -258,8 +258,11 @@ task remove_duplicate_annotations: :environment do questions = Question.joins(:annotations).group("questions.id").having("count(annotations.id) > count(DISTINCT annotations.text)") questions.each do |q| + # store already de-duplicated id's so we dont remove them in later iterations + removed = [] q.annotations.each do |a| - conflicts = Annotation.where(question_id: a.question_id, text: a.text).where.not(id: a.id) + removed << a.id + conflicts = Annotation.where(question_id: a.question_id, text: a.text).where.not(id: removed) conflicts.each {|c| c.destroy } end end diff --git a/test/functional/roles_controller_test.rb b/test/functional/roles_controller_test.rb index 7937867..9f2d7ce 100644 --- a/test/functional/roles_controller_test.rb +++ b/test/functional/roles_controller_test.rb @@ -7,10 +7,10 @@ setup do scaffold_plan scaffold_org_admin(@plan.template.org) - + # This should NOT be unnecessary! Owner should have full access @plan.roles << Role.create(user: @user, plan: @plan, access: 15) - + end # TODO: Cleanup routes for this one. The controller currently only responds to create, update, destroy @@ -21,19 +21,19 @@ # role PATCH /roles/:id roles#update # PUT /roles/:id roles#update # DELETE /roles/:id roles#destroy - + # POST /roles (roles_path) # ---------------------------------------------------------- test "create a new role" do params = {plan_id: @plan.id, access_level: 4} - + # Should redirect user to the root path if they are not logged in! post roles_path, {role: params} assert_unauthorized_redirect_to_root_path sign_in @user - + # Known user @invitee = User.where.not(id: [@plan.owner.id, @user.id]).first post roles_path, {user: @invitee.email, role: params} @@ -50,15 +50,15 @@ assert_redirected_to share_plan_path(@plan) assert_equal @invitee.id, Role.last.user_id, "expected no record to have been created!" assert assigns(:role) - + # Unknown user post roles_path, {user: 'unknown_user@org.org', role: params} - assert_equal _('Invitation to unknown_user@org.org issued successfully.'), flash[:notice] + assert_equal _('Invitation to unknown_user@org.org issued successfully. \nPlan shared with unknown_user@org.org.'), flash[:notice] assert_response :redirect assert_redirected_to share_plan_path(@plan) assert_equal User.find_by(email:'unknown_user@org.org').id, Role.last.user_id, "expected the record to have been created!" assert assigns(:role) - + # Invite owner @invitee = User.find_by(id: @plan.owner.id) post roles_path, {user: @invitee.email, role: params} @@ -67,26 +67,26 @@ assert_redirected_to share_plan_path(@plan) assert_not_equal @invitee.id, Role.last.user_id, "expected no record to have been created!" assert assigns(:role) - + # Missing email post roles_path, {role: {plan_id: @plan.id, access_level: 4}} assert_equal _('Please enter an email address'), flash[:notice] assert_response :redirect assert_redirected_to share_plan_path(@plan) assert assigns(:role) - end - + end + # PUT /role/:id (role_path) # ---------------------------------------------------------- test "update the role" do @invitee = User.last role = Role.create(user: @invitee, plan: @plan, access: 1) params = {access_level: 2} - + # Should redirect user to the root path if they are not logged in! put role_path(role), {role: params} assert_unauthorized_redirect_to_root_path - + sign_in @user # Valid save @@ -96,7 +96,7 @@ assert_redirected_to share_plan_path(@plan) assert assigns(:role) assert_equal 13, role.reload.access, "expected the record to have been updated" - + # TODO: Role should require a user, plan and an access level :/ # Invalid save # put role_path(role), {role: {user: nil}} @@ -105,26 +105,26 @@ # assert_redirected_to share_plan_path(@plan) # assert assigns(:role) end - + # DELETE /role/:id (role_path) # ---------------------------------------------------------- test "delete the section" do @invitee = User.last role = Role.create(user: @invitee, plan: @plan, access: 1) - + # Should redirect user to the root path if they are not logged in! delete role_path(role) assert_unauthorized_redirect_to_root_path - + sign_in @user - + delete role_path(role) assert_equal _('Access removed'), flash[:notice] assert_response :redirect assert_redirected_to share_plan_path(@plan) - assert_raise ActiveRecord::RecordNotFound do + assert_raise ActiveRecord::RecordNotFound do Role.find(role.id).nil? end end - + end diff --git a/test/functional/templates_controller_test.rb b/test/functional/templates_controller_test.rb index 0987259..78a4724 100644 --- a/test/functional/templates_controller_test.rb +++ b/test/functional/templates_controller_test.rb @@ -238,7 +238,7 @@ assert_equal 0, customization.version assert_not customization.published? - assert_not customization.dirty? + assert customization.dirty? # Make sure the funder templates data is not modifiable! customization.phases.each do |p| 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?("