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
2505 intake unique check on passthroguh nameid number #3442
Draft
sambodeme
wants to merge
6
commits into
main
Choose a base branch
from
2505-intake-unique-check-on-passthroguh-nameid-number
base: main
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
2505 intake unique check on passthroguh nameid number #3442
sambodeme
wants to merge
6
commits into
main
from
2505-intake-unique-check-on-passthroguh-nameid-number
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Terraform plan for meta No changes. Your infrastructure matches the configuration.
📝 Plan generated in Pull Request Checks #2400 |
Terraform plan for dev No changes. Your infrastructure matches the configuration.
📝 Plan generated in Pull Request Checks #2400 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
#2505 requires further clarification to be completed. The main question is whether the goal is to prevent the duplication of passthrough entities across all non-direct awards, as is currently implemented, or whether the goal is more narrowly defined. Specifically, is the aim to prevent duplicates only in situations where an award is linked to a list of passthrough entities separated by a
|
, or should duplicates be avoided only when the federal program names are identical?The implementation in this PR is checking for duplicate passthrough entities across all non-direct awards. As indicated by the results on GitHub, this approach has resulted in the failure of some test cases that had previously passed. Additionally, I have tested (in local) the approach consisting of preventing duplicates when program names are identical; however, this also led to the failure of some existing tests.
While the test failures can be resolved regardless of the chosen approach, the fact that these tests were previously successful suggests the possibility of data discrepancies in the disseminated reports (once this new rule get enforced), requiring a re-run or patching. The scope of potential data issues related to dissemination will depend on the agreed upon implementation approach.
PR checklist: submitters
main
into your branch shortly before creating the PR. (You should also be mergingmain
into your branch regularly during development.)git status | grep migrations
. If there are any results, you probably need to add them to the branch for the PR. Your PR should have only one new migration file for each of the component apps, except in rare circumstances; you may need to delete some and re-runpython manage.py makemigrations
to reduce the number to one. (Also, unless in exceptional circumstances, your PR should not delete any migration files.)PR checklist: reviewers
make docker-clean; make docker-first-run && docker compose up
; then rundocker compose exec web /bin/bash -c "python manage.py test"
The larger the PR, the stricter we should be about these points.