From 276e0777d86d3fcae0bb6117cea8484a6d879d3a Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Mon, 24 Aug 2026 03:12:04 +0800 Subject: [PATCH 1/2] fix(AssessmentsIndex): hide import from marketplace button if user cannot access --- .../assessments/index.json.jbuilder | 7 ++ .../__test__/AssessmentsIndex.test.tsx | 86 +++++++++++++++++++ .../__test__/AssessmentsTable.test.tsx | 1 + .../pages/AssessmentsIndex/index.tsx | 29 ++++--- .../types/course/assessment/assessments.ts | 2 + .../assessments_marketplace_spec.rb | 65 ++++++++++++++ 6 files changed, 177 insertions(+), 13 deletions(-) create mode 100644 client/app/bundles/course/assessment/pages/AssessmentsIndex/__test__/AssessmentsIndex.test.tsx diff --git a/app/views/course/assessment/assessments/index.json.jbuilder b/app/views/course/assessment/assessments/index.json.jbuilder index 14a964d704..d462681285 100644 --- a/app/views/course/assessment/assessments/index.json.jbuilder +++ b/app/views/course/assessment/assessments/index.json.jbuilder @@ -1,5 +1,6 @@ # frozen_string_literal: true achievements_enabled = !current_component_host[:course_achievements_component].nil? +marketplace_enabled = !current_component_host[:course_assessment_marketplace_component].nil? submissions_hash = @assessments.to_h { |assessment| [assessment.id, assessment.submissions] } # Empty for every course except the marketplace's snapshot container viewed by a system admin. marketplace_versions = defined?(@marketplace_versions) ? @marketplace_versions : {} @@ -16,6 +17,12 @@ json.display do json.canCreateAssessments can?(:create, Course::Assessment.new(tab: @tab)) json.canManageMonitor @can_manage_monitor && @monitoring_component_enabled + # Gates the "Import Assessments" button, which only links into the marketplace. Marketplace access + # is per-person allow-listed (see Course::AssessmentMarketplaceAbilityComponent) and not implied by + # `:create` on assessments, so a manager without it would otherwise be sent to a 403. Mirrors the + # gate on the marketplace sidebar item in Course::AssessmentMarketplaceComponent. + json.canImportAssessments marketplace_enabled && can?(:access_marketplace, current_course) + # True only in the marketplace's snapshot container, viewed by a system admin. Switches on the # container-only Listing/Version/Source columns and the search toolbar — every other course's # assessments index must stay exactly as it was. diff --git a/client/app/bundles/course/assessment/pages/AssessmentsIndex/__test__/AssessmentsIndex.test.tsx b/client/app/bundles/course/assessment/pages/AssessmentsIndex/__test__/AssessmentsIndex.test.tsx new file mode 100644 index 0000000000..d4d7bb5959 --- /dev/null +++ b/client/app/bundles/course/assessment/pages/AssessmentsIndex/__test__/AssessmentsIndex.test.tsx @@ -0,0 +1,86 @@ +import { render } from 'test-utils'; +import { AssessmentsListData } from 'types/course/assessment/assessments'; + +import { fetchAssessments } from '../../../operations/assessments'; +import AssessmentsIndex from '../index'; + +jest.mock('../../../operations/assessments', () => ({ + fetchAssessments: jest.fn(), +})); + +const mockFetchAssessments = fetchAssessments as jest.MockedFunction< + typeof fetchAssessments +>; + +const listData = ( + canCreateAssessments: boolean, + canImportAssessments: boolean, +): AssessmentsListData => ({ + display: { + isStudent: false, + isGamified: false, + isKoditsuExamEnabled: false, + timelineAlgorithm: 'fixed', + allowRandomization: false, + isAchievementsEnabled: false, + isMonitoringEnabled: false, + bonusAttributes: false, + endTimes: false, + canCreateAssessments, + canImportAssessments, + tabId: 42, + tabTitle: 'Assessments: Default', + tabUrl: '/courses/1/assessments', + canManageMonitor: false, + isMarketplaceContainer: false, + category: { + id: 1, + title: 'Assessments', + tabs: [{ id: 42, title: 'Default' }], + }, + }, + assessments: [], +}); + +const renderIndex = ( + canCreateAssessments: boolean, + canImportAssessments: boolean, +): ReturnType => { + mockFetchAssessments.mockResolvedValue( + listData(canCreateAssessments, canImportAssessments), + ); + + return render(); +}; + +// The import button only links into the marketplace, whose access is allow-listed per person. +// Riding it on `canCreateAssessments` showed every manager a button that only led to a 403. +it('hides the import button from a user who cannot reach the marketplace', async () => { + const page = renderIndex(true, false); + + expect( + await page.findByRole('button', { name: 'New Assessment' }), + ).toBeVisible(); + expect( + page.queryByRole('link', { name: 'Import Assessments' }), + ).not.toBeInTheDocument(); +}); + +it('shows the import button to a user who can reach the marketplace', async () => { + const page = renderIndex(true, true); + + expect( + await page.findByRole('link', { name: 'Import Assessments' }), + ).toBeVisible(); +}); + +it('shows the import button alone when the user cannot create assessments', async () => { + const page = renderIndex(false, true); + + expect( + await page.findByRole('link', { name: 'Import Assessments' }), + ).toBeVisible(); + expect( + page.queryByRole('button', { name: 'New Assessment' }), + ).not.toBeInTheDocument(); +}); diff --git a/client/app/bundles/course/assessment/pages/AssessmentsIndex/__test__/AssessmentsTable.test.tsx b/client/app/bundles/course/assessment/pages/AssessmentsIndex/__test__/AssessmentsTable.test.tsx index 73ef86ed8b..c35a6ca174 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentsIndex/__test__/AssessmentsTable.test.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentsIndex/__test__/AssessmentsTable.test.tsx @@ -44,6 +44,7 @@ const listData = ( bonusAttributes: false, endTimes: false, canCreateAssessments: true, + canImportAssessments: true, tabId: 1, tabTitle: 'Assessments: Default', tabUrl: '/courses/1/assessments', diff --git a/client/app/bundles/course/assessment/pages/AssessmentsIndex/index.tsx b/client/app/bundles/course/assessment/pages/AssessmentsIndex/index.tsx index 82a28038ba..2883eea5fa 100644 --- a/client/app/bundles/course/assessment/pages/AssessmentsIndex/index.tsx +++ b/client/app/bundles/course/assessment/pages/AssessmentsIndex/index.tsx @@ -30,23 +30,26 @@ const AssessmentsIndex = (): JSX.Element => { {(data, refreshable): JSX.Element => ( - + {data.display.canCreateAssessments && ( + + )} ) } diff --git a/client/app/types/course/assessment/assessments.ts b/client/app/types/course/assessment/assessments.ts index 0e37f412d0..2db1a9be83 100644 --- a/client/app/types/course/assessment/assessments.ts +++ b/client/app/types/course/assessment/assessments.ts @@ -103,6 +103,8 @@ export interface AssessmentsListData { bonusAttributes: boolean; endTimes: boolean; canCreateAssessments: boolean; + /** Whether the user may reach the marketplace the import button links to. */ + canImportAssessments: boolean; tabId: number; tabTitle: string; tabUrl: string; diff --git a/spec/controllers/course/assessment/assessments_marketplace_spec.rb b/spec/controllers/course/assessment/assessments_marketplace_spec.rb index 198502401b..92d497191a 100644 --- a/spec/controllers/course/assessment/assessments_marketplace_spec.rb +++ b/spec/controllers/course/assessment/assessments_marketplace_spec.rb @@ -505,5 +505,70 @@ def show_for(target_course, target_assessment) expect(response.parsed_body['marketplaceUpdate']['canUpdateInPlace']).to be(false) end end + + # The "Import Assessments" button only links into the marketplace, whose access is allow-listed + # per person. It used to ride on `canCreateAssessments`, so every manager saw a button that led + # straight to a 403. + describe 'GET #index — canImportAssessments' do + # Nothing rolls back here, so an `everyone` rule leaked by an earlier spec file would grant + # access to the not-allow-listed users below. Same cleanup as assessment_marketplace_ability_spec. + before do + Course::Assessment::Marketplace::AllowlistRule.delete_all + Course::Assessment::Marketplace::AccessBlock.delete_all + end + + def display_for(target_course) + get :index, as: :json, params: { course_id: target_course.id } + response.parsed_body['display'] + end + + it 'grants it to a system admin' do + controller_sign_in(controller, admin) + + expect(display_for(course)).to include('canImportAssessments' => true) + end + + context 'as a course manager' do + let(:manager) { create(:course_manager, course: course).user } + + before { controller_sign_in(controller, manager) } + + # The regression: creating assessments and reaching the marketplace are separate permissions. + it 'withholds it from a manager who is not allow-listed, who may still create assessments' do + display = display_for(course) + + expect(display).to include('canCreateAssessments' => true) + expect(display).to include('canImportAssessments' => false) + end + + it 'grants it once the manager is allow-listed' do + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: manager) + + expect(display_for(course)).to include('canImportAssessments' => true) + end + + it 'withholds it from an allow-listed manager who is blocked' do + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: manager) + create(:course_assessment_marketplace_access_block, user: manager) + + expect(display_for(course)).to include('canImportAssessments' => false) + end + + # The marketplace controller is a `ComponentController`, so the destination 404s when the + # course has the component switched off, however the ability resolves. + it 'withholds it when the course has the marketplace component disabled' do + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: manager) + course.set_component_enabled_boolean!(:course_assessment_marketplace_component, false) + + expect(display_for(course)).to include('canImportAssessments' => false) + end + end + + it 'withholds it from a course student' do + controller_sign_in(controller, create(:course_student, course: course).user) + + expect(display_for(course)).to include('canImportAssessments' => false) + end + end end end From c7ef2e6606f9c5e083ae6023486d34e52a6f6ca5 Mon Sep 17 00:00:00 2001 From: adi-herwana-nus Date: Mon, 24 Aug 2026 03:25:20 +0800 Subject: [PATCH 2/2] style(rubocop): address minor style violations post-circleci migration --- .rubocop_todo.yml | 84 ------------------- .../application_authentication_concern.rb | 6 +- .../question/text_responses_controller.rb | 4 +- .../application_html_formatters_helper.rb | 2 +- .../submission/auto_feedback_job.rb | 4 +- .../course/assessment/questions_concern.rb | 4 +- .../course/video/watch_statistics_concern.rb | 6 +- .../assessment/answer/multiple_response.rb | 2 +- .../assessment/answer/programming_file.rb | 2 +- app/models/course/lesson_plan/item.rb | 4 +- app/models/course/settings/email.rb | 2 +- .../course/settings/survey_component.rb | 4 - .../parse_invitation_concern.rb | 3 +- .../codaveri_problem_generation_service.rb | 2 +- .../duplication/object_duplication_service.rb | 2 +- .../programming/_response.json.jbuilder | 4 +- lib/autoload/duplicator.rb | 2 +- .../has_one_many_attachments_spec.rb | 4 +- 18 files changed, 23 insertions(+), 118 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index dfc07876e9..2b159e8c96 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -92,12 +92,6 @@ Lint/RedundantCopDisableDirective: - 'spec/support/reference_timelines_helper.rb' - 'spec/support/stubs/ssid/api_stubs.rb' -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -Lint/RedundantRequireStatement: - Exclude: - - 'app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb' - # Offense count: 2 Lint/ReturnInVoidContext: Exclude: @@ -111,12 +105,6 @@ Lint/SafeNavigationConsistency: Exclude: - 'app/views/course/assessment/answer/programming/_programming.json.jbuilder' -# Offense count: 1 -# Configuration parameters: AllowRBSInlineAnnotation. -Lint/SelfAssignment: - Exclude: - - 'spec/libraries/has_one_many_attachments_spec.rb' - # Offense count: 1 Lint/StructNewOverride: Exclude: @@ -130,11 +118,6 @@ Lint/SymbolConversion: Exclude: - 'app/controllers/concerns/signals/emission_concern.rb' -# Offense count: 1 -Lint/UnmodifiedReduceAccumulator: - Exclude: - - 'app/models/concerns/course/assessment/questions_concern.rb' - # Offense count: 7 # This cop supports safe autocorrection (--autocorrect). Lint/UselessAssignment: @@ -160,25 +143,6 @@ Lint/UselessConstantScoping: - 'lib/autoload/coursemology_docker_container.rb' - 'lib/extensions/attachable/active_record/base.rb' -# Offense count: 1 -# This cop supports unsafe autocorrection (--autocorrect-all). -Lint/UselessMethodDefinition: - Exclude: - - 'app/models/course/settings/survey_component.rb' - -# Offense count: 1 -# This cop supports unsafe autocorrection (--autocorrect-all). -Lint/UselessOr: - Exclude: - - 'app/services/course/assessment/question/codaveri_problem_generation_service.rb' - -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: CheckForMethodsWithNoSideEffects. -Lint/Void: - Exclude: - - 'app/models/concerns/course/video/watch_statistics_concern.rb' - # Offense count: 99 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes, Max. Metrics/AbcSize: @@ -456,11 +420,6 @@ Metrics/PerceivedComplexity: - 'app/services/course/assessment/reminder_service.rb' - 'app/services/course/assessment/submission/statistics_download_service.rb' -# Offense count: 2 -Naming/AccessorMethodName: - Exclude: - - 'app/controllers/concerns/application_authentication_concern.rb' - # Offense count: 24 # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: EnforcedStyle, BlockForwardingName. @@ -799,14 +758,6 @@ Style/HashEachMethods: - 'lib/tasks/db/populate_assessment_links.rake' - 'spec/libraries/course/conditional/user_satisfiability_graph_spec.rb' -# Offense count: 3 -# This cop supports safe autocorrection (--autocorrect). -Style/IfUnlessModifier: - Exclude: - - 'app/controllers/course/assessment/question/text_responses_controller.rb' - - 'app/jobs/course/assessment/submission/auto_feedback_job.rb' - - 'app/views/course/assessment/question/programming/_response.json.jbuilder' - # Offense count: 3 # This cop supports unsafe autocorrection (--autocorrect-all). Style/MapIntoArray: @@ -832,14 +783,6 @@ Style/MapToSet: - 'app/models/instance/user_role_request.rb' - 'lib/tasks/db/add_missing_email_settings.rake' -# Offense count: 3 -# This cop supports unsafe autocorrection (--autocorrect-all). -Style/MinMaxComparison: - Exclude: - - 'app/models/course/assessment/answer/programming_file.rb' - - 'app/services/course/duplication/object_duplication_service.rb' - - 'lib/autoload/duplicator.rb' - # Offense count: 7 # This cop supports safe autocorrection (--autocorrect). # Configuration parameters: AllowMethodComparison, ComparisonsThreshold. @@ -945,25 +888,6 @@ Style/RedundantParentheses: - 'app/services/course/assessment/question/programming/java/java_package_service.rb' - 'app/services/course/assessment/question/programming/python/python_package_service.rb' -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -Style/RedundantRegexpArgument: - Exclude: - - 'app/helpers/application_html_formatters_helper.rb' - -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -Style/RedundantRegexpEscape: - Exclude: - - 'app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb' - -# Offense count: 3 -# This cop supports safe autocorrection (--autocorrect). -Style/RedundantSelf: - Exclude: - - 'app/models/course/assessment/answer/multiple_response.rb' - - 'app/models/course/lesson_plan/item.rb' - # Offense count: 13 # This cop supports safe autocorrection (--autocorrect). Style/RedundantStringEscape: @@ -1058,11 +982,3 @@ Style/TernaryParentheses: - 'lib/tasks/coursemology/seed_600_gradebook.rake' - 'lib/tasks/coursemology/seed_gradebook.rake' - 'spec/libraries/coursemology_docker_container_spec.rb' - -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyleForMultiline. -# SupportedStylesForMultiline: comma, consistent_comma, diff_comma, no_comma -Style/TrailingCommaInArguments: - Exclude: - - 'app/models/course/settings/email.rb' diff --git a/app/controllers/concerns/application_authentication_concern.rb b/app/controllers/concerns/application_authentication_concern.rb index 0295511322..3a1b7a5da7 100644 --- a/app/controllers/concerns/application_authentication_concern.rb +++ b/app/controllers/concerns/application_authentication_concern.rb @@ -22,7 +22,7 @@ def current_session_id end def token_from_request - @token_from_request ||= get_token_from_bearer || get_token_from_cookies + @token_from_request ||= token_from_bearer || token_from_cookies end def current_decoded_token @@ -46,7 +46,7 @@ def authenticate_token @decoded_token.decoded_token end - def get_token_from_bearer + def token_from_bearer authorization_header_elements = request.headers['Authorization']&.split # render json: REQUIRES_AUTHENTICATION, status: :unauthorized and return unless authorization_header_elements @@ -65,7 +65,7 @@ def get_token_from_bearer token end - def get_token_from_cookies + def token_from_cookies cookies.encrypted[:access_token] end end diff --git a/app/controllers/course/assessment/question/text_responses_controller.rb b/app/controllers/course/assessment/question/text_responses_controller.rb index b0f0d6967d..d1c376e1d4 100644 --- a/app/controllers/course/assessment/question/text_responses_controller.rb +++ b/app/controllers/course/assessment/question/text_responses_controller.rb @@ -8,9 +8,7 @@ class Course::Assessment::Question::TextResponsesController < Course::Assessment before_action :load_question_assessment, only: [:edit, :update] def new - if params[:file_upload] == 'true' - @text_response_question.hide_text = true - end + @text_response_question.hide_text = true if params[:file_upload] == 'true' return unless params[:comprehension] == 'true' @text_response_question.is_comprehension = true diff --git a/app/helpers/application_html_formatters_helper.rb b/app/helpers/application_html_formatters_helper.rb index 3c25e5ad8a..be900d3a31 100644 --- a/app/helpers/application_html_formatters_helper.rb +++ b/app/helpers/application_html_formatters_helper.rb @@ -242,7 +242,7 @@ def sanitize_and_format_code(code, language, start_line) def process_ckeditor_rich_text_with_pipeline(pipeline, text) text_with_updated_code_tag = remove_internal_adjacent_code_tags(text) format_with_pipeline(pipeline, text_with_updated_code_tag). - gsub(//, '
') # Add lines to tables + gsub('
', '
') # Add lines to tables end # Filters the given text through the given pipeline. diff --git a/app/jobs/course/assessment/submission/auto_feedback_job.rb b/app/jobs/course/assessment/submission/auto_feedback_job.rb index 0edf9b864a..52b82a7b13 100644 --- a/app/jobs/course/assessment/submission/auto_feedback_job.rb +++ b/app/jobs/course/assessment/submission/auto_feedback_job.rb @@ -13,9 +13,7 @@ def perform_tracked(submission) instance = Course.unscoped { submission.assessment.course.instance } ActsAsTenant.with_tenant(instance) do submission.current_answers.each do |current_answer| - if current_answer.specific.self_respond_to?(:generate_feedback) - current_answer.specific.generate_feedback - end + current_answer.specific.generate_feedback if current_answer.specific.self_respond_to?(:generate_feedback) end end end diff --git a/app/models/concerns/course/assessment/questions_concern.rb b/app/models/concerns/course/assessment/questions_concern.rb index 081bf7e065..8a26cd7a29 100644 --- a/app/models/concerns/course/assessment/questions_concern.rb +++ b/app/models/concerns/course/assessment/questions_concern.rb @@ -62,9 +62,7 @@ def next_unanswered(submission) correctly_answered_questions = correctly_answered_questions(submission) return first if correctly_answered_questions.empty? - reduce(nil) do |_, question| - break question unless correctly_answered_questions.include?(question) - end + find { |question| correctly_answered_questions.exclude?(question) } end private diff --git a/app/models/concerns/course/video/watch_statistics_concern.rb b/app/models/concerns/course/video/watch_statistics_concern.rb index 245d5213ed..d95c8a726c 100644 --- a/app/models/concerns/course/video/watch_statistics_concern.rb +++ b/app/models/concerns/course/video/watch_statistics_concern.rb @@ -134,9 +134,9 @@ def correct_interval(event, last_start, video_duration) # @return [Hash] The hash containing arrays of start times and end times # of closed intervals. def handle_unclosed_interval(result, last_start, video_duration) - if [result[:end].size, 0].include? result[:start].size - result - elsif last_start.session.last_video_time > correct_interval(last_start, last_start, video_duration) + return result if [result[:end].size, 0].include?(result[:start].size) + + if last_start.session.last_video_time > correct_interval(last_start, last_start, video_duration) result[:end] << last_start.session.last_video_time else result[:start].pop diff --git a/app/models/course/assessment/answer/multiple_response.rb b/app/models/course/assessment/answer/multiple_response.rb index 975d69686e..ff319272ce 100644 --- a/app/models/course/assessment/answer/multiple_response.rb +++ b/app/models/course/assessment/answer/multiple_response.rb @@ -24,7 +24,7 @@ def retrieve_random_seed self.random_seed ||= Random.new_seed save - self.random_seed + random_seed end def compare_answer(other_answer) diff --git a/app/models/course/assessment/answer/programming_file.rb b/app/models/course/assessment/answer/programming_file.rb index 86cdcd81d4..73e41aa72b 100644 --- a/app/models/course/assessment/answer/programming_file.rb +++ b/app/models/course/assessment/answer/programming_file.rb @@ -28,7 +28,7 @@ def lines(line_numbers = nil) case line_numbers when Range - line_begin = line_numbers.min < 0 ? 0 : line_numbers.min + line_begin = [line_numbers.min, 0].max lines[line_begin..line_numbers.max] when Integer lines[line_numbers] diff --git a/app/models/course/lesson_plan/item.rb b/app/models/course/lesson_plan/item.rb index e89a8df2ed..714b599b26 100644 --- a/app/models/course/lesson_plan/item.rb +++ b/app/models/course/lesson_plan/item.rb @@ -255,8 +255,8 @@ def set_default_reference_time end def link_default_reference_time - self.default_reference_time.reference_timeline = course.default_reference_timeline - self.default_reference_time.lesson_plan_item = self + default_reference_time.reference_timeline = course.default_reference_timeline + default_reference_time.lesson_plan_item = self end def validate_only_one_default_reference_time diff --git a/app/models/course/settings/email.rb b/app/models/course/settings/email.rb index 26d7c804df..08a0177b30 100644 --- a/app/models/course/settings/email.rb +++ b/app/models/course/settings/email.rb @@ -41,7 +41,7 @@ class Course::Settings::Email < ApplicationRecord # A set of email settings that students are able to manage. STUDENT_SETTING = Set[:opening_reminder, :closing_reminder, :grades_released, :new_comment, - :new_topic, :post_replied, ].map { |v| settings[v] }.freeze + :new_topic, :post_replied ].map { |v| settings[v] }.freeze # A set of email settings that managers are able to manage. MANAGER_SETTING = Set[:opening_reminder, :closing_reminder_summary, :new_comment, :new_submission, :new_topic, diff --git a/app/models/course/settings/survey_component.rb b/app/models/course/settings/survey_component.rb index 8eacc7a69a..43da9419d3 100644 --- a/app/models/course/settings/survey_component.rb +++ b/app/models/course/settings/survey_component.rb @@ -2,10 +2,6 @@ class Course::Settings::SurveyComponent < Course::Settings::Component include Course::Settings::LessonPlanSettingsConcern - def lesson_plan_item_settings - super - end - def showable_in_lesson_plan? settings.lesson_plan_items ? settings.lesson_plan_items['enabled'] : true end diff --git a/app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb b/app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb index 32f9cfd4b4..d1329b0c3f 100644 --- a/app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb +++ b/app/services/concerns/course/user_invitation_service/parse_invitation_concern.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true require 'csv' -require 'set' # This concern includes methods required to parse the invitations data. # This can either be from a form, or a CSV file. @@ -178,7 +177,7 @@ def header_alias_map end def normalize_header(value) - value&.strip&.downcase&.gsub(/[\s_\-]+/, '') + value&.strip&.downcase&.gsub(/[\s_-]+/, '') end def build_header_map!(row) diff --git a/app/services/course/assessment/question/codaveri_problem_generation_service.rb b/app/services/course/assessment/question/codaveri_problem_generation_service.rb index 1f0f4aacd9..fe4dcd59e6 100644 --- a/app/services/course/assessment/question/codaveri_problem_generation_service.rb +++ b/app/services/course/assessment/question/codaveri_problem_generation_service.rb @@ -43,7 +43,7 @@ def codaveri_generate_problem private def initialize(assessment, params, language, version) # rubocop:disable Metrics/AbcSize - custom_prompt = params[:custom_prompt].to_s || '' + custom_prompt = params[:custom_prompt].to_s @payload = { userId: assessment.creator_id.to_s, courseName: assessment.course.title, diff --git a/app/services/course/duplication/object_duplication_service.rb b/app/services/course/duplication/object_duplication_service.rb index 2008d42b6e..56cebe4ce8 100644 --- a/app/services/course/duplication/object_duplication_service.rb +++ b/app/services/course/duplication/object_duplication_service.rb @@ -27,7 +27,7 @@ def duplicate_objects(source_course, destination_course, objects, options = {}) # @return [Float] Time difference between the +start_at+ of both courses. def time_shift(source_course, destination_course) shift = destination_course.start_at - source_course.start_at - shift >= 0 ? shift : 0 + [shift, 0].max end end diff --git a/app/views/course/assessment/question/programming/_response.json.jbuilder b/app/views/course/assessment/question/programming/_response.json.jbuilder index 43ecb070b3..eaa2304dca 100644 --- a/app/views/course/assessment/question/programming/_response.json.jbuilder +++ b/app/views/course/assessment/question/programming/_response.json.jbuilder @@ -1,9 +1,7 @@ # frozen_string_literal: true json.redirectAssessmentUrl course_assessment_path(current_course, @assessment) -if check_import_job? - json.importJobUrl job_path(@programming_question.import_job) -end +json.importJobUrl job_path(@programming_question.import_job) if check_import_job? if redirect_to_edit json.id @programming_question.id diff --git a/lib/autoload/duplicator.rb b/lib/autoload/duplicator.rb index 6ef9bff2a8..8b49bb39e5 100644 --- a/lib/autoload/duplicator.rb +++ b/lib/autoload/duplicator.rb @@ -62,7 +62,7 @@ def time_shift(original_time) # config/application.rb could be unsuitable as `Time.zone.local` does not work there. max_time = Time.zone.local(9999, 12, 31, 0, 0, 0) shifted_time = original_time + @time_shift_amount - shifted_time < max_time ? shifted_time : max_time + [shifted_time, max_time].min end # Checks if an item has been duplicated. diff --git a/spec/libraries/has_one_many_attachments_spec.rb b/spec/libraries/has_one_many_attachments_spec.rb index 454a07f0f2..99c8105b26 100644 --- a/spec/libraries/has_one_many_attachments_spec.rb +++ b/spec/libraries/has_one_many_attachments_spec.rb @@ -96,7 +96,9 @@ def clear_attribute_changes(attributes = changed_attributes.keys) describe '#attachment=' do context 'when the same attachment is specified' do - before { attachable.attachment = attachable.attachment } + # The self-assignment is the subject of this test: assigning the current attachment back + # must be a no-op. + before { attachable.attachment = attachable.attachment } # rubocop:disable Lint/SelfAssignment it 'does not change the attachment' do expect(attachable.attachment_changed?).to be(false)