Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested Placement Validations #4791

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

adamjcolvin
Copy link
Contributor

@adamjcolvin adamjcolvin commented Nov 6, 2024

Context

Trello

When we create placements as part of creating a trainee, we don't quite follow the same logic as when creating a placement on its own. When creating a placement on its own, we don't seem to use the hesa mapping logic that we use when creating them with a trainee. I'm not sure if this is what we expect, but on looking at the hesa mapping logic, it seemed that was an issue with it.

def urn
  urn = @params[:urn]
  return if school_urn_applicable?(urn)

  urn
end

def school_urn_applicable?(urn)
  Attributes::NOT_APPLICABLE_SCHOOL_URNS.exclude?(urn)
end

This logic seems to return nil for the URN if the URN is not within the array NOT_APPLICABLE_SCHOOL_URNS. This will be most URNs since the array is only small:

NOT_APPLICABLE_SCHOOL_URNS = %w[900000 900010 900020 900030].freeze

Changes proposed in this pull request

I've added a test to the post_hesa_trainees_spec which reproduces the issue. I've then updated the logic in the hesa_mapper so that the test now passes.

Guidance to review

Should we actually be using the hesa mappers when creating a single placement, as we do for nested placements?

Important business

  • Does this PR introduce any PII fields that need to be overwritten or deleted in db/scripts/sanitise.sql?
  • Does this PR change the database schema? If so, have you updated the config/analytics.yml file and considered whether you need to send 'import_entity' events?
  • Do we need to send any updates to DQT as part of the work in this PR?
  • Does this PR need an ADR?

NB: Please notify the #twd_data_insights team and ask for a review if new fields are being added to analytics.yml

@adamjcolvin adamjcolvin changed the title Make Nested Placements the Same as Single Placements Nested Placement Validations Nov 7, 2024
@adamjcolvin adamjcolvin marked this pull request as ready for review November 12, 2024 10:39
@adamjcolvin adamjcolvin marked this pull request as draft November 12, 2024 10:51
Copy link

sonarcloud bot commented Nov 12, 2024

@adamjcolvin adamjcolvin marked this pull request as ready for review November 12, 2024 12:37
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.

1 participant