From 94588e46987e387340cf9cf4034c9f3961c8fc8a Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Fri, 24 Jul 2020 17:00:42 +0200 Subject: [PATCH 01/30] Refactor task archive and move permissions [SCI-4817] --- app/controllers/canvas_controller.rb | 10 ++++----- app/controllers/my_modules_controller.rb | 6 +++--- app/models/experiment.rb | 6 ++---- app/permissions/experiment.rb | 12 ++++++++++- app/views/canvas/edit/_my_module.html.erb | 25 +++++++++++------------ 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/app/controllers/canvas_controller.rb b/app/controllers/canvas_controller.rb index 7eb546042..f1e94096e 100644 --- a/app/controllers/canvas_controller.rb +++ b/app/controllers/canvas_controller.rb @@ -35,7 +35,7 @@ class CanvasController < ApplicationController to_archive = update_params[:remove].split(',') if to_archive.all? do |id| is_int?(id) && - can_manage_module?(MyModule.find_by_id(id)) + can_archive_module?(MyModule.find_by(id: id)) end to_archive.collect!(&:to_i) else @@ -117,16 +117,14 @@ class CanvasController < ApplicationController # Okay, JSON parsed! unless to_move.is_a?(Hash) && to_move.keys.all? do |id| - id.is_a?(String) && - (!is_int?(id) || can_manage_module?(MyModule.find_by_id(id))) + id.is_a?(String) && can_move_module?(MyModule.find_by(id: id)) end && to_move.values.all? do |exp_id| - exp_id.is_a?(String) && - can_manage_experiment?(Experiment.find_by_id(exp_id)) + exp_id.is_a?(String) && can_manage_experiment?(Experiment.find_by(id: exp_id)) end return render_403 end - rescue + rescue StandardError return render_403 end end diff --git a/app/controllers/my_modules_controller.rb b/app/controllers/my_modules_controller.rb index ded8bbe71..8b814ed0f 100644 --- a/app/controllers/my_modules_controller.rb +++ b/app/controllers/my_modules_controller.rb @@ -9,7 +9,7 @@ class MyModulesController < ApplicationController before_action :load_vars before_action :load_projects_tree, only: %i(protocols results activities archive) - before_action :check_manage_permissions_archive, only: %i(update) + before_action :check_archive_and_restore_permissions, only: %i(update) before_action :check_manage_permissions, only: %i(description due_date update_description update_protocol_description) before_action :check_view_permissions, except: %i(update update_description update_protocol_description toggle_task_state) @@ -374,11 +374,11 @@ class MyModulesController < ApplicationController render_403 && return unless can_manage_module?(@my_module) end - def check_manage_permissions_archive + def check_archive_and_restore_permissions render_403 && return unless if my_module_params[:archived] == 'false' can_restore_module?(@my_module) else - can_manage_module?(@my_module) + can_archive_module?(@my_module) end end diff --git a/app/models/experiment.rb b/app/models/experiment.rb index 58bb7c899..4b8a3e13b 100644 --- a/app/models/experiment.rb +++ b/app/models/experiment.rb @@ -138,13 +138,11 @@ class Experiment < ApplicationRecord archive_modules(to_archive, current_user) if to_archive.any? # Update only existing tasks positions to release positions for new tasks - existing_positions = positions - .slice(*positions.keys.map { |k| k unless k.to_s.start_with?('n') }.compact) + existing_positions = positions.slice(*positions.keys.map { |k| k unless k.to_s.start_with?('n') }.compact) update_module_positions(existing_positions) if existing_positions.any? # Move only existing tasks to release positions for new tasks - existing_to_move = to_move - .slice(*to_move.keys.map { |k| k unless k.to_s.start_with?('n') }.compact) + existing_to_move = to_move.slice(*to_move.keys.map { |k| k unless k.to_s.start_with?('n') }.compact) move_modules(existing_to_move, current_user) if existing_to_move.any? # add new modules diff --git a/app/permissions/experiment.rb b/app/permissions/experiment.rb index ec4909f27..f31c56241 100644 --- a/app/permissions/experiment.rb +++ b/app/permissions/experiment.rb @@ -72,12 +72,17 @@ Canaid::Permissions.register_for(MyModule) do end end - # module: update, archive, move + # module: update # result: create, update can :manage_module do |user, my_module| can_manage_experiment?(user, my_module.experiment) end + # module: archive + can :archive_module do |user, my_module| + can_manage_experiment?(user, my_module.experiment) + end + # NOTE: Must not be dependent on canaid parmision for which we check if it's # active # module: restore @@ -86,6 +91,11 @@ Canaid::Permissions.register_for(MyModule) do my_module.archived? end + # module: move + can :move_module do |user, my_module| + can_manage_experiment?(user, my_module.experiment) + end + # module: assign/reassign/unassign users can :manage_users_in_module do |user, my_module| user.is_owner_of_project?(my_module.experiment.project) diff --git a/app/views/canvas/edit/_my_module.html.erb b/app/views/canvas/edit/_my_module.html.erb index c4a17c6b2..228a8a405 100644 --- a/app/views/canvas/edit/_my_module.html.erb +++ b/app/views/canvas/edit/_my_module.html.erb @@ -7,7 +7,6 @@ data-module-conns="<%= construct_module_connections(my_module) %>"> <% module_group = my_module.my_module_group %> - <% can_manage_module_group = module_group && (module_group.new_record? || module_group.my_modules.all? { |my_module| can_manage_module?(my_module) }) %>
@@ -21,35 +20,35 @@
  • <%= t('projects.index.options_header') %>
    • <% if can_manage_module?(my_module) %>
  • - <%=t "experiments.canvas.edit.edit_module" %> + <%= t('experiments.canvas.edit.edit_module') %>
    • <% end %> <% if can_manage_experiment?(my_module.experiment) %>
  • - <%=t "experiments.canvas.edit.clone_module" %> + <%= t('experiments.canvas.edit.clone_module') %>
  • > - <%=t "experiments.canvas.edit.clone_module_group" %> + <%= t('experiments.canvas.edit.clone_module_group') %>
    • <% end %> - <% if can_manage_module?(my_module) %> + <% if can_move_module?(my_module) %>
  • - <%=t "experiments.canvas.edit.move_module" %> + <%= t('experiments.canvas.edit.move_module') %>
    • <% end %> - <% if can_manage_module_group %> + <% if module_group.my_modules.all? { |my_module| can_move_module?(my_module) } %>
  • - <%=t "experiments.canvas.edit.move_module_group" %> + <%= t('experiments.canvas.edit.move_module_group') %>
    • <% end %> - <% if can_manage_module?(my_module) %> + <% if can_archive_module?(my_module) %>
  • - <%=t "experiments.canvas.edit.delete_module" %> + <%= t('experiments.canvas.edit.delete_module') %>
    • <% end %> - <% if can_manage_module_group %> + <% if module_group.my_modules.all? { |my_module| can_archive_module?(my_module) } %>
  • - <%=t "experiments.canvas.edit.delete_module_group" %> + <%= t('experiments.canvas.edit.delete_module_group') %>
    • <% end %> @@ -59,7 +58,7 @@ <% if can_manage_experiment?(my_module.experiment) %>
    - <%=t "experiments.canvas.edit.drag_connections" %> + <%= t('experiments.canvas.edit.drag_connections') %>
    <% end %> From 7bedbd3511c74eae3ef977620b957378806b1677 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Fri, 24 Jul 2020 18:38:53 +0200 Subject: [PATCH 02/30] Fix canvas update method [SCI-4817] --- app/controllers/canvas_controller.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/controllers/canvas_controller.rb b/app/controllers/canvas_controller.rb index f1e94096e..d5d95e540 100644 --- a/app/controllers/canvas_controller.rb +++ b/app/controllers/canvas_controller.rb @@ -33,10 +33,7 @@ class CanvasController < ApplicationController to_archive = [] if update_params[:remove].present? to_archive = update_params[:remove].split(',') - if to_archive.all? do |id| - is_int?(id) && - can_archive_module?(MyModule.find_by(id: id)) - end + if to_archive.all? { |id| can_archive_module?(MyModule.find_by(id: id)) } to_archive.collect!(&:to_i) else return render_403 @@ -117,10 +114,10 @@ class CanvasController < ApplicationController # Okay, JSON parsed! unless to_move.is_a?(Hash) && to_move.keys.all? do |id| - id.is_a?(String) && can_move_module?(MyModule.find_by(id: id)) + !is_int?(id) || can_move_module?(MyModule.find_by(id: id)) end && to_move.values.all? do |exp_id| - exp_id.is_a?(String) && can_manage_experiment?(Experiment.find_by(id: exp_id)) + can_manage_experiment?(Experiment.find_by(id: exp_id)) end return render_403 end From ce8d12e1c4a5a96eda100613f6c47af5305413e0 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Wed, 5 Aug 2020 09:59:58 +0200 Subject: [PATCH 03/30] Create new activities for exporting assigned items (live & snapshots) [SCI-4881] --- .../my_module_repositories_controller.rb | 21 +++++++++++------- ..._module_repository_snapshots_controller.rb | 22 +++++++++++++++++++ app/helpers/global_activities_helper.rb | 2 +- app/services/repository_zip_export.rb | 11 +++++----- .../_full_view_snapshot_table.html.erb | 2 +- config/initializers/extends.rb | 9 ++++---- config/locales/global_activities/en.yml | 4 ++++ config/routes.rb | 1 + 8 files changed, 53 insertions(+), 19 deletions(-) diff --git a/app/controllers/my_module_repositories_controller.rb b/app/controllers/my_module_repositories_controller.rb index 54add8ab1..7ecc50060 100644 --- a/app/controllers/my_module_repositories_controller.rb +++ b/app/controllers/my_module_repositories_controller.rb @@ -4,8 +4,7 @@ class MyModuleRepositoriesController < ApplicationController include ApplicationHelper before_action :load_my_module - before_action :load_repository, except: %i(repositories_dropdown_list repositories_list_html export_repository) - before_action :load_repository_or_snapshot, only: :export_repository + before_action :load_repository, except: %i(repositories_dropdown_list repositories_list_html) before_action :check_my_module_view_permissions before_action :check_repository_view_permissions, except: %i(repositories_dropdown_list repositories_list_html) before_action :check_assign_repository_records_permissions, only: :update @@ -120,6 +119,18 @@ class MyModuleRepositoriesController < ApplicationController def export_repository if params[:header_ids] RepositoryZipExport.generate_zip(params, @repository, current_user) + + Activities::CreateActivityService.call( + activity_type: :export_inventory_items_assigned_to_task, + owner: current_user, + subject: @repository, + team: current_team, + message_items: { + my_module: @my_module.id, + repository: @repository.id + } + ) + render json: { message: t('zip_export.export_request_success') }, status: :ok else render json: { message: t('zip_export.export_error') }, status: :unprocessable_entity @@ -138,12 +149,6 @@ class MyModuleRepositoriesController < ApplicationController render_404 unless @repository end - def load_repository_or_snapshot - @repository = Repository.accessible_by_teams(current_team).find_by(id: params[:id]) - @repository ||= RepositorySnapshot.find_by(id: params[:id]) - render_404 unless @repository - end - def check_my_module_view_permissions render_403 unless can_read_experiment?(@my_module.experiment) end diff --git a/app/controllers/my_module_repository_snapshots_controller.rb b/app/controllers/my_module_repository_snapshots_controller.rb index 4493b2343..379ed0155 100644 --- a/app/controllers/my_module_repository_snapshots_controller.rb +++ b/app/controllers/my_module_repository_snapshots_controller.rb @@ -101,6 +101,28 @@ class MyModuleRepositorySnapshotsController < ApplicationController render json: {} end + def export_repository_snapshot + if params[:header_ids] + RepositoryZipExport.generate_zip(params, @repository_snapshot, current_user) + + Activities::CreateActivityService.call( + activity_type: :export_inventory_snapshot_items_assigned_to_task, + owner: current_user, + subject: @repository_snapshot, + team: current_team, + message_items: { + my_module: @my_module.id, + repository_snapshot: @repository_snapshot.id, + created_at: @repository_snapshot.created_at + } + ) + + render json: { message: t('zip_export.export_request_success') }, status: :ok + else + render json: { message: t('zip_export.export_error') }, status: :unprocessable_entity + end + end + private def load_my_module diff --git a/app/helpers/global_activities_helper.rb b/app/helpers/global_activities_helper.rb index e9e4f374d..7d34a02dd 100644 --- a/app/helpers/global_activities_helper.rb +++ b/app/helpers/global_activities_helper.rb @@ -12,7 +12,7 @@ module GlobalActivitiesHelper if value.is_a? String value elsif value['type'] == 'Time' # use saved date for printing - l(Time.at(value['value']), format: :full_date) + l(Time.at(value['value']), format: :full) else no_links ? generate_name(value) : generate_link(value, activity) end diff --git a/app/services/repository_zip_export.rb b/app/services/repository_zip_export.rb index 2fe9c4632..13cc0ea91 100644 --- a/app/services/repository_zip_export.rb +++ b/app/services/repository_zip_export.rb @@ -7,11 +7,12 @@ module RepositoryZipExport # Fetch rows in the same order as in the currently viewed datatable if params[:my_module_id] rows = if repository.is_a?(RepositorySnapshot) - repository.repository_rows - else - repository.repository_rows.joins(:my_module_repository_rows) - .where(my_module_repository_rows: { my_module_id: params[:my_module_id] }) - end + repository.repository_rows + else + repository.repository_rows + .joins(:my_module_repository_rows) + .where(my_module_repository_rows: { my_module_id: params[:my_module_id] }) + end else ordered_row_ids = params[:row_ids] id_row_map = RepositoryRow.where(id: ordered_row_ids, diff --git a/app/views/my_modules/repositories/_full_view_snapshot_table.html.erb b/app/views/my_modules/repositories/_full_view_snapshot_table.html.erb index e95245d03..0df8097c9 100644 --- a/app/views/my_modules/repositories/_full_view_snapshot_table.html.erb +++ b/app/views/my_modules/repositories/_full_view_snapshot_table.html.erb @@ -8,7 +8,7 @@ data-default-order="<%= default_snapshot_table_order_as_js_array %>" data-default-table-columns="<%= default_snapshot_table_columns %>" data-load-state-url="<%= repository_load_table_state_path(@repository_snapshot) %>" - data-export-url="<%= export_repository_my_module_repository_path(@my_module ,@repository_snapshot) %>" + data-export-url="<%= export_repository_snapshot_my_module_repository_snapshot_path(@my_module, @repository_snapshot) %>" data-versions-sidebar-url="<%= full_view_sidebar_my_module_repository_snapshots_path(@my_module, @repository_snapshot.parent_id) %>" > diff --git a/config/initializers/extends.rb b/config/initializers/extends.rb index e2fac7942..0caf096d8 100644 --- a/config/initializers/extends.rb +++ b/config/initializers/extends.rb @@ -143,9 +143,8 @@ class Extends step: nil } - ACTIVITY_MESSAGE_ITEMS_TYPES = - ACTIVITY_SUBJECT_TYPES + %w(User Tag RepositoryColumn RepositoryRow Step Asset TinyMceAsset Repository) - .freeze + ACTIVITY_MESSAGE_ITEMS_TYPES = ACTIVITY_SUBJECT_TYPES + %w(User Tag RepositoryColumn RepositoryRow Step + Asset TinyMceAsset Repository RepositorySnapshot).freeze ACTIVITY_TYPES = { create_project: 0, @@ -287,7 +286,9 @@ class Extends archive_inventory_item: 142, restore_inventory_item: 143, archive_inventory: 144, - restore_inventory: 145 + restore_inventory: 145, + export_inventory_items_assigned_to_task: 146, + export_inventory_snapshot_items_assigned_to_task: 147 } ACTIVITY_GROUPS = { diff --git a/config/locales/global_activities/en.yml b/config/locales/global_activities/en.yml index 24444f933..0ab5de4c1 100644 --- a/config/locales/global_activities/en.yml +++ b/config/locales/global_activities/en.yml @@ -173,6 +173,8 @@ en: delete_chemical_structure_on_protocol_html: "%{user} deleted chemical structure %{asset_name} on protocol %{protocol}." delete_chemical_structure_on_task_html: "%{user} deleted chemical structure %{asset_name} on task %{my_module}." protocol_description_in_task_edited_html: "%{user} edited protocol description on task %{my_module}." + export_inventory_items_assigned_to_task_html: "%{user} exported inventory item(s) assigned to task %{my_module} from inventory %{repository}: Live version." + export_inventory_snapshot_items_assigned_to_task_html: "%{user} exported inventory item(s) assigned to task %{my_module} from inventory %{repository_snapshot}: Snapshot of %{created_at}." activity_name: create_project: "Project created" @@ -311,6 +313,8 @@ en: delete_chemical_structure_on_protocol: "Chemical structure on protocol deleted" delete_chemical_structure_on_task: "Chemical structure on task deleted" protocol_description_in_task_edited: "Protocol description in task edited" + export_inventory_items_assigned_to_task: "Task-assigned inventory items exported (live version)" + export_inventory_snapshot_items_assigned_to_task: "Task-assigned inventory items exported (snapshot)" activity_group: projects: "Projects" diff --git a/config/routes.rb b/config/routes.rb index 50c39fae5..a2f5ec3cb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -395,6 +395,7 @@ Rails.application.routes.draw do member do get :full_view_table post :index_dt + post :export_repository_snapshot get :status end From 6a50f7f27e53fea40d1338a48893fe09d0da52be Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Wed, 5 Aug 2020 13:15:38 +0200 Subject: [PATCH 04/30] Improve handling of old repository table states [SCI-4889] --- .../repository_table_state_column_update_service.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/services/repository_table_state_column_update_service.rb b/app/services/repository_table_state_column_update_service.rb index e32b0acee..57d5c25a4 100644 --- a/app/services/repository_table_state_column_update_service.rb +++ b/app/services/repository_table_state_column_update_service.rb @@ -44,18 +44,18 @@ class RepositoryTableStateColumnUpdateService end end - if state.dig('order', 0, 0) == old_column_index + if state.dig('order', 0, 0).to_i == old_column_index # Fallback to default order if user had table ordered by # the deleted column state['order'] = Constants::REPOSITORY_TABLE_DEFAULT_STATE['order'] - elsif state.dig('order', 0, 0) > old_column_index + elsif state.dig('order', 0, 0).to_i > old_column_index state['order'][0][0] -= 1 end state['length'] = (state['length'] - 1) state['time'] = (Time.now.to_f * 1_000).to_i table_state.save - rescue NoMethodError => e + rescue StandardError => e Rails.logger.error e.message RepositoryTableStateService.new(user, repository).create_default_state end From 3a8fc01ff72c176b88b150c77f1e359aececaa8b Mon Sep 17 00:00:00 2001 From: Mojca Lorber Date: Wed, 5 Aug 2020 15:43:43 +0200 Subject: [PATCH 05/30] Fix inventory card view for snapshots as default view --- app/controllers/repository_rows_controller.rb | 13 ++- .../_repository_row_info_modal.html.erb | 98 +++++++++---------- 2 files changed, 57 insertions(+), 54 deletions(-) diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index d7242864e..6289696f9 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -4,9 +4,9 @@ class RepositoryRowsController < ApplicationController include ApplicationHelper include MyModulesHelper - before_action :load_repository - before_action :load_repository_row, only: %i(update show assigned_task_list) - before_action :check_read_permissions, except: %i(create update delete_records copy_records) + before_action :load_repository, except: :show + before_action :load_repository_row, only: %i(update assigned_task_list) + before_action :check_read_permissions, except: %i(show create update delete_records copy_records) before_action :check_snapshotting_status, only: %i(create update delete_records copy_records) before_action :check_create_permissions, only: :create before_action :check_delete_permissions, only: %i(delete_records archive_records restore_records) @@ -51,7 +51,12 @@ class RepositoryRowsController < ApplicationController end def show - @assigned_modules = @repository_row.my_modules.joins(experiment: :project) + @repository_row = RepositoryRow.find_by(id: params[:id]) + render_403 unless can_read_repository?(@repository_row.repository) + + row = RepositoryRow.find_by(id: @repository_row.parent_id) if @repository_row.parent_id + row ||= @repository_row + @assigned_modules = row.my_modules.joins(experiment: :project) @viewable_modules = @assigned_modules.viewable_by_user(current_user, current_user.teams) @private_modules = @assigned_modules - @viewable_modules diff --git a/app/views/repositories/_repository_row_info_modal.html.erb b/app/views/repositories/_repository_row_info_modal.html.erb index ce9ed0173..cd773eaef 100644 --- a/app/views/repositories/_repository_row_info_modal.html.erb +++ b/app/views/repositories/_repository_row_info_modal.html.erb @@ -39,58 +39,56 @@ <% end %>

    - <% if @repository_row.repository.is_a?(Repository) %> - <% if @assigned_modules.size.positive? %> -
    - <%= t('repository_row.modal_info.title', nr: @assigned_modules.size) %> - <%= t('repository_row.modal_info.private_tasks', nr: @private_modules.size) if @private_modules.size.positive? %> -
    - <% if @viewable_modules.size.positive? %> -
    - - - <% @viewable_modules.each do |my_module| %> - - - - <% end %> - - - - + <% if @assigned_modules.size.positive? %> +
    + <%= t('repository_row.modal_info.title', nr: @assigned_modules.size) %> + <%= t('repository_row.modal_info.private_tasks', nr: @private_modules.size) if @private_modules.size.positive? %> +
    + <% if @viewable_modules.size.positive? %> +
    +
    + + <% @viewable_modules.each do |my_module| %> + + - - -
    - <% end %> - <% else %> - <%= t('repository_row.modal_info.no_tasks') %> + <% end %> + + + + + + + +
    <% end %> + <% else %> + <%= t('repository_row.modal_info.no_tasks') %> <% end %> From bdf1e62f601b77d3edfae163d3ceba448a89380a Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Wed, 5 Aug 2020 16:14:43 +0200 Subject: [PATCH 06/30] Fix word reports when using headings --- app/services/reports/docx/private_methods.rb | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/app/services/reports/docx/private_methods.rb b/app/services/reports/docx/private_methods.rb index 9b025459b..68d7a2b5d 100644 --- a/app/services/reports/docx/private_methods.rb +++ b/app/services/reports/docx/private_methods.rb @@ -19,12 +19,17 @@ module Reports::Docx::PrivateMethods tiny_mce_table(elem[:data]) elsif elem[:type] == 'newline' style = elem[:style] || {} - @docx.p elem[:value] do - align style[:align] - color style[:color] - bold style[:bold] - italic style[:italic] - style style[:style] if style[:style] + # print heading if its heading + # Mixing heading with other style setting causes problems for Word + if %w(h1 h2 h3 h4 h5).include?(style[:style]) + @docx.public_send(style[:style], elem[:value]) + else + @docx.p elem[:value] do + align style[:align] + color style[:color] + bold style[:bold] + italic style[:italic] + end end elsif elem[:type] == 'image' Reports::Docx.render_img_element(@docx, elem) From 04c294adb1677c3fb324f9c37a31ec6859f84bf1 Mon Sep 17 00:00:00 2001 From: Miha Mencin Date: Thu, 6 Aug 2020 13:23:40 +0200 Subject: [PATCH 07/30] Fix the failing test: repository row should be in correct repository --- app/controllers/repository_rows_controller.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index 6289696f9..a2dfeb280 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -53,10 +53,9 @@ class RepositoryRowsController < ApplicationController def show @repository_row = RepositoryRow.find_by(id: params[:id]) render_403 unless can_read_repository?(@repository_row.repository) - - row = RepositoryRow.find_by(id: @repository_row.parent_id) if @repository_row.parent_id - row ||= @repository_row - @assigned_modules = row.my_modules.joins(experiment: :project) + render_403 unless @repository_row.repository_id == params[:repository_id] + + @assigned_modules = @repository_row.my_modules.joins(experiment: :project) @viewable_modules = @assigned_modules.viewable_by_user(current_user, current_user.teams) @private_modules = @assigned_modules - @viewable_modules From ae013b6fa8c5ede01d3a52215c5060b91decb75f Mon Sep 17 00:00:00 2001 From: Miha Mencin Date: Thu, 6 Aug 2020 14:06:23 +0200 Subject: [PATCH 08/30] join two renders --- app/controllers/repository_rows_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index a2dfeb280..688fdea53 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -52,8 +52,7 @@ class RepositoryRowsController < ApplicationController def show @repository_row = RepositoryRow.find_by(id: params[:id]) - render_403 unless can_read_repository?(@repository_row.repository) - render_403 unless @repository_row.repository_id == params[:repository_id] + render_403 if !can_read_repository?(@repository_row.repository) || @repository_row.repository_id != params[:repository_id] @assigned_modules = @repository_row.my_modules.joins(experiment: :project) @viewable_modules = @assigned_modules.viewable_by_user(current_user, current_user.teams) From 3c3a2bfb8bf63016a9331a5ea7a4eb2f82b72438 Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Thu, 6 Aug 2020 15:18:20 +0200 Subject: [PATCH 09/30] Add check if user is locked, show link unlock instructions --- app/views/users/shared/_links.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/users/shared/_links.html.erb b/app/views/users/shared/_links.html.erb index 11b16ffd5..1d520d0f6 100644 --- a/app/views/users/shared/_links.html.erb +++ b/app/views/users/shared/_links.html.erb @@ -17,7 +17,7 @@ <%= link_to t("devise.links.not_receive_confirmation"), new_confirmation_path(resource_name) %>
    <% end -%> - <%- if devise_mapping.lockable? && resource_class.unlock_strategy_enabled?(:email) && controller_name != 'unlocks' %> + <%- if devise_mapping.lockable? && resource_class.unlock_strategy_enabled?(:email) && controller_name != 'unlocks' && flash[:alert] == t('devise.failure.locked') %> <%= link_to t("devise.links.not_receive_unlock"), new_unlock_path(resource_name) %>
    <% end -%> From 3d3fd1625c2437b2017bd8ffd08bd40ee8547f8f Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Fri, 7 Aug 2020 14:54:05 +0200 Subject: [PATCH 10/30] Add created_at and user to API Step serializer [SCI-4911] --- app/controllers/api/v1/steps_controller.rb | 2 +- app/serializers/api/v1/step_serializer.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v1/steps_controller.rb b/app/controllers/api/v1/steps_controller.rb index 367ad09e4..b21265825 100644 --- a/app/controllers/api/v1/steps_controller.rb +++ b/app/controllers/api/v1/steps_controller.rb @@ -59,7 +59,7 @@ module Api end def permitted_includes - %w(tables assets checklists checklists.checklist_items comments) + %w(tables assets checklists checklists.checklist_items comments user) end def load_step_for_managing diff --git a/app/serializers/api/v1/step_serializer.rb b/app/serializers/api/v1/step_serializer.rb index 9ed994531..d79485f2f 100644 --- a/app/serializers/api/v1/step_serializer.rb +++ b/app/serializers/api/v1/step_serializer.rb @@ -8,8 +8,9 @@ module Api include InputSanitizeHelper type :steps - attributes :id, :name, :description, :position, :completed + attributes :id, :name, :description, :created_at, :position, :completed attribute :completed_on, if: -> { object.completed? } + belongs_to :user, serializer: UserSerializer belongs_to :protocol, serializer: ProtocolSerializer has_many :assets, serializer: AssetSerializer has_many :checklists, serializer: ChecklistSerializer From 91d9f5d6ee18a29426cc486196f02459ecdd0ba9 Mon Sep 17 00:00:00 2001 From: Oleksii Kriuchykhin Date: Fri, 7 Aug 2020 16:51:57 +0200 Subject: [PATCH 11/30] Fix rendering of RTE in API [SCI-4910] --- .../api/v1/protocols_controller.rb | 21 +++++++++++-------- app/controllers/api/v1/steps_controller.rb | 6 ++++-- app/controllers/api/v1/tasks_controller.rb | 4 ++-- app/serializers/api/v1/protocol_serializer.rb | 15 +++++++++++++ app/serializers/api/v1/step_serializer.rb | 5 ++++- app/serializers/api/v1/task_serializer.rb | 9 +++++++- config/routes.rb | 2 +- 7 files changed, 46 insertions(+), 16 deletions(-) diff --git a/app/controllers/api/v1/protocols_controller.rb b/app/controllers/api/v1/protocols_controller.rb index 107e1ce61..7ad7a0836 100644 --- a/app/controllers/api/v1/protocols_controller.rb +++ b/app/controllers/api/v1/protocols_controller.rb @@ -3,23 +3,26 @@ module Api module V1 class ProtocolsController < BaseController - before_action :load_team - before_action :load_project - before_action :load_experiment - before_action :load_task + include Api::V1::ExtraParams + + before_action :load_team, :load_project, :load_experiment, :load_task + before_action only: :show do + load_protocol(:id) + end def index protocols = @task.protocols .page(params.dig(:page, :number)) .per(params.dig(:page, :size)) render jsonapi: protocols, - each_serializer: ProtocolSerializer + each_serializer: ProtocolSerializer, rte_rendering: render_rte?, team: @team end - private - - def load_protocol - @protocol = @task.protocols.find(params.require(:id)) + def show + render jsonapi: @protocol, serializer: ProtocolSerializer, + include: include_params, + rte_rendering: render_rte?, + team: @team end end end diff --git a/app/controllers/api/v1/steps_controller.rb b/app/controllers/api/v1/steps_controller.rb index 367ad09e4..f211a984c 100644 --- a/app/controllers/api/v1/steps_controller.rb +++ b/app/controllers/api/v1/steps_controller.rb @@ -16,13 +16,15 @@ module Api render jsonapi: steps, each_serializer: StepSerializer, include: include_params, - rte_rendering: render_rte? + rte_rendering: render_rte?, + team: @team end def show render jsonapi: @step, serializer: StepSerializer, include: include_params, - rte_rendering: render_rte? + rte_rendering: render_rte?, + team: @team end def create diff --git a/app/controllers/api/v1/tasks_controller.rb b/app/controllers/api/v1/tasks_controller.rb index 0e304f7b3..ac4696d82 100644 --- a/app/controllers/api/v1/tasks_controller.rb +++ b/app/controllers/api/v1/tasks_controller.rb @@ -19,11 +19,11 @@ module Api .page(params.dig(:page, :number)) .per(params.dig(:page, :size)) - render jsonapi: tasks, each_serializer: TaskSerializer, rte_rendering: render_rte? + render jsonapi: tasks, each_serializer: TaskSerializer, rte_rendering: render_rte?, team: @team end def show - render jsonapi: @task, serializer: TaskSerializer, rte_rendering: render_rte? + render jsonapi: @task, serializer: TaskSerializer, rte_rendering: render_rte?, team: @team end def create diff --git a/app/serializers/api/v1/protocol_serializer.rb b/app/serializers/api/v1/protocol_serializer.rb index 176814486..69dbeda20 100644 --- a/app/serializers/api/v1/protocol_serializer.rb +++ b/app/serializers/api/v1/protocol_serializer.rb @@ -3,6 +3,10 @@ module Api module V1 class ProtocolSerializer < ActiveModel::Serializer + include ApplicationHelper + include ActionView::Helpers::TextHelper + include InputSanitizeHelper + type :protocols attributes :id, :name, :authors, :description, :protocol_type has_many :protocol_keywords, @@ -12,6 +16,17 @@ module Api unless: -> { object.protocol_keywords.empty? } has_many :steps, serializer: StepSerializer, if: -> { object.steps.any? } belongs_to :parent, serializer: ProtocolSerializer, if: -> { object.parent.present? } + + def description + if instance_options[:rte_rendering] + custom_auto_link(object.tinymce_render(:description), + simple_format: false, + tags: %w(img), + team: instance_options[:team]) + else + object.description + end + end end end end diff --git a/app/serializers/api/v1/step_serializer.rb b/app/serializers/api/v1/step_serializer.rb index 9ed994531..8bc2fdd8d 100644 --- a/app/serializers/api/v1/step_serializer.rb +++ b/app/serializers/api/v1/step_serializer.rb @@ -18,7 +18,10 @@ module Api def description if instance_options[:rte_rendering] - custom_auto_link(object.tinymce_render(:description), simple_format: false, tags: %w(img)) + custom_auto_link(object.tinymce_render(:description), + simple_format: false, + tags: %w(img), + team: instance_options[:team]) else object.description end diff --git a/app/serializers/api/v1/task_serializer.rb b/app/serializers/api/v1/task_serializer.rb index ebf635928..6064c2a66 100644 --- a/app/serializers/api/v1/task_serializer.rb +++ b/app/serializers/api/v1/task_serializer.rb @@ -3,6 +3,10 @@ module Api module V1 class TaskSerializer < ActiveModel::Serializer + include ApplicationHelper + include ActionView::Helpers::TextHelper + include InputSanitizeHelper + type :tasks attributes :id, :name, :started_on, :due_date, :description, :state, :archived has_many :output_tasks, key: :outputs, @@ -22,7 +26,10 @@ module Api def description if instance_options[:rte_rendering] - custom_auto_link(object.tinymce_render(:description), simple_format: false, tags: %w(img)) + custom_auto_link(object.tinymce_render(:description), + simple_format: false, + tags: %w(img), + team: instance_options[:team]) else object.description end diff --git a/config/routes.rb b/config/routes.rb index 50c39fae5..e2b4c743e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -715,7 +715,7 @@ Rails.application.routes.draw do resources :task_tags, only: %i(index show), path: 'tags', as: :tags - resources :protocols, only: %i(index) do + resources :protocols, only: %i(index show) do resources :steps do resources :assets, only: %i(index show create), path: 'attachments' resources :checklists, path: 'checklists' do From 3c1c562e4ca066bf94c4e97e3ef3797006902997 Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Fri, 7 Aug 2020 12:17:27 +0200 Subject: [PATCH 12/30] Override password reset action, sign_in user only if 2fa disabled --- app/controllers/users/passwords_controller.rb | 31 ++++++++++++++----- config/initializers/devise.rb | 4 ++- config/routes.rb | 3 +- features/result.feature | 1 + 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/app/controllers/users/passwords_controller.rb b/app/controllers/users/passwords_controller.rb index 53cc34e39..df21e92db 100644 --- a/app/controllers/users/passwords_controller.rb +++ b/app/controllers/users/passwords_controller.rb @@ -15,15 +15,32 @@ class Users::PasswordsController < Devise::PasswordsController # end # PUT /resource/password - # def update - # super - # end + def update + self.resource = resource_class.reset_password_by_token(resource_params) + yield resource if block_given? - # protected + if resource.errors.empty? + resource.unlock_access! if unlockable?(resource) + if !resource.two_factor_auth_enabled? + flash_message = resource.active_for_authentication? ? :updated : :updated_not_active + set_flash_message!(:notice, flash_message) + resource.after_database_authentication + sign_in(resource_name, resource) + else + set_flash_message!(:notice, :updated_not_active) + end + respond_with resource, location: after_resetting_password_path_for(resource) + else + set_minimum_password_length + respond_with resource + end + end - # def after_resetting_password_path_for(resource) - # super(resource) - # end + protected + + def after_resetting_password_path_for(resource) + resource.two_factor_auth_enabled? ? new_session_path(resource_name) : after_sign_in_path_for(resource) + end # The path used after sending reset password instructions # def after_sending_reset_password_instructions_path_for(resource_name) diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 0981cc7e3..517a5a3ac 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -243,7 +243,9 @@ Devise.setup do |config| # When set to false, does not sign a user in automatically after their password is # reset. Defaults to true, so a user is signed in automatically after a reset. - config.sign_in_after_reset_password = false + # + # This setting has no effect, controller has been overriden at controllers/users/passwords_controller.rb + # config.sign_in_after_reset_password = false # ==> Configuration for :encryptable # Allow you to use another encryption algorithm besides bcrypt (default). You can use diff --git a/config/routes.rb b/config/routes.rb index 50c39fae5..d224e6402 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,7 +14,8 @@ Rails.application.routes.draw do sessions: 'users/sessions', invitations: 'users/invitations', confirmations: 'users/confirmations', - omniauth_callbacks: 'users/omniauth_callbacks' } + omniauth_callbacks: 'users/omniauth_callbacks', + passwords: 'users/passwords' } root 'dashboards#show' diff --git a/features/result.feature b/features/result.feature index ebc7406d4..3bb6757de 100644 --- a/features/result.feature +++ b/features/result.feature @@ -20,6 +20,7 @@ Scenario: Unsuccessful add Text result Given I am on Task results page And I click "Add new result" button And I click on "Text" within dropdown menu + And WAIT And I click "Add" button Then I should see "can't be blank" And I click "Cancel" button From a3ad8b5aab1dcaacda932bb3161b1cd5a0f4fb55 Mon Sep 17 00:00:00 2001 From: aignatov-bio Date: Mon, 10 Aug 2020 12:55:56 +0200 Subject: [PATCH 13/30] Fix dashboard calendar due date selection --- app/controllers/dashboard/calendars_controller.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/app/controllers/dashboard/calendars_controller.rb b/app/controllers/dashboard/calendars_controller.rb index 5f098cfdf..6390b12cf 100644 --- a/app/controllers/dashboard/calendars_controller.rb +++ b/app/controllers/dashboard/calendars_controller.rb @@ -6,9 +6,9 @@ module Dashboard include MyModulesHelper def show - date = DateTime.parse(params[:date]) - start_date = date.at_beginning_of_month.utc - 7.days - end_date = date.at_end_of_month.utc + 14.days + date = params[:date].in_time_zone(current_user.time_zone) + start_date = date.at_beginning_of_month.utc - 8.days + end_date = date.at_end_of_month.utc + 15.days due_dates = current_user.my_modules.active.uncomplete .joins(experiment: :project) .where(experiments: { archived: false }) @@ -16,16 +16,18 @@ module Dashboard .where('my_modules.due_date > ? AND my_modules.due_date < ?', start_date, end_date) .joins(:protocols).where(protocols: { team_id: current_team.id }) .pluck(:due_date) - render json: { events: due_dates.map { |i| { date: i } } } + render json: { events: due_dates.map { |i| { date: i.to_date } } } end def day - date = DateTime.parse(params[:date]).utc + date = params[:date].in_time_zone(current_user.time_zone) + start_date = date.utc + end_date = date.end_of_day.utc my_modules = current_user.my_modules.active.uncomplete .joins(experiment: :project) .where(experiments: { archived: false }) .where(projects: { archived: false }) - .where('DATE(my_modules.due_date) = DATE(?)', date) + .where('my_modules.due_date > ? AND my_modules.due_date < ?', start_date, end_date) .where(projects: { team_id: current_team.id }) render json: { html: render_to_string(partial: 'shared/my_modules_list_partial.html.erb', locals: { From e9028961db261cde3862e9888568a25ba5408871 Mon Sep 17 00:00:00 2001 From: aignatov-bio Date: Mon, 10 Aug 2020 13:47:32 +0200 Subject: [PATCH 14/30] Add border to tinymce table in docx --- app/services/reports/docx/private_methods.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/reports/docx/private_methods.rb b/app/services/reports/docx/private_methods.rb index 9b025459b..1f4c7048b 100644 --- a/app/services/reports/docx/private_methods.rb +++ b/app/services/reports/docx/private_methods.rb @@ -287,7 +287,7 @@ module Reports::Docx::PrivateMethods Reports::Docx.render_p_element(c, cell_content, scinote_url: scinote_url, link_style: link_style, skip_br: true) elsif cell_content[:type] == 'table' - c.table formated_cell_content[:data] + c.table formated_cell_content[:data], border_size: Constants::REPORT_DOCX_TABLE_BORDER_SIZE elsif cell_content[:type] == 'image' Reports::Docx.render_img_element(c, cell_content, table: { columns: row.children.length / 3 }) end @@ -301,7 +301,7 @@ module Reports::Docx::PrivateMethods if options[:nested_table] docx_table else - @docx.table docx_table + @docx.table docx_table, border_size: Constants::REPORT_DOCX_TABLE_BORDER_SIZE end end From e429c78d567cd5bc36105185b4f3e1a3b1073c99 Mon Sep 17 00:00:00 2001 From: Urban Rotnik Date: Mon, 10 Aug 2020 14:35:47 +0200 Subject: [PATCH 15/30] Fix permission check --- app/controllers/search_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index af71167f3..302cba1bb 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -263,7 +263,7 @@ class SearchController < ApplicationController def search_repository @repository = Repository.find_by_id(params[:repository]) - render_403 unless can_read_repository?(@repository) + render_403 unless user.teams.include?(repository.team) || repository.private_shared_with?(user.teams) @repository_results = [] if @repository_search_count_total > 0 @repository_results = From 94e6b554f1d3b119819de734af52a18b4ed7384a Mon Sep 17 00:00:00 2001 From: Mojca Lorber Date: Mon, 10 Aug 2020 17:29:29 +0200 Subject: [PATCH 16/30] Revert back the view --- .../_repository_row_info_modal.html.erb | 98 ++++++++++--------- 1 file changed, 50 insertions(+), 48 deletions(-) diff --git a/app/views/repositories/_repository_row_info_modal.html.erb b/app/views/repositories/_repository_row_info_modal.html.erb index cd773eaef..ce9ed0173 100644 --- a/app/views/repositories/_repository_row_info_modal.html.erb +++ b/app/views/repositories/_repository_row_info_modal.html.erb @@ -39,56 +39,58 @@ <% end %>

    - <% if @assigned_modules.size.positive? %> -
    - <%= t('repository_row.modal_info.title', nr: @assigned_modules.size) %> - <%= t('repository_row.modal_info.private_tasks', nr: @private_modules.size) if @private_modules.size.positive? %> -
    - <% if @viewable_modules.size.positive? %> -
    - - - <% @viewable_modules.each do |my_module| %> - - + <% if @repository_row.repository.is_a?(Repository) %> + <% if @assigned_modules.size.positive? %> +
    + <%= t('repository_row.modal_info.title', nr: @assigned_modules.size) %> + <%= t('repository_row.modal_info.private_tasks', nr: @private_modules.size) if @private_modules.size.positive? %> +
    + <% if @viewable_modules.size.positive? %> +
    +
    + + <% @viewable_modules.each do |my_module| %> + + + + <% end %> + + + + - <% end %> - - - - - - - -
    + + + + <% end %> + <% else %> + <%= t('repository_row.modal_info.no_tasks') %> <% end %> - <% else %> - <%= t('repository_row.modal_info.no_tasks') %> <% end %> From 7af9c387cf5f63b303616db8da52ee85de3e0fb0 Mon Sep 17 00:00:00 2001 From: Mojca Lorber Date: Tue, 11 Aug 2020 10:27:23 +0200 Subject: [PATCH 17/30] Fix errors --- app/controllers/repository_rows_controller.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index 688fdea53..23e920e6e 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -52,8 +52,10 @@ class RepositoryRowsController < ApplicationController def show @repository_row = RepositoryRow.find_by(id: params[:id]) - render_403 if !can_read_repository?(@repository_row.repository) || @repository_row.repository_id != params[:repository_id] - + if !can_read_repository?(@repository_row.repository) || @repository_row.repository_id != params[:repository_id].to_i + return render_403 + end + @assigned_modules = @repository_row.my_modules.joins(experiment: :project) @viewable_modules = @assigned_modules.viewable_by_user(current_user, current_user.teams) @private_modules = @assigned_modules - @viewable_modules From 5d15e738661944cd5070b4a8cc4f15c1508b2e95 Mon Sep 17 00:00:00 2001 From: Mojca Lorber Date: Tue, 11 Aug 2020 13:33:44 +0200 Subject: [PATCH 18/30] Fix failing tests --- app/controllers/repository_rows_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/repository_rows_controller.rb b/app/controllers/repository_rows_controller.rb index 23e920e6e..eb969ca80 100644 --- a/app/controllers/repository_rows_controller.rb +++ b/app/controllers/repository_rows_controller.rb @@ -52,9 +52,9 @@ class RepositoryRowsController < ApplicationController def show @repository_row = RepositoryRow.find_by(id: params[:id]) - if !can_read_repository?(@repository_row.repository) || @repository_row.repository_id != params[:repository_id].to_i - return render_403 - end + return render_404 unless @repository_row + return render_404 unless @repository_row.repository_id == params[:repository_id].to_i + return render_403 unless can_read_repository?(@repository_row.repository) @assigned_modules = @repository_row.my_modules.joins(experiment: :project) @viewable_modules = @assigned_modules.viewable_by_user(current_user, current_user.teams) From 84b99eaf3dfaf0a33732f05f18b1b9c00a3f3d81 Mon Sep 17 00:00:00 2001 From: aignatov-bio Date: Tue, 11 Aug 2020 15:39:35 +0200 Subject: [PATCH 19/30] Fix error handling on registration page --- app/views/users/registrations/new.html.erb | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/views/users/registrations/new.html.erb b/app/views/users/registrations/new.html.erb index fb8211b8e..329e1c204 100644 --- a/app/views/users/registrations/new.html.erb +++ b/app/views/users/registrations/new.html.erb @@ -65,9 +65,11 @@