Remove `Applicant` and `/applicants/*` endpoints #798

slifty · 2024-02-27T18:40:19Z

This PR removes the concept of an Applicant from the PDC, along with the endpoints and entity associations.

Related to #777
Related to #730

Note this is built off of #794 and shouldn't be merged until after that PR is merged

codecov · 2024-02-27T18:43:01Z

Codecov Report

Attention: Patch coverage is 90.42553% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 92.47%. Comparing base (eeff0cd) to head (6a82e33).

❗ Current head 6a82e33 differs from pull request most recent head 789ef97. Consider uploading reports for the commit 789ef97 to get more accurate results

Files	Patch %	Lines
src/handlers/organizationsHandlers.ts	81.81%	7 Missing and 1 partial ⚠️
...c/database/operations/create/createOrganization.ts	90.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #798      +/-   ##
==========================================
- Coverage   92.79%   92.47%   -0.32%     
==========================================
  Files          94       91       -3     
  Lines        1498     1422      -76     
  Branches      226      214      -12     
==========================================
- Hits         1390     1315      -75     
+ Misses        102      101       -1     
  Partials        6        6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

reefdog

Nice! There is a small bit of malformed sample data in the tests.

I also notice that this PR doesn't release the proposal_submitter_email requirement on bulk uploads, even though it's no longer used for anything. Totally fine if you want to consider that a follow-up downstream effect, but let me know if that's the case or if it was an oversight.

Should be a quick re-review!

CHANGELOG.md

src/database/operations/load/__test__/loadBundle.int.test.ts

slifty · 2024-02-29T15:12:25Z

src/tasks/processBulkUpload.ts

@@ -197,16 +196,6 @@ const assertBulkUploadCsvIsValid = async (csvPath: string): Promise<void> => {
 	await assertCsvContainsRowsOfEqualLength(csvPath);
 };

-export const getApplicantEmailFieldIndexForBulkUpload = async (


See here for removal of theproposal_submitter_email stuff @reefdog

slifty · 2024-02-29T15:12:37Z

I also notice that this PR doesn't release the proposal_submitter_email requirement on bulk uploads, even though it's no longer used for anything. Totally fine if you want to consider that a follow-up downstream effect, but let me know if that's the case or if it was an oversight.

Just checking... I thought that was taken out? Did I miss something?

reefdog · 2024-02-29T16:21:37Z

I also notice that this PR doesn't release the proposal_submitter_email requirement on bulk uploads, even though it's no longer used for anything. Totally fine if you want to consider that a follow-up downstream effect, but let me know if that's the case or if it was an oversight.

Just checking... I thought that was taken out? Did I miss something?

When I tested it locally against the front-end, I received UI errors that I traced back to log output from the service saying the email was required. Let me re-run that now to verify…

Okay yup, confirmed, on this branch submitting a CSV without that field returns an error:

res: {
  "statusCode": 200,
  "headers": {
    "x-powered-by": "Express",
    "access-control-allow-origin": "*",
    "content-type": "application/json; charset=utf-8",
    "content-length": "8770",
    "etag": "W/\"2242-1TYeA6Gw9uZVhLCIzaa6dC0rVTU\""
  }
}
responseTime: 9
[2024-02-29T16:20:53.582Z] INFO (78074): Bulk upload has failed
source: "/Users/justin/Projects/ots/clients/pdc/service/src/jobQueue.ts"
scope: {
  "label": "job",
  "workerId": "worker-867e36f1b4425319c1",
  "taskIdentifier": "processBulkUpload",
  "jobId": "57"
}
err: {
  "type": "Error",
  "message": "proposal_submitter_email is a required field.",
  "stack":
      Error: proposal_submitter_email is a required field.
          at /Users/justin/Projects/ots/clients/pdc/service/src/tasks/processBulkUpload.ts:142:10
          at Array.forEach (<anonymous>)
          at assertShortCodesIncludeRequiredFields (/Users/justin/Projects/ots/clients/pdc/service/src/tasks/processBulkUpload.ts:138:17)
          at assertShortCodesAreValid (/Users/justin/Projects/ots/clients/pdc/service/src/tasks/processBulkUpload.ts:164:2)
          at assertCsvContainsValidShortCodes (/Users/justin/Projects/ots/clients/pdc/service/src/tasks/processBulkUpload.ts:175:8)
          at processTicksAndRejections (node:internal/process/task_queues:95:5)
          at async assertBulkUploadCsvIsValid (/Users/justin/Projects/ots/clients/pdc/service/src/tasks/processBulkUpload.ts:195:2)
          at async processBulkUpload (/Users/justin/Projects/ots/clients/pdc/service/src/tasks/processBulkUpload.ts:362:3)
          at async doNext (/Users/justin/Projects/ots/clients/pdc/service/node_modules/graphile-worker/src/worker.ts:252:18)
}

slifty · 2024-02-29T19:54:40Z

@reefdog ahh I see it now; ok thanks, that should be fixed.

reefdog

It works! 🎉 Thanks. Does package.json need its version bumped, too?

The concept of applicant never really got used, and we now know that proposal data may or may not be associated with an applicant in the PDC. We also know that applicants are ONLY organizations at this stage of the PDC, and a proposal might actually be associated with more than one. For these reasons we are removing the concept of an applicant, and removing the requirement that an applicant be defined in order for a proposal to be created. Issue #777 Allow access to a new /organizations endpoint

slifty · 2024-03-05T15:59:43Z

The other PR talked about this too but we'll be taking version out of package.json (#807)

This field is no longer required by the API[1], so we can stop telling users it’s required. [1] PhilanthropyDataCommons/service#798 Issue #690 Remove `proposal_submitter_email` bulk upload requirement

slifty requested a review from reefdog February 27, 2024 18:40

slifty force-pushed the 777-remove-applicants branch from 558b729 to 6a82e33 Compare February 27, 2024 18:42

slifty changed the base branch from main to 777-add-organizations February 27, 2024 18:44

reefdog requested changes Feb 28, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

src/database/operations/load/__test__/loadBundle.int.test.ts Outdated Show resolved Hide resolved

slifty force-pushed the 777-add-organizations branch 3 times, most recently from e9cb673 to 42a9178 Compare February 29, 2024 15:10

slifty commented Feb 29, 2024

View reviewed changes

slifty force-pushed the 777-add-organizations branch 2 times, most recently from 2020d04 to 0556d19 Compare February 29, 2024 19:47

slifty force-pushed the 777-remove-applicants branch 2 times, most recently from ec750ac to 72a86f9 Compare February 29, 2024 19:54

slifty requested a review from reefdog February 29, 2024 19:54

reefdog reviewed Feb 29, 2024

View reviewed changes

reefdog approved these changes Mar 1, 2024

View reviewed changes

Base automatically changed from 777-add-organizations to main March 1, 2024 02:02

slifty force-pushed the 777-remove-applicants branch from 72a86f9 to 789ef97 Compare March 5, 2024 15:57

slifty merged commit 7ade935 into main Mar 5, 2024
3 checks passed

slifty deleted the 777-remove-applicants branch March 5, 2024 15:59

reefdog mentioned this pull request Apr 16, 2024

Remove proposal_submitter_email bulk upload requirement PhilanthropyDataCommons/front-end#690

Closed

bickelj mentioned this pull request Apr 25, 2024

Implement GET /applicants by externalId #315

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `Applicant` and `/applicants/*` endpoints #798

Remove `Applicant` and `/applicants/*` endpoints #798

slifty commented Feb 27, 2024 •

edited

Loading

codecov bot commented Feb 27, 2024 •

edited

Loading

reefdog left a comment

slifty Feb 29, 2024

slifty commented Feb 29, 2024

reefdog commented Feb 29, 2024 •

edited

Loading

slifty commented Feb 29, 2024

reefdog left a comment •

edited

Loading

slifty commented Mar 5, 2024

Remove Applicant and /applicants/* endpoints #798

Remove Applicant and /applicants/* endpoints #798

Conversation

slifty commented Feb 27, 2024 • edited Loading

codecov bot commented Feb 27, 2024 • edited Loading

Codecov Report

reefdog left a comment

Choose a reason for hiding this comment

slifty Feb 29, 2024

Choose a reason for hiding this comment

slifty commented Feb 29, 2024

reefdog commented Feb 29, 2024 • edited Loading

slifty commented Feb 29, 2024

reefdog left a comment • edited Loading

Choose a reason for hiding this comment

slifty commented Mar 5, 2024

Remove `Applicant` and `/applicants/*` endpoints #798

Remove `Applicant` and `/applicants/*` endpoints #798

slifty commented Feb 27, 2024 •

edited

Loading

codecov bot commented Feb 27, 2024 •

edited

Loading

reefdog commented Feb 29, 2024 •

edited

Loading

reefdog left a comment •

edited

Loading