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

enforce consistency between has_apc and the apc data itself, and prov… #2429

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

richard-jones
Copy link
Contributor

…ide a migration to fix any existing inconsistent records


enforce consistency between has_apc and the apc data itself

has_apc = False did not cause the apc data in the record to be removed, which was resulting in an inconsistent record, and oddities in display

This PR...

  • has scripts to run
  • has migrations to run
  • adds new infrastructure
  • changes the CI pipeline
  • affects the public site
  • affects the editorial area
  • affects the publisher area
  • affects the monitoring

Developer Checklist

Developers should review and confirm each of these items before requesting review

  • Code meets acceptance criteria from issue
  • Unit tests are written and all pass
    * [ ] User Test Scripts (if required) are written and have been run through
  • Project's coding standards are met
    • No deprecated methods are used
    • No magic strings/numbers - all strings are in constants or messages files
    • ES queries are wrapped in a Query object rather than inlined in the code
    • Where possible our common library functions have been used (e.g. dates manipulated via dates)
    • Cleaned up commented out code, etc
    • Urls are constructed with url_for not hard-coded
  • Code documentation and related non-code documentation has all been updated
  • Migation has been created and tested
  • There is a recent merge from develop

Reviewer Checklist

Reviewers should review and confirm each of these items before approval
If there are multiple reviewers, this section should be duplicated for each reviewer

  • Code meets acceptance criteria from issue
  • Unit tests are written and all pass
  • User Test Scripts (if required) are written and have been run through
  • Project's coding standards are met
    • No deprecated methods are used
    • No magic strings/numbers - all strings are in constants or messages files
    • ES queries are wrapped in a Query object rather than inlined in the code
    • Where possible our common library functions have been used (e.g. dates manipulated via dates)
    • Cleaned up commented out code, etc
    • Urls are constructed with url_for not hard-coded
  • Code documentation and related non-code documentation has all been updated
  • Migation has been created and tested
  • There is a recent merge from develop

Testing

No test scripts to be run.
Nonetheless, I was unable to reproduce this issue locally via the forms, and would need a crafted CSV to demonstrate the issue, which I don't have. The fix is therefore speculative in the sense that I have not been able to produce the issue locally and then demonstrate that this fixes it.
If it's possible to replicate live on test and run the migration to show the fix, that would be very useful.

Deployment

Migrations

See:

portality/migrate/20241031_4005_apc_model_consistency/

…ide a migration to fix any existing inconsistent records
@richard-jones richard-jones changed the base branch from develop to master October 31, 2024 10:21
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.

1 participant