Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix deferred course start date on offer deferred #9880

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

CatalinVoineag
Copy link
Contributor

@CatalinVoineag CatalinVoineag commented Sep 27, 2024

Context

Previously, it didn't show the actual date of the course start when it's deferred.

It was showing the date of the current cycle, not the next. So 2024 not 2025 for example

Changes proposed in this pull request

Added deferred_start_date

Guidance to review

Is the logic of the method correct?
Go on the review app and # as a candidate with a deferred offer and check the start date of your course is correct, in the next cycle, should be 2025 for a deferred offer in 2024 cycle

Screencast.2024-09-27.11.47.04.mp4

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
  • This code does not rely on migrations in the same Pull Request
  • If this code includes a migration adding or changing columns, it also backfills existing records for consistency
  • If this code adds a column to the DB, decide whether it needs to be in analytics yml file or analytics blocklist
  • API release notes have been updated if necessary
  • If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • Required environment variables have been updated added to the Azure KeyVault
  • Inform data insights team due to database changes
  • Make sure all information from the Trello card is in here
  • Rebased main
  • Cleaned commit history
  • Tested by running locally
  • Add PR link to Trello card

Previously, it didn't show the actual date of the course start when it's
deferred
@CatalinVoineag CatalinVoineag self-assigned this Sep 27, 2024
@CatalinVoineag CatalinVoineag added the deploy_v2 Deploy the review app to AKS label Sep 27, 2024
@github-actions github-actions bot temporarily deployed to review_aks-9880 September 27, 2024 09:46 Destroyed
@CatalinVoineag CatalinVoineag marked this pull request as ready for review September 27, 2024 10:51
@CatalinVoineag
Copy link
Contributor Author

Copy link
Collaborator

@inulty-dfe inulty-dfe left a comment

Choose a reason for hiding this comment

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

If the application has been deferred, there exists a course in the next cycle with a start date, right? Would it be better to get the start date for that course?

application_choice.current_course.start_date

Copy link
Contributor

@elceebee elceebee left a comment

Choose a reason for hiding this comment

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

We render this same information in the application_status_tag_component.

It is correctly rendered there.

I'm happy with the method you've written, or Iain's suggestion (I don't feel strongly about it). But it would be good to use the same method in both places.

Copy link
Collaborator

@inulty-dfe inulty-dfe left a comment

Choose a reason for hiding this comment

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

🚀

@CatalinVoineag CatalinVoineag merged commit afd21f1 into main Sep 30, 2024
60 checks passed
@CatalinVoineag CatalinVoineag deleted the cv/fix-course-start-date branch September 30, 2024 09:20
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
deploy_v2 Deploy the review app to AKS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants