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.

Sign up for GitHub

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

Jump to bottom

Implement GET /proposals/{proposalId} endpoint #101

Closed
slifty opened this issue Oct 18, 2022 · 6 comments
Closed

Implement GET /proposals/{proposalId} endpoint #101

slifty opened this issue Oct 18, 2022 · 6 comments
Assignees
@slifty

Comments

@slifty
Copy link
Member

slifty commented Oct 18, 2022 •
edited

As mentioned in #34 we need the ability to load a specific proposal. We have explored the pattern of populating deep fields when loading a single ID and I think it would be appropriate to do so here as well, which would mean the returned proposal would include the various proposal versions and proposal field values.
@slifty slifty self-assigned this Oct 18, 2022
@slifty slifty mentioned this issue Oct 19, 2022
Closed
@reefdog reefdog mentioned this issue Dec 14, 2022
Open
bickelj added a commit that referenced this issue Dec 30, 2022
@bickelj
Add GET /proposals/{proposalId} endpoint
cc3e1c6 
A call to /proposals/{proposalId} returns a single proposal with its
immediate attributes.

Issue #101 Implement GET /proposals/{proposalId} endpoint
@bickelj bickelj mentioned this issue Dec 30, 2022
Merged
@bickelj
Copy link
Contributor

bickelj commented Dec 30, 2022

Had we originally decided to return deep objects on all the single-id endpoints? Or should we default to shallow objects on all endpoints and then offer deep objects on some, as suggested around #157 (comment)? PR #192 assumes shallow objects to be the default. The next step would be to allow deep objects when specified.

bickelj added a commit that referenced this issue Dec 30, 2022
@bickelj
Add GET /proposals/{proposalId} endpoint
bf2d99d 
A call to /proposals/{proposalId} returns a single proposal with its
immediate attributes.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Dec 30, 2022
@bickelj
Add GET /proposals/{proposalId} endpoint
c21ac7c 
A call to /proposals/{proposalId} returns a single proposal with its
immediate attributes.

Issue #101 Implement GET /proposals/{proposalId} endpoint
@bickelj
Copy link
Contributor

bickelj commented Jan 5, 2023 •
edited

I am still working on the deep objects code and tests. But here is an example response when asking to include fields and values with curl ... http://host/proposals/1?includeFieldsAndValues=true | jq:

{
  "id": 1,
  "applicantId": 1,
  "opportunityId": 1,
  "externalId": "AnIdGeneratedByAGms",
  "createdAt": "2022-11-10T20:07:04.695Z",
  "versions": [
    {
      "id": 2,
      "proposalId": 1,
      "applicationFormId": 4,
      "version": 1,
      "createdAt": "2023-01-04T16:28:47.276Z",
      "fieldValues": [
        {
          "id": 1,
          "proposalVersionId": 2,
          "applicationFormFieldId": 3,
          "position": 1,
          "value": "Jones",
          "createdAt": "2023-01-04T16:29:57.877Z",
          "applicationFormField": {
            "id": 3,
            "applicationFormId": 4,
            "canonicalFieldId": 280,
            "position": 1,
            "label": "Family Name",
            "createdAt": "2023-01-04T16:27:40.175Z"
          }
        },
        {
          "id": 2,
          "proposalVersionId": 2,
          "applicationFormFieldId": 4,
          "position": 2,
          "value": "Jim",
          "createdAt": "2023-01-04T16:30:08.974Z",
          "applicationFormField": {
            "id": 4,
            "applicationFormId": 4,
            "canonicalFieldId": 279,
            "position": 2,
            "label": "Given Name",
            "createdAt": "2023-01-04T16:27:40.175Z"
          }
        }
      ]
    }
  ]
}

bickelj added a commit that referenced this issue Jan 9, 2023
@bickelj
Add GET /proposals/{proposalId} endpoint
48f1971 
A call to /proposals/{proposalId} returns a single proposal with its
immediate attributes.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 9, 2023
@bickelj
Add optional param to GET /proposals/{proposalId}
5582675 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
@bickelj bickelj mentioned this issue Jan 9, 2023
Closed
bickelj added a commit that referenced this issue Jan 9, 2023
@bickelj
Add GET /proposals/{proposalId} endpoint
71a16b3 
A call to /proposals/{proposalId} returns a single proposal with its
immediate attributes.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 9, 2023
@bickelj
Add optional param to GET /proposals/{proposalId}
049bbd8 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 11, 2023
@bickelj
Add GET /proposals/{proposalId} endpoint
8b47346 
A call to /proposals/{proposalId} returns a single proposal with its
immediate attributes.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 16, 2023
@bickelj
Add GET /proposals/{proposalId} endpoint
bddae03 
A call to /proposals/{proposalId} returns a single proposal with its
immediate attributes.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 16, 2023
@bickelj
Add optional param to GET /proposals/{proposalId}
01a0e71 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
@bickelj
Copy link
Contributor

bickelj commented Jan 18, 2023

Yesterday I got a draft of the alternative approach at #198 (comment) working locally, although it is in a less-than-elegant state right now. My goal is to clean it up a bit today. There will need to be a fourth query to include the application form fields that relate to the proposal values, too.

@bickelj
Copy link
Contributor

bickelj commented Jan 19, 2023 •
edited

Using async/await (versus .then(.then(.then()))) really helped clean up code that runs multiple queries concurrently.

bickelj added a commit that referenced this issue Jan 20, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
f8105b4 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
@bickelj bickelj mentioned this issue Jan 20, 2023
Merged
bickelj added a commit that referenced this issue Jan 20, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
fcd5ea6 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 23, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
abc0291 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 23, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
6ce9d01 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 24, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
c9e413b 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 24, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
42a1e98 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 30, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
bf75d07 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Jan 30, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
c602b6a 
Simplify implementation further based on review feedback.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Feb 2, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
ca13026 
Simplify implementation further based on review feedback.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Feb 2, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
c8097c0 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Feb 2, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
0d56542 
Simplify implementation further based on review feedback.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Feb 7, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
0da52e4 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Feb 7, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
9b3bdb1 
Simplify implementation further based on review feedback.

Issue #101 Implement GET /proposals/{proposalId} endpoint
bickelj added a commit that referenced this issue Feb 7, 2023
@bickelj
Add option to deep GET /proposals/{proposalId}
551be26 
When optional query parameter includeFieldsAndValues is set to true on
GET /proposals/{proposalId}, include all proposal versions, associated
values, and associated application form fields in the response.

By returning this almost-fully-deep object tree, it is more convenient
for the caller and more efficient in terms of request, query, and
response counts. The assumption is that the purpose of the PDC is to
see and compare which fields are used for what purpose for proposals.

This commit consolidates incremental changes that should be seen here:
#210

Issue #101 Implement GET /proposals/{proposalId} endpoint
@bickelj
Copy link
Contributor

bickelj commented Feb 14, 2023 •
edited

There are two more things on the /proposals endpoints before I am comfortable closing this issue.

  1. OpenAPI documentation updates
  2. If throw error inside a catch inside a then does not propagate to the outer promise chain, we need to call next inside the innermost catch blocks.

bickelj added a commit that referenced this issue Feb 14, 2023
@bickelj
Fix and document GET /proposals/{id} endpoint
699a2e4 
Add more complete details to the OpenAPI documentation, specifically
about the `includeFieldsAndValues` query parameter. Also fix a
potential bug where a promise could be left hanging.

Issue #101 Implement GET /proposals/{proposalId} endpoint
@bickelj bickelj mentioned this issue Feb 14, 2023
Merged
bickelj added a commit that referenced this issue Feb 14, 2023
@bickelj
Fix and document GET /proposals/{id} endpoint
f76cde9 
Add more complete details to the OpenAPI documentation, specifically
about the `includeFieldsAndValues` query parameter. Also fix a
potential bug where a promise could be left hanging.

Issue #101 Implement GET /proposals/{proposalId} endpoint
@bickelj
Copy link
Contributor

bickelj commented Feb 27, 2023

Resolved by merge of #192, #210, and #232.

@bickelj bickelj closed this as completed Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@slifty slifty

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@slifty @bickelj