[CPDLP-3913] Remove any remnants of NPQ in ECF app #5461

mooktakim · 2025-01-21T16:15:39Z

Context

Ticket: https://dfedigital.atlassian.net/browse/CPDLP-3913

Changes proposed in this pull request

Removed REMOVE_NPQ_REFERENCES and related
Removed NPQ css
Removed NPQ form services
Removed NPQ logic from Cohort
Removed NPQ locales
Removed NPQ payment calculators
From rails credentials, removed:
- QUALIFIED_TEACHERS_API_URL
- QUALIFIED_TEACHERS_API_KEY
Added npq_registration_start_date to ignored columns for Cohort
Removed feature npq_capping

Guidance to review

github-actions · 2025-01-21T16:22:59Z

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

leandroalemao · 2025-01-22T16:51:01Z

app/models/cohort.rb

@@ -22,10 +21,6 @@ def self.active_registration_cohort
    where(registration_start_date: ..Date.current).order(start_year: :desc).first
  end

-  def self.active_npq_registration_cohort
-    where(npq_registration_start_date: ..Date.current).order(start_year: :desc).first.presence || current


Will we drop npq_registration_start_date from cohorts table as well?

We're keeping the columns for now, just removing the logic.

I think maybe removing the column might be a good shout, let's discuss tomorrow

leandroalemao

Would it be nice to remove feature flag as well?

disable_npq
npq_capping

app/models/cohort.rb

mooktakim · 2025-01-23T16:57:45Z

Would it be nice to remove feature flag as well?

disable_npq npq_capping

good question about the feature flags. For now left it. Will discuss

cwrw

thanks Mook a few minor comments, also maybe we can look to remove some NPQ content in app/views/sandbox/show.html.erb, db/new_seeds/base/add_lead_providers_and_cips.rb, otherwise looking great :)

docs/lib/govuk_tech_docs/open_api/renderer.rb

spec/requests/api/v3/delivery_partners_spec.rb

cwrw · 2025-01-23T17:14:44Z

app/models/cohort.rb

@@ -22,10 +21,6 @@ def self.active_registration_cohort
    where(registration_start_date: ..Date.current).order(start_year: :desc).first
  end

-  def self.active_npq_registration_cohort
-    where(npq_registration_start_date: ..Date.current).order(start_year: :desc).first.presence || current


I think maybe removing the column might be a good shout, let's discuss tomorrow

app/models/cohort.rb

ethax-ross

Looks good, couple of files I think can go:

https://github.com/DFE-Digital/early-careers-framework/blob/main/spec/fixtures/files/api_reference/api_spec_no_npq.json
https://github.com/DFE-Digital/early-careers-framework/blob/main/spec/fixtures/files/api_reference/api_spec.json

And a few references kicking about still (these are just a few but there are more on searching):

early-careers-framework/spec/factories/participant_identity.rb

Lines 10 to 12 in 1931220

    
           trait :npq_origin do 
        
             origin { "npq" } 
        
           end

early-careers-framework/app/models/participant_identity.rb

Line 12 in 1931220

npq: "npq",

cdb02bc

cwrw · 2025-01-24T10:48:04Z

I was thinking we leave the participant identity origin, as it will still exist and we don't intent on ever updating it/backfilling it

…fault

…API_KEY from rails credentials

…start_year_2020_or_earlier?'

…n saparate pr

cwrw

thanks Mook!

mooktakim temporarily deployed to review January 21, 2025 16:19 — with GitHub Actions Inactive

mooktakim force-pushed the 3913-remove-any-remnants-of-npq-in-ecf-app branch from b3cf381 to 88131be Compare January 21, 2025 16:25

mooktakim temporarily deployed to review January 21, 2025 16:30 — with GitHub Actions Inactive

mooktakim temporarily deployed to review January 21, 2025 16:55 — with GitHub Actions Inactive

mooktakim force-pushed the 3913-remove-any-remnants-of-npq-in-ecf-app branch from 6689322 to 8d9889d Compare January 22, 2025 11:01

mooktakim temporarily deployed to review January 22, 2025 11:05 — with GitHub Actions Inactive

mooktakim temporarily deployed to review January 22, 2025 11:16 — with GitHub Actions Inactive

mooktakim temporarily deployed to review January 22, 2025 11:27 — with GitHub Actions Inactive

mooktakim force-pushed the 3913-remove-any-remnants-of-npq-in-ecf-app branch from 1d80071 to 25692ac Compare January 22, 2025 11:37

mooktakim marked this pull request as ready for review January 22, 2025 11:37

mooktakim temporarily deployed to review January 22, 2025 11:42 — with GitHub Actions Inactive

mooktakim temporarily deployed to review January 22, 2025 12:08 — with GitHub Actions Inactive

cwrw assigned leandroalemao and ethax-ross Jan 22, 2025

mooktakim force-pushed the 3913-remove-any-remnants-of-npq-in-ecf-app branch from a51bd78 to c64a04d Compare January 22, 2025 15:37

mooktakim temporarily deployed to review January 22, 2025 15:42 — with GitHub Actions Inactive

leandroalemao reviewed Jan 22, 2025

View reviewed changes

cwrw reviewed Jan 23, 2025

View reviewed changes

app/models/cohort.rb Show resolved Hide resolved

cwrw reviewed Jan 23, 2025

View reviewed changes

ethax-ross reviewed Jan 24, 2025

View reviewed changes

mooktakim force-pushed the 3913-remove-any-remnants-of-npq-in-ecf-app branch from c64a04d to b1e5fc5 Compare January 24, 2025 17:08

mooktakim temporarily deployed to review January 24, 2025 17:11 — with GitHub Actions Inactive

mooktakim temporarily deployed to review January 24, 2025 17:24 — with GitHub Actions Inactive

mooktakim mentioned this pull request Jan 27, 2025

[CPDLP-3913] add npq_registration_start_date to ignored_columns #5479

Closed

mooktakim added 2 commits January 27, 2025 12:57

[CPDLP-3913] Removed REMOVE_NPQ_REFERENCES and related, turn on by de…

060b231

…fault

[CPDLP-3913] removed NPQ css

df6961b

mooktakim added 19 commits January 27, 2025 12:57

[CPDLP-3913] removed NPQ form services

f057bf6

[CPDLP-3913] removed NPQ logic from Cohort

8dfb76b

[CPDLP-3913] removed NPQ locales

4d113c8

[CPDLP-3913] removed NPQ payment calculators

2ab9967

[CPDLP-3913] removed NPQ column from create cohort service

98147e4

[CPDLP-3913] removed NPQ parts from docs renderer

9bded3c

[CPDLP-3913] removed NPQ from Importers::CreateSchedule

f9380d7

[CPDLP-3913] removed NPQ mention on view

f704335

[CPDLP-3913] removed unused shared examples

36d2aa9

[CPDLP-3913] removed QUALIFIED_TEACHERS_API_URL & QUALIFIED_TEACHERS_…

f86c79b

…API_KEY from rails credentials

[CPDLP-3913] remove column Cohort.npq_registration_start_date

c3bc882

[CPDLP-3913] removed 'npq_capping' feature

8881fd9

[CPDLP-3913] removed NPQ content

30fca4b

[CPDLP-3913] removed use 'schemas_data

e45d9fe

[CPDLP-3913] spec fixture for api is not needed anymore

4727c3a

[CPDLP-3913] brought back 'npq_plus_one_or_earlier?' and renamed to '…

72db384

…start_year_2020_or_earlier?'

[CPDLP-3913] Ignore npq_registration_start_date only, remove column o…

7268d59

…n saparate pr

[CPDLP-3913] schema.rb

44b7a3f

[CPDLP-3913] updated docs renderer with realistic test

2d0eab0

mooktakim force-pushed the 3913-remove-any-remnants-of-npq-in-ecf-app branch from 11c5f2d to 2d0eab0 Compare January 27, 2025 14:04

mooktakim mentioned this pull request Jan 27, 2025

[CPDLP-3913] migration to remove cohorts.npq_registration_start_date #5485

Merged

mooktakim had a problem deploying to review January 27, 2025 14:09 — with GitHub Actions Failure

mooktakim temporarily deployed to review January 27, 2025 14:27 — with GitHub Actions Inactive

cwrw approved these changes Jan 27, 2025

View reviewed changes

mooktakim added this pull request to the merge queue Jan 27, 2025

Merged via the queue into main with commit e381012 Jan 27, 2025
37 checks passed

mooktakim deleted the 3913-remove-any-remnants-of-npq-in-ecf-app branch January 27, 2025 14:50

mooktakim temporarily deployed to review January 27, 2025 14:50 — with GitHub Actions Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CPDLP-3913] Remove any remnants of NPQ in ECF app #5461

[CPDLP-3913] Remove any remnants of NPQ in ECF app #5461

mooktakim commented Jan 21, 2025 •

edited

Loading

github-actions bot commented Jan 21, 2025

leandroalemao Jan 22, 2025

mooktakim Jan 23, 2025

cwrw Jan 23, 2025

leandroalemao left a comment

mooktakim commented Jan 23, 2025

cwrw left a comment

cwrw Jan 23, 2025

ethax-ross left a comment

cwrw commented Jan 24, 2025

cwrw left a comment

[CPDLP-3913] Remove any remnants of NPQ in ECF app #5461

[CPDLP-3913] Remove any remnants of NPQ in ECF app #5461

Conversation

mooktakim commented Jan 21, 2025 • edited Loading

Context

Changes proposed in this pull request

Guidance to review

github-actions bot commented Jan 21, 2025

leandroalemao Jan 22, 2025

Choose a reason for hiding this comment

mooktakim Jan 23, 2025

Choose a reason for hiding this comment

cwrw Jan 23, 2025

Choose a reason for hiding this comment

leandroalemao left a comment

Choose a reason for hiding this comment

mooktakim commented Jan 23, 2025

cwrw left a comment

Choose a reason for hiding this comment

cwrw Jan 23, 2025

Choose a reason for hiding this comment

ethax-ross left a comment

Choose a reason for hiding this comment

cwrw commented Jan 24, 2025

cwrw left a comment

Choose a reason for hiding this comment

mooktakim commented Jan 21, 2025 •

edited

Loading