-
Notifications
You must be signed in to change notification settings - Fork 13
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
[Issue #1364] Update API schema with modified DB schema fields #1448
Changes from all commits
f9c051d
b31a094
99b24fe
86ee26d
4aa3731
e585861
579de15
e3b1b2e
111dc77
65f6add
869f62c
34b431e
c361ef2
64d7aa6
b7f5985
7ab8873
95ea3cb
8f7635a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,8 @@ def search_opportunities( | |
# TODO - when we want to sort by non-opportunity table fields we'll need to change this | ||
.order_by(sort_fn(getattr(Opportunity, search_params.pagination.order_by))) | ||
.where(Opportunity.is_draft.is_(False)) # Only ever return non-drafts | ||
# Filter anything without a current opportunity summary | ||
.where(Opportunity.current_opportunity_summary != None) # noqa: E711 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes it so we only pull opportunity records that have a summary when we make the request? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Opportunities have to have a current opportunity summary (and thus an opportunity status which is that table) in order to be searchable. This is deliberate to avoid showing opportunities before their summaries, which are really the public notices of funding, are actually live/public. Note this doesn't mean the opportunity is entirely hidden, if you navigate to the page of an opportunity directly, you would be able to find it. |
||
.options(joinedload("*")) # Automatically load all relationships | ||
) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,16 @@ def _build_opportunities(db_session: db.Session) -> None: | |
factories.OpportunityFactory.create_batch(size=5, is_archived_forecast_summary=True) | ||
factories.OpportunityFactory.create_batch(size=5, no_current_summary=True) | ||
|
||
# generate a few opportunities with mostly null values | ||
all_null_opportunities = factories.OpportunityFactory.create_batch(size=5, all_fields_null=True) | ||
for all_null_opportunity in all_null_opportunities: | ||
summary = factories.OpportunitySummaryFactory.create( | ||
all_fields_null=True, opportunity=all_null_opportunity | ||
) | ||
factories.CurrentOpportunitySummaryFactory.create( | ||
opportunity=all_null_opportunity, opportunity_summary=summary | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
logger.info("Finished creating opportunities") | ||
|
||
logger.info("Creating records in the transfer_topportunity table") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these be removed if they're not added back anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed them from the model to avoid any confusion as the summary object has similar values. While I don't know what we want to do with these long-term, the current system returns the summary values, so I removed these in the top-level opportunity for now.