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

Replace instances of PGDE with QTS / change API course summary #4645

Merged
merged 3 commits into from
Nov 11, 2024

Conversation

tomas-stefano
Copy link
Contributor

@tomas-stefano tomas-stefano commented Nov 1, 2024

Context

There is inconsistency of terminology between GiT and Find. GiT use “QTS with PGDE” (which is more clear/intuitive) whereas Find uses “PGDE with QTS”.

Changes

This PR replaces all instances of PGDE with QTS to QTS with PGDE.

Coordination between other teams

It was decided by Apply and Find teach leads to change the API content of the course description to the expected ones

  1. PGCE with QTS becomes QTS with PGCE
  2. PGDE with QTS becomes QTS with PGDE

In order to coordinate this change with this other team this PR:

  1. Changes everything in the UI
  2. Uses a feature flag for the course.summary field in the API.

So this is the plan to deploy this:

  • Merge the feature flag PR
  • Merge the code that uses the feature flag (This one!)
  • Turn on the feature flag on QA when having a session with Register & Apply (PR)
  • Have the Register & Apply sign off that everything worked
  • Turn on the feature flag on Production (PR) and also deploy the API docs change here about the content change on API on the history/updates section.
  • Cleanup the feature flag (PR to be added here)

@tomas-stefano
Copy link
Contributor Author

@tomas-stefano tomas-stefano force-pushed the td/33-replace-instances-of-pgde-with-qts-changes branch 2 times, most recently from f0c013f to d6b407e Compare November 4, 2024 12:24
@tomas-stefano tomas-stefano marked this pull request as ready for review November 4, 2024 12:30
@tomas-stefano tomas-stefano requested a review from a team as a code owner November 4, 2024 12:30
<% when "QTS" %>
<%= govuk_details(summary_text: "QTS") do %>
<% case course.qualification %>
<% when 'qts' %>
Copy link
Contributor Author

@tomas-stefano tomas-stefano Nov 4, 2024

Choose a reason for hiding this comment

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

I changed the way it was done slightly.

Before: it was using the translation to do a case for another translation.
After: Passing the course qualification and then translate.

I thought to split into partials but then I rollback because the code will be something like:

View -> Component -> Another component -> Multiple partials

@@ -516,11 +516,20 @@ def program_type_description
def description
return qualifications_description if teacher_degree_apprenticeship?

study_mode_string = (full_time_or_part_time? ? ', ' : ' ') +
study_mode_description
qualifications_description + study_mode_string + program_type_description
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know the summary above is almost a duplication of this description but my plan is to remove this description field after the feature flag is removed.

Copy link
Contributor

@gms-gs gms-gs left a comment

Choose a reason for hiding this comment

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

:shipit:

This is done behind a feature flag.

The course.description and qualifications_description will be deleted
when we cleanup the feature flag after the feature flag is enable in
production.
Talked with content designer and we agreed to change these places
to follow the pattern QTS with PGDE
@tomas-stefano tomas-stefano force-pushed the td/33-replace-instances-of-pgde-with-qts-changes branch from d6b407e to c5f8792 Compare November 11, 2024 12:08
@tomas-stefano tomas-stefano merged commit 435a353 into main Nov 11, 2024
19 checks passed
@tomas-stefano tomas-stefano deleted the td/33-replace-instances-of-pgde-with-qts-changes branch November 11, 2024 15:07
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