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

[CPDLP-3958] Remove extra lead providers from seeds #5521

Merged
merged 3 commits into from
Feb 11, 2025
Merged

Conversation

leandroalemao
Copy link
Contributor

@leandroalemao leandroalemao commented Feb 7, 2025

Context

We currently have a set of known providers in production, however in seeds we create many other providers unnecessarily and the data for contracts and statements is not set up properly.

Changes proposed in this pull request

Ensure all seed data and participants for both ECTs/Mentors is attached to only the known lead providers.

Copy link

github-actions bot commented Feb 7, 2025

Review app deployed to https://cpd-ecf-review-5521-web.test.teacherservices.cloud

@@ -2,6 +2,16 @@

# This is actually ECFLeadProvider in all but name. See https://github.com/DFE-Digital/early-careers-framework/issues/698
class LeadProvider < ApplicationRecord
ALL_PROVIDERS = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this list can be in the seeds not in the main model? as we already define them there and this avoids confusion?

Copy link
Collaborator

@ethax-ross ethax-ross left a comment

Choose a reason for hiding this comment

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

Looks good past Abeer's commet 👍

Comment on lines 16 to 17
let(:lead_provider_1) { create(:lead_provider, name: "Capita") }
let(:school_1) { NewSeeds::Scenarios::Schools::School.new.build.with_partnership_in(cohort:, lead_provider: lead_provider_1).chosen_fip_and_partnered_in(cohort:) }
Copy link
Contributor

Choose a reason for hiding this comment

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

school_1 doesn't seem to be used, not needed maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great spot tks 👍 😄

Copy link
Contributor

@mooktakim mooktakim left a comment

Choose a reason for hiding this comment

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

Just one comment, otherwise good 👍

@leandroalemao leandroalemao added this pull request to the merge queue Feb 11, 2025
Merged via the queue into main with commit 6ed57ce Feb 11, 2025
42 checks passed
@leandroalemao leandroalemao deleted the CPDLP-3958 branch February 11, 2025 12:52
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.

4 participants