CUMULUS-3757: FOR MASTER - Update granule upsert logic to allow updating collection info #3901
Conversation
Nnaga1
changed the title
There was a problem hiding this comment.
Hey Naga - a couple of questions/inconsistencies, but this should be quickly approve-able once they're sorted.
Please note, I verified tests/content matched #3891, and am relying on approved units for functionality in this review. If you'd like a deploy/full test, please let me know.
packages/api-client/src/granules.ts
Outdated
| @@ -602,6 +613,74 @@ export const associateExecutionWithGranule = async (params: { | |||
| }); | |||
| }; | |||
|
|
|||
| /** | |||
| * Bulk update granules' collectionId to a payload-specified collectionId in postgres | |||
There was a problem hiding this comment.
This docstring is different than #3891, is that intentional?
There was a problem hiding this comment.
unintentional, I just copied wrong probably, will change to what is in 3757
| @@ -58,7 +58,7 @@ const { getBucketsConfigKey } = require('@cumulus/common/stack'); | |||
| const { getDistributionBucketMapKey } = require('@cumulus/distribution-utils'); | |||
| const { constructCollectionId } = require('@cumulus/message/Collections'); | |||
|
|
|||
| const { create, patch, patchGranule } = require('../../endpoints/granules'); | |||
| const { create, patch, patchGranuleAndReturnStatus } = require('../../endpoints/granules'); | |||
There was a problem hiding this comment.
Note for followup: why is 'del' gone in main?
packages/db/src/lib/granule.ts
Outdated
| @@ -354,3 +356,26 @@ export const getGranulesByGranuleId = async ( | |||
| .where({ granule_id: granuleId }); | |||
| return records; | |||
| }; | |||
|
|
|||
| /** | |||
| * Get granules from table where granule_id matches provided granuleId | |||
There was a problem hiding this comment.
This docstring doesn't match the 18.5.3 docstring, is that intentional?
There was a problem hiding this comment.
unintentional copying, fixed
packages/db/src/lib/granule.ts
Outdated
| * Get granules from table where granule_id matches provided granuleId | ||
| * | ||
| * @param {Knex} knex - DB client or transaction | ||
| * @param {string} granuleId - Granule ID |
There was a problem hiding this comment.
This param is incorrect, and doesn't match 18.5.3
Nit: Also..... we shouldn't type the docstrings in .ts for already annotated params, for both PRs.
There was a problem hiding this comment.
changed 👍 due to copying probably ^
Summary: Summary of changes
Addresses CUMULUS-XX: Develop amazing new feature
Changes
PR Checklist