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

[Forecasted Pipeline] Add forecast-specific fields to OpenAPI schema definition #874

Closed
TylerHendrickson opened this issue Jul 12, 2024 · 8 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request Grant Finder

Comments

@TylerHendrickson
Copy link
Member

TylerHendrickson commented Jul 12, 2024

Blocked by

N/A

Blocks

Why is this issue important?

We want to ensure that published GrantModificationEvents emitted by this service are well-defined for downstream consumers. Now that we are planning to include data about forecasted grant opportunities in these events, the OpenAPI schema definition should be updated to reflect the new data fields.

Current State

Fields specific to forecasted grant opportunities are absent from the openapi/openapi.yaml file, which serves as the official spec for events published to AWS EventBridge by the PublishGrantEvents Lambda function.

Expected State

The OpenAPI schema includes definitions for the new fields. (See #330, along with usdigitalresponse/usdr-gost#3212 (comment) for more details) on the fields that need to be documented.

Implementation Plan

Update the Grant object type schema definition.

While not set in stone (I'm certainly open to discussion), consider the following proposal for how to incorporate the new fields:

  • Add the following fields to the Grant object schema:
    • fiscal_year
      • not required
      • string type, with year format
      • min/max (i.e. fixed) length of 4 characters
      • regex pattern ^\d{4}$
  • Add the following fields to the GrantorContact object schema:
    • name
      • not required
      • string type
    • phone
      • not required
      • string type
  • Add the following fields to the OpportunityMilestones object schema:
    • award_date
      • not required
      • string type, with date format
    • project_start_date
      • not required
      • string type, with date format
    • forecast_creation_date
      • not required
      • string type, with date format

Relevant Code Snippets

No response

@TylerHendrickson TylerHendrickson added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 12, 2024
@masimons
Copy link

@TylerHendrickson which field does forecasted_estimations map to in the list of new columns to be added in 3212 ? Just trying to understand the diff between this list and the one Jeff drafted in that ticket, where there are proposed columns estimated_synopsis_close_date_explanation and is_forecasted.

@TylerHendrickson
Copy link
Member Author

@masimons Just recapping what we discussed in the meeting today:

  • In the proposal above, forcasted_estimations maps to a a new container object type (proposed name ForecastedEstimations in the OP) type for other fields to-be-populated from values pulled from the source XML:
    • EstimatedAwardDate -> .forecasted_estimations.award_date
    • EstimatedProjectStartDate -> .forecasted_estimations.project_start_date
    • EstimatedSynopsisCloseDate -> .forecasted_estimations.synopsis_close.date (where .forcasted_estimations.synopsis_close_date is itself an object of type CloseDate)
    • EstimatedSynopsisCloseDateExplanation -> .forecasted_estimations.synopsis_close.explanation (where .forcasted_estimations.synopsis_close_date is itself an object of type CloseDate)
    • EstimatedSynopsisPostDate -> .forecasted_estimations.post_date
  • Some of these fields are arguably duplicitous – the proposal treats them as distinct fields ("these are the values which are estimated for the forecast") which will presumably eventually align with extant fields once the Grant is no longer forecasted
    • e.g. .forecasted_estimations.synopsis_close.date is relevant for forecasted grants, but once the grant is fully published (and no longer a forecast), the relevant value would be .opportunity_milestones.close.date.
    • Therefore, there's an argument that, contrary to my above proposal, we reuse the values (e.g. just populate EstimatedSynopsisCloseDate -> .opportunity_milestones.close.date until the grant is no longer being forecasted, at which point the mapping becomes CloseDate -> opportunity_milestones.close.date).
      • I can think of reasons why it would be useful to treat them as distinct, but perhaps reusing existing fields would help promote clearer semantics – in particular, that a forecasted grant is a grant that just hasn't been opened yet, is still subject to change, and therefore has estimated values instead of official ones.
  • IIRC is_forecasted was meant to just be a helper value on the Finder side to help disambiguate database records in the grants table. Rather than creating a discrete field on this schema, I think it would be better to derive that based on whether post_date is in the past or future.

1 similar comment
@TylerHendrickson
Copy link
Member Author

@masimons Just recapping what we discussed in the meeting today:

  • In the proposal above, forcasted_estimations maps to a a new container object type (proposed name ForecastedEstimations in the OP) type for other fields to-be-populated from values pulled from the source XML:
    • EstimatedAwardDate -> .forecasted_estimations.award_date
    • EstimatedProjectStartDate -> .forecasted_estimations.project_start_date
    • EstimatedSynopsisCloseDate -> .forecasted_estimations.synopsis_close.date (where .forcasted_estimations.synopsis_close_date is itself an object of type CloseDate)
    • EstimatedSynopsisCloseDateExplanation -> .forecasted_estimations.synopsis_close.explanation (where .forcasted_estimations.synopsis_close_date is itself an object of type CloseDate)
    • EstimatedSynopsisPostDate -> .forecasted_estimations.post_date
  • Some of these fields are arguably duplicitous – the proposal treats them as distinct fields ("these are the values which are estimated for the forecast") which will presumably eventually align with extant fields once the Grant is no longer forecasted
    • e.g. .forecasted_estimations.synopsis_close.date is relevant for forecasted grants, but once the grant is fully published (and no longer a forecast), the relevant value would be .opportunity_milestones.close.date.
    • Therefore, there's an argument that, contrary to my above proposal, we reuse the values (e.g. just populate EstimatedSynopsisCloseDate -> .opportunity_milestones.close.date until the grant is no longer being forecasted, at which point the mapping becomes CloseDate -> opportunity_milestones.close.date).
      • I can think of reasons why it would be useful to treat them as distinct, but perhaps reusing existing fields would help promote clearer semantics – in particular, that a forecasted grant is a grant that just hasn't been opened yet, is still subject to change, and therefore has estimated values instead of official ones.
  • IIRC is_forecasted was meant to just be a helper value on the Finder side to help disambiguate database records in the grants table. Rather than creating a discrete field on this schema, I think it would be better to derive that based on whether post_date is in the past or future.

@masimons
Copy link

masimons commented Jul 19, 2024

@TylerHendrickson Thanks for elaborating on this! I agree that is_forecasted is probably better off derived.

While I was originally thinking that it might be nice to have a bit of historical data about estimations so that we could, at some point, get an idea of how "far off" the estimated dates are, your point about those fields eventually aligning would remove that ability. The more I think about this, the more I think that "re-using" current fields is the cleanest way to go.

That being said, if an estimated grant is removed by grants.gov before the open date, then our system wouldn't know about it from what I understand, and we may erroneously treat it as active when the open date rolls around. Perhaps an edge case that won't ever happen, but, we can just use a different field for the estimated open date, which may be what you had in mind anyways.

Looks like this is at least a possibility fwiw - dropping this link for future reference: https://apply07.grants.gov/help/html/help/index.htm#t=Grantors%2FDeleteGrantOpportunities.htm

@TylerHendrickson
Copy link
Member Author

@masimons

The more I think about this, the more I think that "re-using" current fields is the cleanest way to go.

I think I agree; I'll update the proposal on this OP for this issue. Thanks for the feedback!

(cc @joshgarza – I'll flag in a separate comment once I've made those changes to the ticket so you know this is ready for work.)

@TylerHendrickson
Copy link
Member Author

Sprinkling some documentation assets around:

@masimons
Copy link

masimons commented Aug 2, 2024

@TylerHendrickson just wondering if you and josh were able to chat this week about how to model the estimated end date and open/post date for the forecasted grants? have we solidified that yet?

@TylerHendrickson
Copy link
Member Author

@masimons Just a heads-up that the implementation plan for this ticket has been revised (should be for the last time).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request Grant Finder
Projects
Archived in project
Development

No branches or pull requests

4 participants