Skip to content

[lb] Fix site validation on GetCourseOptionFromCodes service#11685

Merged
elceebee merged 1 commit intomainfrom
lb/fix-site-validation-duplicate-site-codes
Feb 23, 2026
Merged

[lb] Fix site validation on GetCourseOptionFromCodes service#11685
elceebee merged 1 commit intomainfrom
lb/fix-site-validation-duplicate-site-codes

Conversation

@elceebee
Copy link
Collaborator

@elceebee elceebee commented Feb 23, 2026

Context

I'm really surprised this hasn't come up before -- This is a bug in the GetCourseOptionFromCodes service used when making an offer and changing courses from the API. But it was raised in Sandbox, support ticket.

Changes proposed in this pull request

Call uniq, so courses with full and part-time options don't give false invalid errors.

Guidance to review

In sandbox

get_course_option_service = GetCourseOptionFromCode.new(
  provider_code: 'E42',
  course_code: '2PPM' # this course has full and part time options for main site
  study_mode: 'part_time',
  site_code: '-' # Main site,
  recruitment_cycle_year: 2026,
)

get_course_option_service.valid? will be false.

This is because

provider = Provider.find_by(code: 'E42')
course = provider.courses.find_by(recruitment_cycle_year: 2026, code: '2PPM')
sites = provider.sites
  .joins(:course_options) 
  .merge(CourseOption.selectable.where(course:)
  .where(code: '-')
  .count 

=> 2

But the same with uniq.count is 1.

Things to check

  • If the code removes any existing feature flags, a data migration has also been added to delete the entry from the database
  • API release notes have been updated if necessary
  • If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • Attach the PR to the Trello card
  • This code adds a column or table to the database
    • This code does not rely on migrations in the same Pull Request
    • decide whether it needs to be in analytics yml file or analytics blocklist
    • data insights team has been informed of the change and have updated the pipeline
    • the sanitise.sql script and 0025-protecting-personal-data-in-production-dump.md ADR have been updated
    • does the code safely backfill existing records for consistency

@elceebee elceebee changed the title call uniq on site [lb] Fix site validation on GetCourseOptionFromCodes service Feb 23, 2026
@elceebee elceebee marked this pull request as ready for review February 23, 2026 16:06
@elceebee elceebee requested a review from a team February 23, 2026 16:08
Copy link
Contributor

@JamieCleare2525 JamieCleare2525 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@elceebee elceebee merged commit 028defa into main Feb 23, 2026
49 of 50 checks passed
@elceebee elceebee deleted the lb/fix-site-validation-duplicate-site-codes branch February 23, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants