Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Implement Source model - Step 1 #856

Merged
merged 3 commits into from Oct 10, 2019
Merged

Implement Source model - Step 1 #856

merged 3 commits into from Oct 10, 2019

Conversation

jwalgran
Copy link
Contributor

@jwalgran jwalgran commented Oct 4, 2019

Overview

The first step of a three step process to add a Source model between facility lists and list items, allowing for the submission of individual facilities.

Each step in the process is intended to be released separately and maintain backward compatibility.

Connects #817

Testing Instructions

  • Checkout develop and ./scripts/resetdb
  • Checkout this branch and ./scripts/migrate
  • Ensure that ./scripts/manage liststosources runs without error
  • Use ./scripts/manage shell_plus to verify that a Source is created for each FacilityList and that a Source is assigned on each FacilityListItem
>>> list(FacilityList.objects.all().order_by('id').values_list('id', flat=True))
[2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
>>> list(Source.objects.all().order_by('facility_list').values_list('facility_list', flat=True))
[2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]
>>> FacilityListItem.objects.filter(source=None).count()
0

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@jwalgran
Copy link
Contributor Author

jwalgran commented Oct 4, 2019

This has a migration conflict. Fixing now.

@jwalgran
Copy link
Contributor Author

jwalgran commented Oct 4, 2019

Fixed the migration conflict and rebased.

Copy link
Contributor

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested. I don't have a clear sense of the larger design change that this is serving, so can't comment on the context, but this implementation is correct.

@rajadain rajadain removed their assignment Oct 4, 2019
@jwalgran
Copy link
Contributor Author

jwalgran commented Oct 4, 2019

Thank you for the review.

I don't have a clear sense of the larger design change that this is serving

This is the highest level view of what this set of pulls it trying to achieve.

Screen Shot 2019-10-04 at 12 53 09 PM

In order to allow submission of individual facilities we are inserting Source in between Contributor and FacilityListItem rather than FacilitiyList. The FacilityList table becomes a place where we store additional metadata about a Source if the the Source is a list.

#857 is where all of the major work happens. #858 is "cleanup." I will be assigning those for review shortly.

Copy link
Contributor

@kellyi kellyi left a comment

Choose a reason for hiding this comment

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

👍 Ran through the testing instructions and everything worked as described. I did not see any lists without sources.

@kellyi kellyi assigned jwalgran and unassigned kellyi Oct 4, 2019
Starts the process of adding support for single-item submissions by introducing
a model that can connect a `FacilityListItem` to a `Contributor` without needing
a `FacilityList`

`FacilityListItem.source` is nullable for now to make the migration backward
compatible.
This command will be used as part of the backward compatible migration process.
It creates or updates `Source` objects from `FacilityList` objects and also
updates the `source` foreign key on related `FacilityListItems`.

It is intended that this command be removed after deployed application code is
updated to create and reference `Source` objects.
@jwalgran
Copy link
Contributor Author

Thanks for the reviews.

@jwalgran jwalgran merged commit 853124e into develop Oct 10, 2019
@jwalgran jwalgran deleted the jcw/source-model-1 branch November 6, 2019 18:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants