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

CLS2-849 change specific programme field to be a many to many field part 1 #5478

Conversation

Richard-Pentecost
Copy link
Contributor

@Richard-Pentecost Richard-Pentecost commented Jun 11, 2024

Description of change

Add new specific_programmes field to the investment project model that is a many to many field and create a migration. Add functionality such that when someone adds or changes the selected_programme for an investment project from the frontend, it updates both the current and the new field with the new value. Remove the ability to change the specific_programme field from django admin whilst this migration process is going on.

Checklist

  • Has this branch been rebased on top of the current main branch?

    Explanation

    The branch should not be stale or have conflicts at the time reviews are requested.

  • Is the CircleCI build passing?

General points

Other things to check

  • Make sure fixtures/test_data.yaml is maintained when updating models
  • Consider the admin site when making changes to models
  • Use select-/prefetch-related field lists in views and search apps, and update them when fields are added
  • Make sure the README is updated e.g. when adding new environment variables

See docs/CONTRIBUTING.md for more guidelines.

@Richard-Pentecost Richard-Pentecost requested a review from a team as a code owner June 11, 2024 19:02
@codecov-commenter
Copy link

codecov-commenter commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.38%. Comparing base (0f6c910) to head (1ffd2f8).
Report is 7 commits behind head on main.

Files Patch % Lines
datahub/investment/project/serializers.py 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5478      +/-   ##
==========================================
- Coverage   96.40%   96.38%   -0.02%     
==========================================
  Files         952      953       +1     
  Lines       22504    22516      +12     
  Branches     2021     2022       +1     
==========================================
+ Hits        21695    21703       +8     
- Misses        658      660       +2     
- Partials      151      153       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@oliverjwroberts oliverjwroberts left a comment

Choose a reason for hiding this comment

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

PR makes first step into changing InvestmentProject.specific_programme, which is currently a foreign key, to a many-to-many field InvestmentProject.specific_programmes. Looks good to me.

Good spot with the multi-step approach (as recommended by docs/How to change the type of a field.md for those that didn't know). I'd be more than happy to assist with subsequent steps, just let me know!

datahub/investment/project/models.py Outdated Show resolved Hide resolved
…t project model that is a many to many field
@Richard-Pentecost Richard-Pentecost force-pushed the feature/CLS2-849-investment-projects-record-multiple-programmes-part-1 branch from 6912538 to 1ffd2f8 Compare June 12, 2024 09:29
@Richard-Pentecost Richard-Pentecost merged commit 5c4d964 into main Jun 12, 2024
2 checks passed
@Richard-Pentecost Richard-Pentecost deleted the feature/CLS2-849-investment-projects-record-multiple-programmes-part-1 branch June 12, 2024 15:37
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