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

Allow providers to csv export all their application choices #10352

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

CatalinVoineag
Copy link
Contributor

@CatalinVoineag CatalinVoineag commented Feb 7, 2025

Context

Previously, we allowed providers to only export their applications
choices only from the current and previous cycle

This commit allows them to export every recruitment cycle they have
application choices for.

Changes proposed in this pull request

Application export

Guidance to review

Go on review app, log as a provider and export applications that are at least 3 cycles ago.

The provider NIoT@Harris Initial Teacher Education has 2023 applications to export, 2023, 2 cycles ago was not possible to export.

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, if included inform data insights team of the changes
  • If this code adds a column that may include PII, the sanitise.sql script and 0025-protecting-personal-data-in-production-dump.md ADR have been updated
  • API release notes have been updated if necessary
  • If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • Attach the PR to the Trello card

Previously, we allowed providers to only export their applications
choices only from the current and previous cycle

This commit allows them to export every recruitment cycle they have
application choices for.
@CatalinVoineag CatalinVoineag self-assigned this Feb 7, 2025
@CatalinVoineag CatalinVoineag added the deploy_v2 Deploy the review app to AKS label Feb 7, 2025
@CatalinVoineag
Copy link
Contributor Author

@CatalinVoineag CatalinVoineag marked this pull request as ready for review February 7, 2025 11:10
@github-actions github-actions bot temporarily deployed to review_aks-10352 February 7, 2025 11:25 Destroyed
spec/rails_helper.rb Outdated Show resolved Hide resolved
@CatalinVoineag CatalinVoineag requested a review from a team February 7, 2025 12:30
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.

This looks great, thanks for adding the line about getting the cycles in the review apps!

One little UI thing: I think it would be better to have the years in reverse order, the current year on top:
image

But I don't feel strongly about it.

The other thing is I'm not sure about reseeding the timetables in the rails_helper. Right now, you note something as a :feature test, it will automatically sed the timetables and set the current time to mid cycle.

If you set the time on any of the tests using the CycleTimetableHelper (eg mid_cycle), it will make sure there are timetables seeded.

In the rare cases where they are needed and not covered by these scenarios, you can just add seed_timetables in the describe line of a test.

I would prefer this method to using the data migration, because how we seed the timetables will evolve as we stop relying on them being defined in a yml file.

@github-actions github-actions bot temporarily deployed to review_aks-10352 February 7, 2025 13:48 Destroyed
@CatalinVoineag
Copy link
Contributor Author

CatalinVoineag commented Feb 7, 2025

This looks great, thanks for adding the line about getting the cycles in the review apps!

One little UI thing: I think it would be better to have the years in reverse order, the current year on top: image

But I don't feel strongly about it.

The other thing is I'm not sure about reseeding the timetables in the rails_helper. Right now, you note something as a :feature test, it will automatically sed the timetables and set the current time to mid cycle.

If you set the time on any of the tests using the CycleTimetableHelper (eg mid_cycle), it will make sure there are timetables seeded.

In the rare cases where they are needed and not covered by these scenarios, you can just add seed_timetables in the describe line of a test.

I would prefer this method to using the data migration, because how we seed the timetables will evolve as we stop relying on them being defined in a yml file.

I've reversed sorted the recruitment cycle years b4d00c9

providers: providers_that_actor_belongs_to,
recruitment_cycle_year: RecruitmentCycleTimetable.pluck(:recruitment_cycle_year),
)
choices.map(&:current_recruitment_cycle_year).uniq.sort.reverse
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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.

Beautiful! So great to have this out so quickly -- a provider only asked for it on Wednesday.

@github-actions github-actions bot temporarily deployed to review_aks-10352 February 7, 2025 14:04 Destroyed
@CatalinVoineag CatalinVoineag merged commit 7dbcc34 into main Feb 7, 2025
23 checks passed
@CatalinVoineag CatalinVoineag deleted the cv/expand-application-exports branch February 7, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy_v2 Deploy the review app to AKS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants